Open rnd256 opened 3 years ago
@endel it looks like you fixed related bug(s) with these commits:
https://github.com/colyseus/colyseus-unity3d/commit/e4bdfe3fc73c04ef149e545050a294f042d6de12
https://github.com/colyseus/colyseus-unity3d/commit/091ba1386c58a237060003d94279e6a7786efc3d
FYI this "not being able to remove delegates" issue existed before those commits too, so they're not the cause of the issue. But I do think the issue has something to do with this: https://github.com/colyseus/colyseus-unity3d/blob/34e08ed23293d8edab9306e9e05b7a3b16cb59c3/Assets/Colyseus/Runtime/Scripts/Serializer/Schema/Schema.cs#L295
Interesting, thanks for reporting again @rnd256, I'm gonna have a look at this during this week!
Hey @rnd256, I just figured what's happening. The instance of gameState.phase
is getting replaced in the first patch arriving from the server. Therefore the local phase
reference is not relevant anymore. By using gameState.phase
inside MyCallback2
it will work properly.
I think the issue here is that the way schema callbacks are attached can be confusing at times. I'd love to have a way to attach schema callbacks without relying on actual instances being present on the client-side. Attaching schema callbacks on deep structures can be particularly confusing.
I'd love to have a way to attach schema callbacks without relying on actual instances being present on the client-side. Attaching schema callbacks on deep structures can be particularly confusing.
By that, do you mean for example being able to declare a gameState.phase
OnChange
handler that persists even if phase
ends up being set to null
and/or a new Phase
instance? I agree that would be much easier to work with.
Hey @rnd256, I just figured what's happening. The instance of
gameState.phase
is getting replaced in the first patch arriving from the server. Therefore the localphase
reference is not relevant anymore. By usinggameState.phase
insideMyCallback2
it will work properly.
Thanks for looking into this! Are you saying that any change to gameState.phase
's inner values (ie. phase.number
) changing invalidates the reference to phase
on the client-side, or is this something special to do with the first patch? Why does a new Phase
instance need to be created client-side if only its inner value(s) are changing?
By that, do you mean for example being able to declare a
gameState.phase
OnChange
handler that persists even ifphase
ends up being set tonull
and/or a newPhase
instance? I agree that would be much easier to work with.
That's exactly what I mean! š Glad you're positive about this!
is this something special to do with the first patch?
Exactly! You'll see that the generated GameState.cs
file contains phase = new Phase()
. This first instance is going to be thrown away as soon as the server sends the first patch (but the callbacks are copied over). After the first patch, the instance is going to remain the same.
That's exactly what I mean! š Glad you're positive about this!
One thing I should mention is that it's still important to be able to create OnChange
handlers for specific items in an ArraySchema
or MapSchema
. But I think those are the only cases where listening on specific objects is desired, and in all other cases listening on schema path is desired.
Exactly! You'll see that the generated
GameState.cs
file containsphase = new Phase()
. This first instance is going to be thrown away as soon as the server sends the first patch (but the callbacks are copied over). After the first patch, the instance is going to remain the same.
Ahh thanks, that makes sense. I didn't realize that all the values in the state were just empty containers immediately after successfully joining a room, and that you had to wait for the first patch before having the real game state and setting up connections. In that case, I'm going to try to adjust my code that currently awaits colyseusClient.JoinOrCreate()
to also await the first state patch. I wonder if it would be a good idea for Colyseus to not resolve the JoinOrCreate()
promise until the first state patch has been registered?
(same goes for colyseusClient.Reconnect()
)
(same goes for
colyseusClient.Reconnect()
)That's exactly what I mean! š Glad you're positive about this!
One thing I should mention is that it's still important to be able to create
OnChange
handlers for specific items in anArraySchema
orMapSchema
. But I think those are the only cases where listening on specific objects is desired, and in all other cases listening on schema path is desired.Exactly! You'll see that the generated
GameState.cs
file containsphase = new Phase()
. This first instance is going to be thrown away as soon as the server sends the first patch (but the callbacks are copied over). After the first patch, the instance is going to remain the same.Ahh thanks, that makes sense. I didn't realize that all the values in the state were just empty containers immediately after successfully joining a room, and that you had to wait for the first patch before having the real game state and setting up connections. In that case, I'm going to try to adjust my code that currently awaits
colyseusClient.JoinOrCreate()
to also await the first state patch. I wonder if it would be a good idea for Colyseus to not resolve theJoinOrCreate()
promise until the first state patch has been registered?
Have you found any way to make the OnChange method work? regards
I already found the solution, your comments were helpful, thank you
private void ClientAdd(string key, DataClientLobby data)
{
var clientLobby = Instantiate(clientLobby_prefab);
clientLobby.Init(data, key.Equals(lobby.SessionId));
entities.Add(data, clientLobby);
data.OnChange += OnChange;
onStateChange?.Invoke();
}
Colyseus Unity client version: 0.14.7 (latest) Colyseus server version: 0.14.1 (not latest, but issue is client-side)
If you store a reference to a
Schema
object other than the root schema, later (after a schema patch?) attempting to remove a delegate from that reference'sOnChange
event will fail silently.See comments in the example client code below for more specifics on what's going wrong. It looks like Colyseus is doing something to the
OnChange
references that's breaking the ability to remove delegates.Example server code:
Example client code: