onChange is triggered when elements are removed

[ipsquiggle]

We've just upgraded to the most recent version of colyseus, and noticed we were getting lots of null exceptions in the onChange handlers on the client.

An investigation revealed that this line has changed from else if to just if.

schema/src/Schema.ts

Line 1028 in aef7a9a

if (change.value !== change.previousValue) {

This means that when an element is removed, the onRemove fires (with previousValue for the value), then onChange fires with value as the value -- which at this point is definitely null.

This defies my intuition about the meaning of these callbacks; the fact that they are two separate callbacks (instead of e.g. just an onChange(currentVal, previousVal, key)) makes me expect that onChange would always have a valid value passed in.

It's also surprising because previously I had to listen to all three call backs (add, change, remove) to monitor the full lifecycle of objects, but now if I do that I get two callbacks when an item is removed.

---

Ultimately, just looking for guidance on whether this is a bug or the new design, and I will update our code accordingly. :)

[endel]

Hi @ipsquiggle, I apologize for the inconvenience but this is currently the intended behavior - reference on migration guide.

---

I'm still not very happy with how the callback API works. It feels something is off by calling methods directly from what was supposed to be raw data structures.

I'd love to detach the callback API from the data structures themselves in the future. Suggestions are very welcome. Cheers!

[ipsquiggle]

I see it in the guide, thanks for the update!

Agreed, that part of the API is awkward, and I've spent some time thinking about it. The problem seems to be that you need "existing data" to be able to define the callbacks on that data (or children of it).

Two alternatives keep coming to mind, though neither is obviously better, but for the sake of discussion:

1. Inside-out event handling: Allow defining callbacks from within schema class, or a client-only wrapper class that the deserializer automatically initializes. This moves Colyeseus to be a bit more like an entity syncing library, which is probably not where you want to go (explicitly not "raw data structures") but may be the lesser evil?
   • Advantages: You define callbacks in the definition of the data i.e. the class, rather than on instances.
   • Cons: Data intermingles with code; Need some way of separating client behaviour from server behaviour against the data.
2. Path-style handling: Define callbacks based on a path, and colyesus distributes them automatically, e.g. state.on('player/items[*]', (item) => {} ). This is probably difficult to get right and I don't know if it's intuitive or not, but it feels well suited to "tree shaped data" like Colyseus has.
   • Advantages: Can define callbacks ahead of time, anticipating that the data will exist at a path eventually.
   • Cons: unusual; impossible for type safety?

[damian-pastorini]

Hello guys!
The recap and my two cents of what I've been chatting with @endel :
The main idea would be to use a global sync events handler, that way we can define events disregarding having any existent data or instances defined.
This will avoid any current nested definition like the current ones:
player.onAdd > onChange / listen > your callback

The colyseus client could have an EventsManager instance that will emit sync events for every message received, for example:
colyseus.emit(receivedMessageKey, completeMessageData); // this would be something like an internal method
On the client side users will be able to listen events even without having the client "connected".

colyseus.on(messageKey, (messageData) => {
    // do your stuff
}

The main point here will be to use the most simple and intuitive name convention for the messageKey, like:
player.propertyChange, or player.removed, etc.
And the message data would have all the required information related, like:

colyseus.on('player.property.update', (messageData) => {
    console.log(messageData.propertyKey, messageData.propertyValue, messageData.propertyPreviousValue); 
}

Or:

colyseus.on('player.specificPropertyName', (messageData) => {
    console.log(messageData.value, messageData.previousValue); 
}

Other benefits will be:

• Making colyseus client complete detached from your game, since you can define all the events at once or whenever you need.
• You can add/remove listeners at any point of the game (which I think is currently impossible with callbacks)
• Getting all the attach / detach events features (like adding or removing multiple listeners at the same time)
• We could you almost any lib in the market, like: https://www.npmjs.com/package/eventemitter3

[ipsquiggle]

@damian-pastorini Makes sense to me. My main concern would be what this looks like for collections of complex elements, and how filtering might look?

First case:

// players is a  of Player
const playersListAddedKey = 'players.added';
const playersEntryAddedKey = 'players.*.added';

The first key triggers when the players field changes, e.g. state.players = new ArraySchema<Player>().
The second key triggers when the items in the players list change e.g. state.players.push(new Player())

It gets a bit necessarily funny when things nest:

// in practice I'd probably build up this key from parts, but ultimately it looks like this:
const heldItemAmountChangedKey = `players.*.items.*.amount.changed`;

Does that make sense? Is there a simpler way of doing this that I'm not seeing?

---

The second case is filtering. If I want to listen to only the first player's items for whatever reason, I need to either: only register the listener on a filtered portion of the tree, or, have enough context in the handler to react properly. I can't actually even imagine what the second case looks like. Here's some imaginings of what filtering might look like

The key could accept direct mappings into collections as a filter:

// simplest index or map key filter -- likely these would be dynamically created
const firstPlayerHealthKey = 'players.[0].health.changed';

Alternatively, we could provide a filter function that slices up the key:

// callbacks in the registration methods for complex filtering
colyseus.onFiltered('players.*', (item, key, subset) => {
  if (key === 0) {
    subset.on('health.changed', () => { } );
  }
});

---

Just thinking out loud here in the hope that it helps.

Throughout all of this, type safety is high on my mind, I'm quite dubious that it can be achieved in this model, but hopefully at least immediate runtime validation could be performed?

[damian-pastorini]

hi @ipsquiggle , ok, let's tackle one by one 😄

---

As for the first items, nested properties, I'm not sure why you are adding those "*", remember these are listeners on the client, it doesn't require to filter any specifics, we are talking about how to avoid the current callbacks implementation.
That been said, indeed as you said, you can listen to the players array schema or for an especific player.property:

// listen when a player is added to the state would look like this:
colyseus.listen('players.add', (playerData) => {}); // you can even get something like (playerKey, playerData) => {}
// then the remove would be like:
colyseus.listen('players.remove', (playerId) => {});
// listen a property would be:
colyseus.listen('player.propertyKey', (newValue, previosValue) => {});

As you can see you will always need to know in advance the properties to be listened, but that's already the case on 0.15 version and so you can have a pre-build list of properties defined to be listened on the client, or you can just go crazy and:

const playerProps = Object.keys(new Player()); // no need a real instance with data, just a new object instance
for (let prop of playerProps) {
    colyseus.listen('player.'+prop, (newValue, previousValue)=> {}); // the trick here is that you can't have nested propertiest
}

But if you need to go deeper you could in the exact same way:

// don't even need to add the word "change"on the event you can assume it's a change when it's about properties
colyseus.listen('player.propertyABorC.subProperty', (newValue, previosValue) => {});

---

As for the filter... I think you are mixing server with client side? You would filter on the server, the client would only receive the updates that will regard to that client, so when you listen for the events on the client you can keep using the events noted above. Or I'm missing something here?

---

Thanks!

[ipsquiggle]

Concerning the Star syntax in the paths

The reason for the * is as mentioned: to distinguish listening to the field containing the array, vs the contents of the array. I see I caused confusion by introducing filters, so let's forget about filters for the moment. Consider these actions on the server, and how you'd distinguish them on the client:

// server
schema.players = new ArraySchema(); // change to the property
schema.players.push(new Player()); // change to the collection
schema.players.at(0).health = 5; // change to item in the collection

Existing 0.15 handling of these:

// client
schema.listen('players', (val: ArraySchema) => {} ); // detect whether the property changed
schema.players.onChange((val: Player) => {}); // detect whether the collection changed
schema.players.onAdd((p: Player) => {
  p.listen('health', (val: number) => {});
}); // detect whether an item in the collection changed << THIS IS THE THING WE HATE

I don't enjoy the syntax of the * but it is just there to illustrate the difference between these scenarios. It could just as easily be entries etc. (Note that my examples have changed a bit from above, as I've thought about this more, sorry if that's confusing.)

// client
colyseus.listen('schema.players.onchange', (val: ArraySchema) => {});
colyseus.listen('schema.players.entries.onchange', (val: Player) => {});
colyseus.listen('schema.players.entries.health.onchange', (val: number) => {});

This distinction is needed because the collection itself is an object with a lifecycle we care about. I worry that there's been another distinction lost here, which is that we've conflated the entries-as-a-set with the entry-itself. I don't think it matters, but it maybe does. (e.g. schema.players.entries.onchange > does this mean the contents of the collection changed, or that an object in the collection had a general onchange event?)

---

In your examples, you listen to the Player object directly.. this may work most of the time, but not always. For example, I have a PointSchema type that I use all over the place, and so I need to listen to its x and y properties in the context of the parent data, never in isolation, as it can mean many different things.

---

I'll be honest, the more I think about this the more it scares me. It's definitely worth exploring, and I really would like to decouple the events from the objects. But it's not obviously better. :(

[damian-pastorini]

Ok, I think you are mixing how schemas and properties works, consider it like this:

• Schemas can only be added or removed from a container (MapSchema, whatever).
• Properties can only be changed (so you can see why we can remove the word "change" from the event listener).

That been said, you can easily know what and where you need to listen, even if it's an schema inside another, or map inside a map, etc.

Here some examples:

1. state.players > is a MapSchema that will contain PlayerSchema instances
2. PlayerSchema.name > is a property type string
3. PlayerSchema.health > is a property type int
4. PlayerSchema.location > is a LocationSchema
5. LocationSchema.x > is a property type int
6. LocationSchema.y > is a property type int
7. LocationSchema.scene > is a property type string
8. PlayerSchema.inventory > is a MapSchema that will contain ItemSchema instances
9. ItemSchema.key > is a property type string

If I'm missing any case here please show me and I would translate it to events, so for those cases you would listen to:

1. colyseus.listen('players.add', (playerId, playerData) => {});
   or colyseus.listen('players.remove', (playerId) => {});
2. colyseus.listen('player.name', (newName, oldName) => {});
3. colyseus.listen('player.health', (newValue, oldValue) => {});
4. you don't need a listener to this because it will exist on the player
5. colyseus.listen('player.location.x', (newValue, oldValue) => {});
6. colyseus.listen('player.location.y', (newValue, oldValue) => {});
7. colyseus.listen('player.location.scene', (newValue, oldValue) => {});
8. colyseus.listen('player.inventory.add', (itemId, newItemData) => {});
   or colyseus.listen('player.inventory.remove', (itemId) => {});
9. colyseus.listen('player.inventory.item.key', (newValue, oldValue) => {});

Even if you have something like the InventorySchema used in another object, you can still listen to the events for the same object:

colyseus.listen('npc.inventory.add', (itemId, itemData) => {});

For me it would be incredible simple using events, I really hope this kicks in :D

[hunkydoryrepair]

Indeed, there is a lot of listener creation necessary. When something is ADDED to a MapSchema or ArraySchema, it is common you then need to add listeners for changes on the newly added item.

I think a big consideration should be Closures (and how to avoid them). Probably the ability to pass in a context. We have to use closures now, which captures the full call stack and frankly just burns up memory. Being able to set a context would save a lot of memory on the closures.

For example, if each PlayerSchema includes a MapSchema, I want a listener on the MapSchema for every player, and I need to add a listener for every Schema within the map schema.

So, during the onChange I get the key into the map (or array), and the new and previous values, but unless we have the player from the closure, we don't know really where to apply the update. We use closures and so use a lot more memory than if we could set the context on the callback.

Using a system where we have a single listener for these changes, like listen('player.inventory.change' would be really powerful, but we need the full path in the call back (the player key, and the inventory index ). Possibly an array of keys?

[endel]

Thank you so much for sharing your thoughts and ideas @ipsquiggle @damian-pastorini @hunkydoryrepair 💙
This week I finally started to experiment & tinker about this, too. Hopefully next week I have something to share!

PS: I've just posted a big roadmap for v3 of schemas here 🙌 colyseus/colyseus#709

[endel]

Hi guys!

So, I've been banging my head for a few days trying to come up with an API, and so far, this seems to be achievable and not too different from what we currently have. (I'm not 100% sure this is viable, I'm currently experimenting with the actual implementation...)

This implementation would use proxies to attach callbacks to structures located at a path in the state.

(It brings nostalgic jQuery feelings...)

$ = getStateCallbacks(room); // The SDK may provide this as "room.$"
$(room.state) // returns a proxy
$(room.state).teams // returns a proxy
$(room.state).childSchema // returns a proxy

There are 2 types of proxies: a proxy to a collection and a proxy to a schema instance:

• Proxy to collections can call
  • onAdd()
  • onRemove()
• Proxy to schema instances can call
  • access child schema or collection properties (returns a new Proxy)
  • listen(propertyName, callback)
  • onChange()

---

onAdd / onRemove in "collections"

$(state).teams.onAdd((team, index) => {/* ... */});
$(state).teams.onRemove((team, index) => {/* ... */});

The state.teams instance here may not be available yet. We can "access" it because of the proxy. The new callback system will need to figure out the right time to attach the callback whenever teams become available.

Alternatively, if the state.teams is available, you could do this instead (it works the same as previous example):

$(state.teams).onAdd((team, index) => {/* ... */});
$(state.teams).onRemove((team, index) => {/* ... */});

deep structures

The same pattern we're used to having would apply here:

$(state).teams.onAdd((team, index) => {
    $(team).entities.onAdd((entity, entityId) => {
        // entity is the actual instance!
    });
});

New method proposal: bindTo() / mapTo() ??

This is a new method I'm thinking of providing. Instead of manually mapping "network properties" to "frontend objects", one could map either all properties, or selected properties that are applied on the frontend object.

$(team).entities.onAdd((entity, entityId) => {
    const frontendObject = createFrontEndObject();
    
    // bind all properties
    $(entity).position.bindTo(frontendObject);

    // bind *some* properties   
    $(entity).position.bindTo(frontendObject, ["x", "y"]);
});

Naming things is hard so not sure how can we call this method. (Help?? 😆)

---

This re-writing and de-coupling will allow for other strategies to "listen" for changes within the state in the future, which may be useful for some applications. One example could be getting the raw "change list" directly as a callback - without binding callbacks to specific structures. Maybe one that fits the React ecosystem as well, by returning copies of a specific portion of the state whenever it changes.

I'm afraid the "stack / context" consideration pointed by @hunkydoryrepair won't be solved in the current proposed strategy. Only a new approach could fix that. As I'd like to keep compatibility and migration as easy as possible, I think that's something we could try with a different "strategy" in the future.

I'm thinking of shipping v3 of schemas with 2 strategies: a "raw changelist callback" and this new callback system with API similar to what we currently have.

---

Please let me know if you have feedback! Thank you for caring and reading all this!

[sylvainpolletvillard]

Hi there,
I use colyseus/schema with React and a Redux store and can confirm manually mapping "network properties" to "frontend objects" is the most common and tedious part of my code. It often looks like this:

room.state.users.onAdd((u) => {
    dispatch(addUser(u))
    const fields: NonFunctionPropNames<User>[] = ["x","y"]
    fields.forEach((field) => {
      u.listen(field, (value) => {
         dispatch(changeUser(u, field, value))
      }
   })
})

room.state.users.onRemove((u) => {
   dispatch(removeUser(u))
})

multiplied X times the number of schemas i have. If my schema contains nested schemas, i have to nest the same callback-to-action-dispatch pipeline for each nested schema.

So obviously i'm much interested in the bindTo() method mentionned above. I'm just wondering how reactivity would work with this system, since React requires immutability and explicit state changes.