StateView: using view.add() before instance is set to state throws error

Author: endel

package.json

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

src/encoder/StateView.ts

@@ -42,14 +42,30 @@ export class StateView {
 
     // TODO: allow to set multiple tags at once
     add(obj: Ref, tag: number = DEFAULT_VIEW_TAG, checkIncludeParent: boolean = true) {
-        if (!obj?.[$changes]) {
+        const changeTree: ChangeTree = obj?.[$changes];
+
+        if (!changeTree) {
             console.warn("StateView#add(), invalid object:", obj);
             return this;
+
+        } else if (
+            !changeTree.parent &&
+            changeTree.refId !== 0 // allow root object
+        ) {
+            /**
+             * TODO: can we avoid this?
+             *
+             * When the "parent" structure has the @view() tag, it is currently
+             * not possible to identify it has to be added to the view as well
+             * (this.addParentOf() is not called).
+             */
+            throw new Error(
+                `Cannot add a detached instance to the StateView. Make sure to assign the "${changeTree.ref.constructor.name}" instance to the state before calling view.add()`
+            );
         }
 
         // FIXME: ArraySchema/MapSchema do not have metadata
         const metadata: Metadata = obj.constructor[Symbol.metadata];
-        const changeTree: ChangeTree = obj[$changes];
         this.visible.add(changeTree);
 
         // add to iterable list (only the explicitly added items)

test/StateView.test.ts

@@ -30,7 +30,38 @@ describe("StateView", () => {
         assertEncodeAllMultiple(encoder, state, [client1, client2])
     });
 
-    it("should allow adding detached instances to the view (not immediately attached)", () => {
+    it("should not be required to add parent structure", () => {
+        /**
+         * TODO/FIXME: this test is asserting to throw an error, but it should
+         * actually work instead.
+         */
+
+        class Item extends Schema {
+            @type("number") amount: number;
+        }
+
+        class State extends Schema {
+            @type("string") prop1 = "Hello world";
+            @view() @type([Item]) items = new ArraySchema<Item>();
+        }
+
+        const state = new State();
+        const encoder = getEncoder(state);
+
+        const client1 = createClientWithView(state);
+
+        const item = new Item().assign({ amount: 0 });
+        assert.throws(() => {
+            client1.view.add(item);
+        }, /Make sure to assign the "Item" instance to the state before calling view.add/);
+
+        // state.items.push(item);
+        // encodeMultiple(encoder, state, [client1]);
+        // assert.strictEqual(client1.state.items.length, 1);
+        // assertEncodeAllMultiple(encoder, state, [client1])
+    });
+
+    xit("should allow adding detached instances to the view (not immediately attached)", () => {
         class Item extends Schema {
             @type("number") amount: number;
         }