fix ChangeTree encoding order when sharing references to an instance.

Author: endel

package.json

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

src/encoder/ChangeTree.ts

@@ -62,7 +62,7 @@ export function deleteOperationAtIndex(changeSet: ChangeSet, index: number) {
     delete changeSet.indexes[index];
 }
 
-function enqueueChangeTree(
+export function enqueueChangeTree(
     root: Root,
     changeTree: ChangeTree,
     changeSet: 'changes' | 'filteredChanges' | 'allFilteredChanges',
@@ -384,6 +384,8 @@ export class ChangeTree<T extends Ref=any> {
             this.root?.remove(previousValue[$changes]);
         }
 
+        deleteOperationAtIndex(this.allChanges, allChangesIndex);
+
         //
         // FIXME: this is looking a ugly and repeated
         //
@@ -392,7 +394,6 @@ export class ChangeTree<T extends Ref=any> {
             enqueueChangeTree(this.root, this, 'filteredChanges');
 
         } else {
-            deleteOperationAtIndex(this.allChanges, allChangesIndex);
             enqueueChangeTree(this.root, this, 'changes');
         }
     }

src/encoder/Root.ts

@@ -1,7 +1,7 @@
 import { OPERATION } from "../encoding/spec";
 import { TypeContext } from "../types/TypeContext";
 import { spliceOne } from "../types/utils";
-import { ChangeTree, setOperationAtIndex } from "./ChangeTree";
+import { ChangeTree, enqueueChangeTree, setOperationAtIndex } from "./ChangeTree";
 
 export class Root {
     protected nextUniqueId: number = 0;
@@ -71,6 +71,23 @@ export class Root {
 
         } else {
             this.refCount[changeTree.refId] = refCount;
+
+            //
+            // When losing a reference to an instance, it is best to move the
+            // ChangeTree to the end of the encoding queue.
+            //
+            // This way, at decoding time, the instance that contains the
+            // ChangeTree will be available before the ChangeTree itself. If the
+            // containing instance is not available, the Decoder will throw
+            // "refId not found" error.
+            //
+            if (changeTree.filteredChanges !== undefined) {
+                this.removeChangeFromChangeSet("filteredChanges", changeTree);
+                enqueueChangeTree(this, changeTree, "filteredChanges");
+            } else {
+                this.removeChangeFromChangeSet("changes", changeTree);
+                enqueueChangeTree(this, changeTree, "changes");
+            }
         }
 
         changeTree.forEachChild((child, _) => this.remove(child));
@@ -80,9 +97,8 @@ export class Root {
 
     removeChangeFromChangeSet(changeSetName: "allChanges" | "changes" | "filteredChanges" | "allFilteredChanges", changeTree: ChangeTree) {
         const changeSet = this[changeSetName];
-        const index = changeSet.indexOf(changeTree);
-        if (index !== -1) {
-            spliceOne(changeSet, index);
+        if (spliceOne(changeSet, changeSet.indexOf(changeTree))) {
+            changeTree[changeSetName].queueRootIndex = -1;
             // changeSet[index] = undefined;
         }
     }

test/InstanceSharing.test.ts

@@ -460,7 +460,7 @@ describe("Instance sharing", () => {
 
     });
 
-    xit("replacing collection of items while keeping a reference to an item", () => {
+    it("replacing collection of items while keeping a reference to an item", () => {
         class Song extends Schema {
             @type("string") url: string;
         }
@@ -480,14 +480,10 @@ describe("Instance sharing", () => {
         const state = new State();
         const decodedState = new State();
 
-        console.log(">> encode()")
         decodedState.decode(state.encode());
 
         state.buckets.set(sessionId, new Player());
-
-        console.log(">> encode()")
         decodedState.decode(state.encode());
-        console.log(Schema.debugRefIds(state));
 
         const newSong = new Song().assign({ url: "song2" });
         state.buckets.get(sessionId).queue.push(newSong);
@@ -498,8 +494,6 @@ describe("Instance sharing", () => {
         state.playing = state.buckets.get(sessionId).queue.shift();
         state.queue = new ArraySchema<Song>();
 
-        console.log(">> encode()")
-        console.log(Schema.debugRefIds(state));
         decodedState.decode(state.encode());
         assert.deepStrictEqual(state.toJSON(), decodedState.toJSON());
     });

test/Schema.ts

@@ -134,7 +134,7 @@ export function encodeMultiple<T extends Schema>(encoder: Encoder<T>, state: T,
     return encodedViews;
 }
 
-export function assertEncodeAllMultiple<T extends Schema>(encoder: Encoder<T>, state: T, referenceClients: Array<{ state: Schema, view: StateView }>) {
+export function encodeAllMultiple<T extends Schema>(encoder: Encoder<T>, state: T, referenceClients: Array<{ state: Schema, view: StateView }>) {
     const clients = referenceClients.map((client) => createClientWithView(state, client.view));
 
     const it = { offset: 0 };
@@ -157,6 +157,12 @@ export function assertEncodeAllMultiple<T extends Schema>(encoder: Encoder<T>, s
         return encoded;
     });
 
+    return { clients, encodedViews };
+}
+
+export function assertEncodeAllMultiple<T extends Schema>(encoder: Encoder<T>, state: T, referenceClients: Array<{ state: Schema, view: StateView }>) {
+    const { clients, encodedViews } = encodeAllMultiple(encoder, state, referenceClients);
+
     referenceClients.forEach((referenceClient, i) => {
         assert.deepStrictEqual(referenceClient.state.toJSON(), clients[i].state.toJSON(), `client${i + 1} state mismatch`);
     });

test/StateView.test.ts

@@ -1,6 +1,6 @@
 import * as assert from "assert";
 import { Schema, type, view, ArraySchema, MapSchema, StateView, Encoder, ChangeTree, $changes, OPERATION } from "../src";
-import { createClientWithView, encodeMultiple, assertEncodeAllMultiple, getDecoder, getEncoder, createInstanceFromReflection, encodeAllForView } from "./Schema";
+import { createClientWithView, encodeMultiple, assertEncodeAllMultiple, getDecoder, getEncoder, createInstanceFromReflection, encodeAllForView, encodeAllMultiple } from "./Schema";
 import { getDecoderStateCallbacks } from "../src/decoder/strategy/StateCallbacks";
 import { nanoid } from "nanoid";
 
@@ -841,6 +841,73 @@ describe("StateView", () => {
 
             assertEncodeAllMultiple(encoder, state, [client1, client2])
         });
+
+        it("replacing collection of items while keeping a reference to an item", () => {
+            class Song extends Schema {
+                @type("string") url: string;
+            }
+
+            class Player extends Schema {
+                @type([Song]) queue = new ArraySchema<Song>();
+            }
+
+            class State extends Schema {
+                @type(Song) playing: Song = new Song();
+                @view() @type([Song]) queue = new ArraySchema<Song>();
+                @type({ map: Player }) buckets = new MapSchema<Player>();
+            }
+
+            const sessionId = "session1";
+
+            const state = new State();
+            const encoder = getEncoder(state);
+
+            const client1 = createClientWithView(state);
+
+            encodeMultiple(encoder, state, [client1]);
+
+            state.buckets.set(sessionId, new Player());
+
+            encodeMultiple(encoder, state, [client1]);
+            console.log(Schema.debugRefIds(state));
+
+            const newSong = new Song().assign({ url: "song2" });
+            state.buckets.get(sessionId).queue.push(newSong);
+
+            state.queue = new ArraySchema<Song>();
+            state.queue.push(newSong);
+
+            state.playing = state.buckets.get(sessionId).queue.shift();
+            state.queue = new ArraySchema<Song>();
+
+            encodeMultiple(encoder, state, [client1]);
+
+            assert.doesNotThrow(() =>
+                encodeAllMultiple(encoder, state, [client1]));
+        });
+
+        it("setting a non-view field to undefined should not interfere on encoding", () => {
+            class Song extends Schema {
+                @type("string") url: string;
+            }
+
+            class State extends Schema {
+                @type(Song) playing: Song = new Song();
+                @view() @type([Song]) queue = new ArraySchema<Song>();
+            }
+
+            const state = new State();
+            const encoder = getEncoder(state);
+
+            const client1 = createClientWithView(state);
+            encodeMultiple(encoder, state, [client1]);
+
+            state.playing = undefined;
+            encodeMultiple(encoder, state, [client1]);
+
+            assert.doesNotThrow(() =>
+                encodeAllMultiple(encoder, state, [client1]));
+        });
     });
 
     describe("checkIsFiltered", () => {