Add new method to observables API

Feature requests
1

Hi guys! Glad to see everyone here again :smile:

I have a proposal for the observables API - Observables | Babylon.js Documentation.

Currently, the API have the next 3 methods: add, remove, removeCallback. The remove method accepts Observer instance, but add accepts callback function. It feels somehow counterintuitive that API for adding ad removing observers have different signatures.

It would be super clear if we were to have 4 methods: add, addCallback/addObserver, remove, removeCallback. That way we have 2 methods which works exclusively with observers and 2 for callbacks. What do you think?

2

Hi!

One usually doesn’t create an observer on their own, and this is why adding a callback makes more sense. Now, I understand what you are saying - a more consistent API would have made more sense here. So, whereas I agree that addCallback might have been more consistent with what the API does, we can’t change it due to backwards compatibility.
I think it might be possible to accept both a callback and an observer in the add function, but, TBH, you are the first one that asks that. The steps to create an observer and pass it are being masked for you, so I assume most people will continue adding a callback.

Note that there is also a usability reason to simplify add and remove - when adding a callback the function you passed is referenced, so you are going to have to keep a reference of this specific function to remove it. it best displayed in the add and remove event listener in the browser:

// this event listener cannot be removed - we don't have a reference to the function
window.addEventListener("whatever", myFunction.bind(this));
// same here
window.addEventListener("whatever", () => { console.log("fun fun fun"); });

if addEventListener would return an observer that can be removed that would be awesome!

Another small note - if you ever argue that it would make sense to have a removefunction on the observer, I would probably agree with you :slight_smile:

2 Likes
3

I don’t see the use-case for creating your own observer… adding a callback function to it, before then adding it to a observerable,
rather than the current straight forward method of just add callback to observerable and observer is automaticly created.

the observable.add() returns the observer object if you for some reason need to access it, or save it to remove it from the observable later in the code.

let observer = scene.onBeforeRenderObservable.add( () => {});

vs

let observer = new BABYLON.Observer( () => {});
scene.onBeforeRenderObservable.addObserver( observer );

1 Like
4

Thanks for your response! It totally makes sense to me that the first example is shorter, but I have 2 concerns about it:

  1. It returns Nullable<Observer<T>>, therefore if I want to manipulate with observer directly I should first check whether it is not a null and only after that perform some actions. It is not the case when we create observer manually. Use-case for manipulating observer directly: I want to set unregisterOnNextCall on some particular events.
  2. While it returns observer, I feel a bit odd to use it that way. For me, the add function looks more like a thing that performs some work and does not return anything. Maybe it just me :person_shrugging:
5

I have never had a null return from an observerable.add() :slight_smile:
and i believe the only case it would ever do that, is if you don’t give it a callback.

If you need an observerable to run only once, you can use
scene.onBeforeRenderObservable.addOnce( () => {});

If in your case you need to update the unregisterOnNextCall based on some logic / after multiple callbacks, updating the observer directly is the only choice, here’s two methods;

let observer;
observer = scene.onBeforeRenderObservable.add((a,b) => {
    if(1 === 1){
        observer.unregisterOnNextCall = true;
        // Method 2;
        // if callback is NOT bound ( function(){}.bind(variable) )
        // this === observer, so this.unregisterOnNextCall = true; does the same
    }
    console.log(observer, this)
});
1 Like
6

I’m just judging from the type declaration. Even if it can’t be null most of the time typescript still force us to do such checks because it is possible to have null instead of the observer.

I saw that function, and it is very useful, but if I want to run it only once after a particular event, sadly, it won’t do the work(

7

Thanks for such details answer. I would still appreciate if it can be viewed as one of the TODO things in future releases :smiley: . The idea with remove method of Observer looks good to me as well :+1: