New test cases for encoding/decoding complex schemas

[mirageisland-ai]

This is a PR adding tests as a reproduction for this issue: #203

[endel]

Hi @mirageisland-ai! 👋 Are you still having issues in your project after this? #200 (comment)

Thanks for taking the time here, I noticed your tests are creating a new instance of decodedState for every encode - this indeed is going to cause refId not found errors, as the .encode() method is incremental.

• Reusing the same decodedState instance with decodedState.decode(state.encode()) simulates following state patches
• In order to create a new decodedState in the middle of operations, it's required to call state.encodeAll() to ensure the client has the previous instances, and not only their mutations.

I've updated your test (see in full here) and didn't notice any refId not found there.

Your tests do reveal inconsistent reference counting on the decoding side, though, which should be investigated! (They emerge exactly due to .encodeAll() in late joins!)

diff --git a/test/Encoder.test.ts b/test/Encoder.test.ts
index 761c790..0a98c60 100644
--- a/test/Encoder.test.ts
+++ b/test/Encoder.test.ts
@@ -1,7 +1,7 @@
 import * as assert from "assert";
 import { MapSchema, Schema, type, ArraySchema, defineTypes, Reflection, Encoder, $changes, entity } from "../src";
 
-import { State, Player, getCallbacks, assertDeepStrictEqualEncodeAll, createInstanceFromReflection, getEncoder, getDecoder } from "./Schema";
+import { State, Player, getCallbacks, assertDeepStrictEqualEncodeAll, createInstanceFromReflection, getEncoder, getDecoder, assertRefIdCounts } from "./Schema";
 
 describe("Encoder", () => {
     const bufferSize = Encoder.BUFFER_SIZE;
@@ -32,21 +32,14 @@ describe("Encoder", () => {
          * Helper that encodes the state, decodes into a fresh reflected instance,
          * and asserts deep equality + refId/refCount match between encoder & decoder.
          */
-        function encodeDecodeAndAssert<S extends Schema>(state: S) {
-            const encoder = getEncoder(state);
-            const bytes = state.encode();
-            const decodedState = createInstanceFromReflection(state);
-            const decoder = getDecoder(decodedState);
-            decodedState.decode(bytes);
-            assertDeepStrictEqualEncodeAll(state);
-            // Ensure refId counts match between encoder & decoder
-            for (const refId in encoder.root.refCount) {
-                assert.strictEqual(
-                    encoder.root.refCount[refId],
-                    decoder.root.refCounts[refId] ?? 0,
-                    `refCount mismatch for refId=${refId}`
-                );
+        function encodeDecodeAndAssert<S extends Schema>(state: S, decodedState?: S) {
+            if (!state['_decoder']) {
+                // simulate joining the room with existing full state
+                decodedState.decode(state.encodeAll());
             }
+            decodedState.decode(state.encode());
+            assertRefIdCounts(state, decodedState);
+            assertDeepStrictEqualEncodeAll(state);
         }
 
         it("should handle moving shared instances between array <-> map <-> field", () => {
@@ -60,6 +53,7 @@ describe("Encoder", () => {
             }
 
             const state = new Container();
+            const decodedState = createInstanceFromReflection(state);
 
             // create two shared items
             const sword = new Item();
@@ -67,22 +61,22 @@ describe("Encoder", () => {
 
             // initial placement
             state.list.push(sword, shield);
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // move "sword" from array to map key "sword"
             state.bag.set("sword", sword);
             state.list.splice(state.list.indexOf(sword), 1);
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // equip the sword (shared reference now field & map)
             state.equipped = sword;
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // unequip and move back to array
             state.equipped = undefined;
             state.list.push(sword);
             state.bag.delete("sword");
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
         });
 
         it("should replace instances multiple times in nested structures", () => {
@@ -95,25 +89,27 @@ describe("Encoder", () => {
             }
 
             const state = new Parent();
+            const decodedState = createInstanceFromReflection(state);
+
             const c1 = new Child().assign({ value: 1 });
             const c2 = new Child().assign({ value: 2 });
             const c3 = new Child().assign({ value: 3 });
 
             state.a = c1;
             state.b = c1; // shared initially
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // replace a with new instance
             state.a = c2;
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // replace b with another new instance
             state.b = c3;
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // finally point both to same again
             state.a = state.b;
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
         });
 
         it("should survive clear & repopulate on ArraySchema with shared children", () => {
@@ -126,25 +122,27 @@ describe("Encoder", () => {
             }
 
             const state = new Graph();
+            const decodedState = createInstanceFromReflection(state);
+
             // add 5 nodes, share references between array & map
             for (let i = 0; i < 5; i++) {
                 const n = new Node();
                 state.nodes.push(n);
                 state.lookup.set(n.id, n);
             }
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // clear array – map still holds them
             state.nodes.clear();
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // repopulate array using map values (shared again)
             state.lookup.forEach(node => state.nodes.push(node));
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
 
             // now clear map completely
             state.lookup.clear();
-            encodeDecodeAndAssert(state);
+            encodeDecodeAndAssert(state, decodedState);
         });
     });
 });

[mirageisland-ai]

Hi again @endel 👋 Thank you for fixing my tests! Apologies for the mistake, I'm not very familiar with the the codebase yet

I can confirm the scenario in issue #200 is fixed after upgrading to version 3.0.37. After trying that fix, I created a few tests to try out some new schema configurations before trying them out in game. But since the tests had issues, the schemas may actually work in game even though the tests failed. I'll create a new Github issue if I run into anything 😅

With your fixed versions of my tests, only 2/3 new tests are failing, but for reasons related to the ref count rather than the refId not found, as you mentioned. I'll update #203 to reflect that!