ArraySchema: fixes consecutive .shift() calls + encodeAll()

Author: endel

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@colyseus/schema",
-  "version": "3.0.25",
+  "version": "3.0.26",
   "description": "Binary state serializer with delta encoding for games",
   "bin": {
     "schema-codegen": "./bin/schema-codegen",

src/encoder/ChangeTree.ts

@@ -267,7 +267,7 @@ export class ChangeTree<T extends Ref=any> {
         const newIndexes = {};
         for (const index in this.indexedOperations) {
             newIndexedOperations[Number(index) + shiftIndex] = this.indexedOperations[index];
-            newIndexes[Number(index) + shiftIndex] = changeSet[index];
+            newIndexes[Number(index) + shiftIndex] = changeSet.indexes[index];
         }
         this.indexedOperations = newIndexedOperations;
         changeSet.indexes = newIndexes;
@@ -372,6 +372,7 @@ export class ChangeTree<T extends Ref=any> {
 
         this.indexedOperations[index] = operation ?? OPERATION.DELETE;
         setOperationAtIndex(changeSet, index);
+        deleteOperationAtIndex(this.allChanges, allChangesIndex);
 
         const previousValue = this.getValue(index);
 
@@ -390,8 +391,6 @@ export class ChangeTree<T extends Ref=any> {
             this.root?.remove(previousValue[$changes]);
         }
 
-        deleteOperationAtIndex(this.allChanges, allChangesIndex);
-
         //
         // FIXME: this is looking a ugly and repeated
         //

src/types/custom/ArraySchema.ts

@@ -360,11 +360,13 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
         if (this.items.length === 0) { return undefined; }
 
         // const index = Number(Object.keys(changeTree.indexes)[0]);
-        const index = this.tmpItems.findIndex((item, i) => item === this.items[0]);
         const changeTree = this[$changes];
 
-        changeTree.delete(index);
-        changeTree.shiftAllChangeIndexes(-1, index);
+        const index = this.tmpItems.findIndex(item => item === this.items[0]);
+        const allChangesIndex = this.items.findIndex(item => item === this.items[0]);
+
+        changeTree.delete(index, OPERATION.DELETE, allChangesIndex);
+        changeTree.shiftAllChangeIndexes(-1, allChangesIndex);
 
         // this.deletedIndexes[index] = true;
 

test/ArraySchema.test.ts

@@ -94,6 +94,53 @@ describe("ArraySchema Tests", () => {
             assert.strictEqual(0, state.turns.length);
         });
 
+        it("consecutive shift calls should not break 'encodeAll'", () => {
+            class Entity extends Schema {
+                @type("number") i: number;
+            }
+            class MyState extends Schema {
+                @type([Entity]) entities;
+            }
+
+            const state = new MyState();
+            state.entities = [];
+
+            for (let i = 0; i < 5; i++) {
+                state.entities.push(new Entity().assign({ i }));
+            }
+
+            const decodedState = createInstanceFromReflection(state);
+            decodedState.decode(state.encode())
+
+            const entitiesChangeTree: ChangeTree = state.entities[$changes];
+            assert.deepStrictEqual({ '0': 0, '1': 1, '2': 2, '3': 3, '4': 4 }, entitiesChangeTree.allChanges.indexes);
+
+            state.entities.shift();
+            assert.deepStrictEqual({ '0': 1, '1': 2, '2': 3, '3': 4 }, entitiesChangeTree.allChanges.indexes);
+
+            state.entities.shift();
+            assert.deepStrictEqual({ '0': 2, '1': 3, '2': 4 }, entitiesChangeTree.allChanges.indexes);
+
+            state.entities.shift();
+            assert.deepStrictEqual({ '0': 3, '1': 4 }, entitiesChangeTree.allChanges.indexes);
+
+            state.entities.shift();
+            assert.deepStrictEqual({ '0': 4 }, entitiesChangeTree.allChanges.indexes);
+
+            assertDeepStrictEqualEncodeAll(state);
+
+            decodedState.decode(state.encode())
+            assertRefIdCounts(state, decodedState);
+
+            state.entities.shift();
+
+            decodedState.decode(state.encode())
+            assertRefIdCounts(state, decodedState);
+
+            // encode all
+            assertDeepStrictEqualEncodeAll(state);
+        });
+
         it("mutate previous instance + shift", () => {
             /**
              * This test shows that flagging the `changeSet` item as `undefined`
@@ -123,14 +170,16 @@ describe("ArraySchema Tests", () => {
             for (let i = 0; i < 2; i++) {
                 for (let j = 0; j < 2; j++) {
                     state.entities.forEach((entity) => entity.i++);
-                    const entity = state.entities.shift();
+                    state.entities.shift();
                 }
-
                 decodedState.decode(state.encode());
             }
 
             assertDeepStrictEqualEncodeAll(state);
             assertRefIdCounts(state, decodedState);
+
+            state.entities.shift();
+            assertDeepStrictEqualEncodeAll(state);
         });
     });
 
@@ -369,6 +418,8 @@ describe("ArraySchema Tests", () => {
             console.log("--- 5 ---")
 
             assert.strictEqual(0, state.turns.length);
+
+            assertDeepStrictEqualEncodeAll(state);
         });
     });
 
@@ -439,6 +490,8 @@ describe("ArraySchema Tests", () => {
 
             decodedState.decode(state.encode());
             assert.deepStrictEqual([0, 1, 2, 3, 4], decodedState.arrayOfNumbers.toJSON());
+
+            assertDeepStrictEqualEncodeAll(state);
         });
 
         it("push, unshift, pop", () => {
@@ -460,6 +513,8 @@ describe("ArraySchema Tests", () => {
 
             decodedState.decode(state.encode());
             assert.deepStrictEqual([0, 1, 2, 3], decodedState.arrayOfNumbers.toJSON());
+
+            assertDeepStrictEqualEncodeAll(state);
         });
 
         it("push, pop, unshift", () => {
@@ -481,6 +536,8 @@ describe("ArraySchema Tests", () => {
 
             decodedState.decode(state.encode());
             assert.deepStrictEqual([0, 1, 2, 3], decodedState.arrayOfNumbers.toJSON());
+
+            assertDeepStrictEqualEncodeAll(state);
         });
 
         it("push, shift, unshift", () => {