fix 'Decode warning: trying to remove refId' when removing array items.

Author: endel

package.json

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

src/Schema.ts

@@ -19,8 +19,6 @@ export class Schema {
     static [$encoder] = encodeSchemaOperation;
     static [$decoder] = decodeSchemaOperation;
 
-    // public [$changes]: ChangeTree;
-
     /**
      * Assign the property descriptors required to track changes on this instance.
      * @param instance
@@ -177,18 +175,18 @@ export class Schema {
     /**
      * Inspect the `refId` of all Schema instances in the tree. Optionally display the contents of the instance.
      *
-     * @param instance Schema instance
+     * @param ref Schema instance
      * @param showContents display JSON contents of the instance
      * @returns
      */
-    static debugRefIds(instance: Ref, showContents: boolean = false, level: number = 0) {
-        const ref = instance;
-        const changeTree = ref[$changes];
-
+    static debugRefIds(ref: Ref, showContents: boolean = false, level: number = 0) {
         const contents = (showContents) ? ` - ${JSON.stringify(ref.toJSON())}` : "";
 
+        const changeTree: ChangeTree = ref[$changes];
+        const refId = changeTree.refId;
+
         let output = "";
-        output += `${getIndent(level)}${ref.constructor.name} (refId: ${ref[$changes].refId})${contents}\n`;
+        output += `${getIndent(level)}${ref.constructor.name} (refId: ${refId})${contents}\n`;
 
         changeTree.forEachChild((childChangeTree) =>
             output += this.debugRefIds(childChangeTree.ref, showContents, level + 1));

src/decoder/ReferenceTracker.ts

@@ -56,7 +56,7 @@ export class ReferenceTracker {
 
         if (refCount === undefined) {
             try {
-                throw new DecodingWarning("trying to remove refId that doesn't exist");
+                throw new DecodingWarning("trying to remove refId that doesn't exist: " + refId);
             } catch (e) {
                 console.warn(e);
             }

src/types/custom/ArraySchema.ts

@@ -38,7 +38,6 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
      * - Then, the encoder iterates over all "owned" properties per instance and encodes them.
      */
     static [$filter] (ref: ArraySchema, index: number, view: StateView) {
-        // console.log("ArraSchema[$filter] VIEW??", !view)
         return (
             !view ||
             typeof (ref[$childType]) === "string" ||
@@ -767,6 +766,7 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
 
     protected [$deleteByIndex](index: number) {
         this.items[index] = undefined;
+        this.tmpItems[index] = undefined; // TODO: do not try to get "tmpItems" at decoding time.
     }
 
     protected [$onEncodeEnd]() {
@@ -776,6 +776,7 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
 
     protected [$onDecodeEnd]() {
         this.items = this.items.filter((item) => item !== undefined);
+        this.tmpItems = this.items.slice(); // TODO: do no use "tmpItems" at decoding time.
     }
 
     toArray() {

test/ArraySchema.test.ts

@@ -1245,43 +1245,6 @@ describe("ArraySchema Tests", () => {
         assertDeepStrictEqualEncodeAll(state);
     });
 
-    xit("TODO: fix 'Decode warning: trying to remove refId that doesn't exist'", () => {
-        class Item extends Schema {
-            @type("string") name: string;
-            constructor(name) {
-                super();
-                this.name = name;
-            }
-        }
-        class Player extends Schema {
-            @type([Item]) items = new ArraySchema<Item>();
-        }
-        class State extends Schema {
-            @type(Player) player = new Player();
-        }
-
-        const state = new State();
-        const decodedState = new State();
-        state.player.items.push(new Item("Item 1"));
-        state.player.items.push(new Item("Item 2"));
-        decodedState.decode(state.encodeAll());
-
-        decodedState.decode(state.encode());
-        assert.deepStrictEqual(state.toJSON(), decodedState.toJSON());
-
-        // Remove Item 1
-        state.player.items.splice(0, 1);
-        decodedState.decode(state.encode());
-
-        assert.deepStrictEqual(state.toJSON(), decodedState.toJSON());
-        state.player.items.splice(0, 1);
-        decodedState.decode(state.encode());
-
-        assert.deepStrictEqual(state.toJSON(), decodedState.toJSON());
-
-        assertDeepStrictEqualEncodeAll(state);
-    });
-
     it("should allow to transfer object between ArraySchema", () => {
         class Item extends Schema {
             @type("uint8") id: number;
@@ -1976,6 +1939,42 @@ describe("ArraySchema Tests", () => {
             assert.strictEqual(50, copy[0]);
             assert.strictEqual(100, copy[1]);
         });
+
+        it("fix 'Decode warning: trying to remove refId that doesn't exist'", () => {
+            /**
+             * This test was triggering "Decode warning: trying to remove refId that doesn't exist"
+             * when removing items from an ArraySchema.
+             *
+             * The issue was caused by ArraySchema using `tmpItems` during `$getByIndex`.
+             * TODO: need to avoid using `tmpItems` during decoding.
+             */
+            class Item extends Schema {
+                @type("string") name: string;
+            }
+            class Player extends Schema {
+                @type([Item]) items = new ArraySchema<Item>();
+            }
+            class State extends Schema {
+                @type(Player) player = new Player();
+            }
+
+            const state = new State();
+            const decodedState = new State();
+            state.player.items.push(new Item().assign({ name: "Item 1" }));
+            state.player.items.push(new Item().assign({ name: "Item 2" }));
+            decodedState.decode(state.encodeAll());
+
+            decodedState.decode(state.encode());
+
+            state.player.items.splice(0, 1);
+            decodedState.decode(state.encode());
+
+            state.player.items.splice(0, 1);
+            decodedState.decode(state.encode());
+
+            assert.deepStrictEqual(state.toJSON(), decodedState.toJSON());
+            assertDeepStrictEqualEncodeAll(state);
+        });
     });
 
 });