add 'onChange' callback back to collections. #155

Author: endel

package.json

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

src/decoder/DecodeOperation.ts

@@ -75,7 +75,7 @@ export function decodeValue(
             // });
         }
 
-        value = null;
+        value = undefined;
     }
 
     if (operation === OPERATION.DELETE) {

src/decoder/strategy/StateCallbacks.ts

@@ -79,12 +79,15 @@ type CollectionCallback<K, V> = {
      */
     onRemove(callback: (item: V, index: K) => void): () => void;
 
-    // /**
-    //  * Trigger callback when an item has been removed to the collection.
-    //  *
-    //  * @param callback
-    //  */
-    // onChange(callback: (item: V, index: K) => void): void;
+    /**
+     * Trigger callback when the value on a key has changed.
+     *
+     * THIS METHOD IS NOT RECURSIVE!
+     * If you want to listen to changes on individual items, you need to attach callbacks to the them directly inside the `onAdd` callback.
+     *
+     * @param callback
+     */
+    onChange(callback: (item: V, index: K) => void): void;
 };
 
 type OnInstanceAvailableCallback = (callback: (ref: Ref, existing: boolean) => void) => void;
@@ -190,7 +193,12 @@ export function getDecoderStateCallbacks<T extends Schema>(decoder: Decoder<T>):
                 }
 
                 // trigger onChange
-                if (change.value !== change.previousValue) {
+                if (
+                    change.value !== change.previousValue &&
+                    // FIXME: see "should not encode item if added and removed at the same patch" test case.
+                    // some "ADD" + "DELETE" operations on same patch are being encoded as "DELETE"
+                    (change.value !== undefined || change.previousValue !== undefined)
+                ) {
                     const replaceCallbacks = $callbacks[OPERATION.REPLACE];
                     for (let i = replaceCallbacks?.length - 1; i >= 0; i--) {
                         replaceCallbacks[i](change.value, change.dynamicIndex ?? change.field);
@@ -339,6 +347,10 @@ export function getDecoderStateCallbacks<T extends Schema>(decoder: Decoder<T>):
                 return $root.addCallback($root.refIds.get(ref), OPERATION.DELETE, callback);
             };
 
+            const onChange = function (ref: Ref, callback: (value: any, key: any) => void) {
+                return $root.addCallback($root.refIds.get(ref), OPERATION.REPLACE, callback);
+            };
+
             return new Proxy({
                 onAdd: function(callback: (value, key) => void, immediate: boolean = true) {
                     //
@@ -375,6 +387,21 @@ export function getDecoderStateCallbacks<T extends Schema>(decoder: Decoder<T>):
                         return onRemove(context.instance, callback);
                     }
                 },
+                onChange: function(callback: (value, key) => void) {
+                    if (context.onInstanceAvailable) {
+                        // collection instance not received yet
+                        let detachCallback = () => {};
+
+                        context.onInstanceAvailable((ref: Ref) => {
+                            detachCallback = onChange(ref, callback)
+                        });
+
+                        return () => detachCallback();
+
+                    } else if (context.instance) {
+                        return onChange(context.instance, callback);
+                    }
+                },
             }, {
                 get(target, prop: string) {
                     if (!target[prop]) {
@@ -390,7 +417,7 @@ export function getDecoderStateCallbacks<T extends Schema>(decoder: Decoder<T>):
     }
 
     function $<T>(instance: T): CallbackProxy<T> {
-        return getProxy(undefined, { instance }) as CallbackProxy<T>;
+        return getProxy(undefined, { instance }) as unknown as CallbackProxy<T>;
     }
 
     return $;

test/MapSchema.test.ts

@@ -372,7 +372,7 @@ describe("Type: MapSchema", () => {
         assertDeepStrictEqualEncodeAll(state);
     });
 
-    it("should not encode item if added and removed at the same patch", () => {
+    it("should not encode item if added and removed at the same patch (Schema child)", () => {
         const state = new State();
         state.mapOfPlayers = new MapSchema<Player>();
         state.mapOfPlayers.set('one', new Player("Jake", 10, 10));
@@ -383,9 +383,10 @@ describe("Type: MapSchema", () => {
 
         let onRemoveCalls = 0;
         let onAddCalls = 0;
-
-        $(decodedState).mapOfPlayers.onRemove(() => onRemoveCalls++);
-        $(decodedState).mapOfPlayers.onAdd(() => onAddCalls++);
+        let onChangeCalls = 0;
+        $(decodedState).mapOfPlayers.onRemove((value, key) => onRemoveCalls++);
+        $(decodedState).mapOfPlayers.onAdd((_, key) => onAddCalls++);
+        $(decodedState).mapOfPlayers.onChange((_, key) => onChangeCalls++);
 
         decodedState.decode(state.encode());
 
@@ -396,9 +397,9 @@ describe("Type: MapSchema", () => {
         const patchBytes = state.encode();
 
         //
-        // TODO: improve me! `DELETE` operation should not be encoded here.
-        // this test conflicts with encodeAll() + encode() for other structures, where DELETE operation is necessary.
-        // // assert.deepStrictEqual([ 4, 1, 0, 1, 11, 193 ], patchBytes);
+        // TODO / FIXME: There's an additional 2 bytes for the "remove" operation here, even though no "add" was made.
+        // (the "ADD" + "DELETE" operations on same patch are being encoded as "DELETE")
+        // // assert.deepStrictEqual([ 255, 2, 129, 11, 255, 1 ], Array.from(patchBytes));
         //
 
         decodedState.decode(patchBytes);
@@ -408,8 +409,56 @@ describe("Type: MapSchema", () => {
         state.mapOfPlayers.delete('one');
 
         decodedState.decode(state.encode());
+        assert.deepStrictEqual(state.toJSON(), decodedState.toJSON());
+
+        assert.strictEqual(1, onRemoveCalls);
+        assert.strictEqual(1, onAddCalls);
+        assert.strictEqual(2, onChangeCalls);
+
+        assertDeepStrictEqualEncodeAll(state);
+    });
+
+    it("should not encode item if added and removed at the same patch (primitive child)", () => {
+        class MyState extends Schema {
+            @type({ map: "string" }) mapOfStrings = new MapSchema<string>();
+        }
+        const state = new MyState();
+        state.mapOfStrings.set('one', "one");
+
+        const decodedState = new MyState();
+        const $ = getCallbacks(decodedState);
+
+        let onRemoveCalls = 0;
+        let onAddCalls = 0;
+        let onChangeCalls = 0;
+        $(decodedState).mapOfStrings.onRemove(() => onRemoveCalls++);
+        $(decodedState).mapOfStrings.onAdd(() => onAddCalls++);
+        $(decodedState).mapOfStrings.onChange(() => onChangeCalls++);
+
+        decodedState.decode(state.encode());
+
+        state.mapOfStrings.set('two', "two");
+        state.mapOfStrings.delete('two');
+
+        const patchBytes = state.encode();
+
+        //
+        // TODO / FIXME: There's an additional 2 bytes for the "remove" operation here, even though no "add" was made.
+        // (the "ADD" + "DELETE" operations on same patch are being encoded as "DELETE")
+        // // assert.deepStrictEqual([ 255, 2, 129, 11, 255, 1 ], Array.from(patchBytes));
+        //
+
+        decodedState.decode(patchBytes);
+        assert.strictEqual(0, onRemoveCalls);
+
+        state.mapOfStrings.delete('one');
+
+        decodedState.decode(state.encode());
+        assert.deepStrictEqual(state.toJSON(), decodedState.toJSON());
 
         assert.strictEqual(1, onRemoveCalls);
+        assert.strictEqual(1, onAddCalls);
+        assert.strictEqual(2, onChangeCalls);
 
         assertDeepStrictEqualEncodeAll(state);
     });