asadm / playroom-unity

9 stars 1 forks source link

UnsubscribeOnPlayerJoin and UnsubscribeOnQuit #51

Closed momintlh closed 2 months ago

momintlh commented 2 months ago

Added functions to unsubscribe to onQuit and OnPlayerJoin based on #48

SaadBazaz commented 2 months ago

Have you tested the following scenario(s):

Unregistering only that specific callback

Pseudocode:

onPlayerJoin(createPlayer)
spawnPlayerUnsubscriber = onPlayerJoin(spawnPlayer)
...
// let's say we do not want to spawn the player upon player joining, if the game hasn't started
if (gameHasNotStarted) {
   spawnPlayerUnsubscriber() // spawnPlayer will no longer be called upon onPlayerJoin. createPlayer will still be called, btw!
}

Unregistering multiple callbacks

Pseudocode:

createPlayerUnsubscriber = onPlayerJoin(createPlayer)
generatePlayerAvatarPlayerUnsubscriber = onPlayerJoin(generatePlayerAvatar)
...
...
createPlayerUnsubscriber() // this should only unsubscribe createPlayer, however generatePlayerAvatar should still fire.
...
generatePlayerAvatarPlayerUnsubscriber() // this should now unsubscribe generatePlayerAvatar
momintlh commented 2 months ago
image

blue is UnsubscribePrintPlayerNames = PlayroomKit.OnPlayerJoin(PrintPlayerName); and green is UnsubscribeAddPlayer = PlayroomKit.OnPlayerJoin(AddPlayer); UnsubscribePrintPlayerNames is called within PrintPlayerName so it shouldn't be called again but when the following line is executed: UnsubscribeAddPlayer = PlayroomKit.OnPlayerJoin(AddPlayer); butPrintPlayerName` is invoked again shown with as blue question mark. This is due to the code below.

Progress and The Real Issue

The ID based approach did work with returning unsubscribe(), but the actual problem seems to be on the Unity side: we have a function which handles the invoking of callbacks:

 private static void OnPlayerJoinWrapperCallback(string id)
        {
            var player = GetPlayer(id);
            foreach (var callback in OnPlayerJoinCallbacks)
            {
                callback?.Invoke(player);
            }
        }

This gets called everytime OnPlayerJoin is called. In OnPlayerJoin we are using:

OnPlayerJoinCallbacks.Add(onPlayerJoinCallback);

Basically, right now every callback within the list gets invoked every time OnPlayerJoin is called. In my build (logs attached) I am using OnPlayerJoin twice with 2 different callbacks, the first one prints the player's name and ID, the other actually instantiates the player's gameObject. I am getting 2 calls for both callbacks with this setup.

So even if we unsub from the JS side there are callbacks within the list in Unity.

SaadBazaz commented 2 months ago

The ID based approach did work with returning unsubscribe(), but the actual problem seems to be on the Unity side: we have a function which handles the invoking of callbacks:

 private static void OnPlayerJoinWrapperCallback(string id)
        {
            var player = GetPlayer(id);
            foreach (var callback in OnPlayerJoinCallbacks)
            {
                callback?.Invoke(player);
            }
        }

Then we may have to refactor this part as well, and use just one array for storing callbacks and their unsubscriberIds. As it'll be hard to manage two arrays which have similar purpose.

We could filter out the unsubscribed IDs.

momintlh commented 2 months ago

After the recent push this is the console: image

When OnPlayerJoin is called again with AddPlayer() as callback, the previous (unsubscribed one is not invoked).

SaadBazaz commented 2 months ago

@momintlh Please resolve conflicts