schema icon indicating copy to clipboard operation
schema copied to clipboard

ArraySchema push/splice/push, item added at wrong index.

Open Zielak opened this issue 4 years ago • 4 comments

I've made this bug reproducible at State handling example of colyseus-example: https://github.com/Zielak/colyseus-examples - branch master.

My use case

I want mark cards selected by the player in their hand. I'm using cardsSelected:ArraySchema<number>. Values are IDs of cards, and i wanted to use the array's index to know in which order the cards were selected.

image


The issue appeared when I tried to select and deselect the same card many times. I've pinned it down to minimal, reproducible case: add/remove/add-again:

image

Note:

  • the array is empty at the beginning,
  • the second onAdd comes with the item being added into index 1

I've also inspected web socket messages on client, and it seems that the server-side Colyseus is sending this incorrect key.

Zielak avatar Nov 12 '20 21:11 Zielak

Thanks for reporting and providing the example @Zielak, thanks also @Steditor for providing a similar scenario here https://github.com/colyseus/colyseus.js/issues/78#issuecomment-724973641 (I believe the use cases are the same)

This issue is going to be a bit difficult to solve. The "dynamicIndex" is going to keep increasing for new records in the array, no matter if you remove items from it or not.

You can get items from the ArraySchema from its "dynamicIndex" - this is not going to be enough, though:

arraySchema.getByIndex(dynamicIndex)

The ArraySchema indexes are very tricky to get right because of the many operations that could happen at once (.splice(), .shift(), .pop(), .push()), that's why I had to use an internal mapping of index -> value for it. There is also the @filterChildren() where local indexes for one client can differ from each other.

It might be possible to work around this somehow and provide the actual local index for the schema callbacks, I just don't want to overly complicate things, as they're already quite complicated! 😰

endel avatar Nov 13 '20 18:11 endel

So this dynamicIndex is going to keep increasing indefinitely. Can this become a concern in some cases, like: a constantly changing array in long-lasting games?

Anyway, I chose to work around this by having an array of Schemas instead and not rely on the arrays' indexes: [{ selectionIndex: 0, childIndex: 3 }] - 4th card in my hand was selected first :)

Zielak avatar Nov 15 '20 10:11 Zielak

Glad you've worked it out @Zielak

So this dynamicIndex is going to keep increasing indefinitely. Can this become a concern in some cases, like: a constantly changing array in long-lasting games?

Hm, well observed! Only if it surpasses Number.MAX_SAFE_INTEGER (9007199254740991). Somebody actually may have a use-case to break this 😅

endel avatar Nov 16 '20 13:11 endel

Could this be possibly related? https://github.com/colyseus/colyseus/issues/407

Honga1 avatar Apr 13 '21 09:04 Honga1