Registering and unregistering callbacks - best practice?

Unity Engine
,
1

Hi Folks,

I kind of struggle with the concept of registering and unregistering callbacks at the best of times. But now I have a context menu (runtime) for a game I’m making. The buttons on the menu do different things depending on the state some other part of the game.

So, I have some methods that I plan to use to show the menu, and set the callbacks on the buttons, but I’m unsure how to unregister them properly.

Example:

Some controller class:

          if (InCreationModeAndNotHovering())
            {
                RightClickMenu.ShowCreateNewRouteHoldMenu();
            }
            else if (InCreationModeAndHoveringNonRouteHold())
            {
                RightClickMenu.ShowAddExistingHoldToRouteMenu();
            }

… there are about 6 more conditionals.

Right Click menu class:

    public void ShowCreateNewRouteHoldMenu()
    {
        ClearPreviousCallbacks();
        contextTitle.text = "Add hold to wall";
        addHoldBtn.RegisterCallback<ClickEvent>(ev => CreateRouteHold());
        addFHandBtn.RegisterCallback<ClickEvent>(ev => CreateFHandHold());
        ...
        addRFootBtn.RegisterCallback<ClickEvent>(ev => CreareRFootHold());
        ShowMenu()
    }

    public void ShowAddExistingHoldToRouteMenu()
    {
        ClearPreviousCallbacks();
        contextTitle.text = "Add hold to wall";
        addHoldBtn.RegisterCallback<ClickEvent>(ev => AddExistingHoldToRoute());
        addFHandBtn.RegisterCallback<ClickEvent>(ev => AddExistingHoldToRouteAsFHandStart());
        ...
        addRFootBtn.RegisterCallback<ClickEvent>(ev => AddExistingHoldToRouteAsRFootStart());
        ShowMenu()
}

... more of these methods for other variations of the context menu.

public void ClearPreviousCallbacks (){
       addHoldBtn.UnRegisterCallback<ClickEvent>(ev => AddExistingHoldToRoute())
       addHoldBtn.UnRegisterCallback<ClickEvent>(ev => CreateRouteHold())
... etc. for all possible callbacks that could be added to the hold btn.

     ... then repeat for every other button for every possible callback that could be added...     
    this method could be 100 lines long.
}

Is this the best way to do this? Seems like a lot.

Edit, so the above doesn’t actually work. It registers the callbacks, but unregistercallback doesn’t seem to do anything. More and more callbacks just keep getting added, so the buttons trigger lots of events, including the same event multiple times, instead of just one event once.

1 Like
2

Before going into your actual issue, if your buttons are actually UIElements.Button objects, you are supposed to use their clicked property for this. It’s a normal C# event, so you use += and -= to register/unregister callbacks from it. There’s also a Button.clickable manipulator with more options for more complex behaviors.

That said, no matter what kind of event you are using, for unregistering a callback, the method must be identifiable as the same method that was registered. The thing is, every time you write a lambda (a method with the => symbol), you are creating a new method; so the methods you are unregistering are not the same as the methods you registered before.

The simplest way to handle this is to create separate named methods to register/unregister as the callbacks. For example:

void RegisterCallbacks()
{
    myButton.clicked += onMyButtonClicked;
    myElement.RegisterCallback<MouseEnterEvent>(OnMyElementEnter);
}

void UnregisterCallbacks()
{
    myButton.clicked -= OnMyButtonClicked;
    myElement.UnregisterCallback<MouseEnterEvent>(OnMyElementEnter)
}

// Some methods with a name we can use to Register/Unregister.
// Notice OnMyButtonClicked doesn't receive an event because the Button.clicked callback expects
// a parameterless method.

void OnMyButtonClicked()
{
    DoSomethingOnClick()
}

void OnMyElementEnter(MouseEnterEvent ev)
{
    DoSomethingOnEnter();
}

Now I’ll say my opinion about best practices for this case. It could be better to just register a callback once when creating a button and never bother with unregistering it. If you need to change some contextual data used in those callbacks, you can store that context in a variable or a property somewhere. You could also set that context variable to null when closing the menus to prevent bugs or memory problems, but maybe you don’t even need that.

This makes it harder to introduce a bug by forgetting to unregister a callback or by registering a callback twice. This way, if a button isn’t doing what it should do, there are less places to check for problems. It should make the code simpler and easier to reuse.

2 Likes
3

Thanks. This is a big help. Out of interest, is there an easy way to do what you have written above while also passing arguments to the events?

4

No problem. What do you mean passing arguments to the event?

5

In some cases, I might want to pass an object along with the event. Something like:

myButton.clicked += onMyButtonClicked;

public void onMyButtonClicked(MyObject obj) {
    Debug.Log(obj.value)
}

The thing being passed is not the event target or anything.

6

That’s the context I suggest you could store in a field somewhere. For example, before showing a menu, you could store the objects that are being edited in some field, maybe a static field, then the callbacks can access those objects through those fields.

1 Like