ArraySchema incorrect removed index (onRemove on client receives incorrect index)

[giuliano-marinelli]

When removes an item from a ArraySchema that's not the last one (for example: array.shift(), that removes first one), The onRemove(instance, key) defined in the Client receives as key and instance always the last one element of the array.

Example

We have a Schema called World that contains an array of Points (schema too) called test, and an action that add elements when data is "a: 1", or removes the first one element if data is "a: 2"

export class World extends Schema {  
  @type([Point])
  test = new ArraySchema<Point>();

  testAc(data) {
    if (data.a == 1) {
      this.test.push(new Point(0,1));
      this.test.push(new Point(0,2));
      this.test.push(new Point(0,3));
    } else (data.a == 2) {
      this.test.shift();
    }
  }
}

Then on Client:

room.send('testAc', {a: 1});
// result in the onChange of test in World
// [0] = Point(0,1)
// [1] = Point(0,2)
// [2] = Point(0,3)
room.send('testAc', {a: 2});
// result in the onChange of test in World
// [0] = Point(0,2)
// [1] = Point(0,3)
room.state.world.onRemove = (instance, key) => {
  console.log(instance, key);
 //result spected: Point(0,1), key: 0
 //BAD RESULT THAT WE RECEIVED INSTEAD: Point(0,3), key: 2
}

I'm using the Client in Angular 10 imported in this way:
import * as BABYLON from '@babylonjs/core/Legacy/legacy';

Thanks, Cheers :)

[endel]

Hi @giuliano-marinelli, this is a known issue and has been fixed already on the latest alpha version of Colyseus (colyseus/colyseus#357, colyseus/schema#82)

If you can I recommend upgrading to the latest alpha version, which seems to be stable so far (I'm using in 2 web projects), here is the preview of the migration guide for it: https://deploy-preview-45--colyseus-docs.netlify.app/migrating/0.14/

[giuliano-marinelli]

Thanks! I upgrade my project then 👍

[endel]

Version 0.14 is out! 🎉 https://github.com/colyseus/colyseus/releases/tag/0.14.0

[Steditor]

This still seems to be a problem in the current version (0.14.0 server / 0.14.1 js client): The client-side onRemove still receives indices that sometimes even are higher than the length of the synchronized array.

[endel]

Hi @Steditor, thanks for reporting, could you please provide more info about which operations you've done, and how is your structure represented so I can reproduce over here?

If you can provide a test scenario like this would be ideal: https://github.com/colyseus/schema/blob/master/test/ArraySchemaTest.ts#L9-L48

Cheers!

[Steditor]

I think I now understand, where the problem came from: I assumed that the calls to onRemove were atomic in the sense, that if you repeatedly remove the first element of an array using splice(0, 1), you'll always get the event "element at index 0 removed". From the test suite I now learned, that this is not the case, but that the passed index seems to be an internal one and not the actual index in the current array. (I'm not sure if that's expected behavior) In the error I described earlier, this internal index was higher than the number of elements still in the array.

Something else is fishy with the indices when removing and adding elements. Maybe it's me using the framework in a wrong way, but: As soon as you mix unshift and splice, the array is all mixed up. See https://github.com/Steditor/schema/blob/master/test/ArraySchemaTest.ts#L516-L555 for a test case.