changeset: cache linked list position when reordering

Author: endel

package.json

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

src/decoder/Decoder.ts

@@ -67,7 +67,7 @@ export class Decoder<T extends Schema = any> {
                 // Trying to access a reference that haven't been decoded yet.
                 //
                 if (!nextRef) {
-                    throw new Error(`"refId" not found: ${nextRefId}`);
+                    // throw new Error(`"refId" not found: ${nextRefId}`);
                     console.error(`"refId" not found: ${nextRefId}`, { previousRef: ref, previousRefId: this.currentRefId });
                     console.warn("Please report this issue to the developers.");
                     this.skipCurrentStructure(bytes, it, totalBytes);

src/encoder/ChangeTree.ts

@@ -41,13 +41,13 @@ export interface ChangeTreeNode {
     changeTree: ChangeTree;
     next?: ChangeTreeNode;
     prev?: ChangeTreeNode;
+    position: number; // Cached position in the linked list for O(1) lookup
 }
 
 // Linked list for change trees
 export interface ChangeTreeList {
     next?: ChangeTreeNode;
     tail?: ChangeTreeNode;
-    length: number;
 }
 
 export interface ChangeSet {
@@ -63,7 +63,7 @@ function createChangeSet(queueRootNode?: ChangeTreeNode): ChangeSet {
 
 // Linked list helper functions
 export function createChangeTreeList(): ChangeTreeList {
-    return { next: undefined, tail: undefined, length: 0 };
+    return { next: undefined, tail: undefined };
 }
 
 export function setOperationAtIndex(changeSet: ChangeSet, index: number) {

src/encoder/Encoder.ts

@@ -59,7 +59,7 @@ export class Encoder<T extends Schema = any> {
         let current: ChangeTreeList | ChangeTreeNode = this.root[changeSetName];
 
         while (current = current.next) {
-            const changeTree = current.changeTree;
+            const changeTree = (current as ChangeTreeNode).changeTree;
 
             if (hasView) {
                 if (!view.isChangeTreeVisible(changeTree)) {
@@ -284,8 +284,8 @@ export class Encoder<T extends Schema = any> {
 
     get hasChanges() {
         return (
-            this.root.changes.length > 0 ||
-            this.root.filteredChanges.length > 0
+            this.root.changes.next !== undefined ||
+            this.root.filteredChanges.next !== undefined
         );
     }
 }

src/encoder/Root.ts

@@ -85,7 +85,7 @@ export class Root {
 
                     } else if (child.parentChain) {
                         // re-assigning a child of the same root, move it next to parent
-                        this.recursivelyMoveNextToParent(child);
+                        this.moveNextToParent(child);
                     }
                 }
             });
@@ -135,6 +135,16 @@ export class Root {
         const parentNode = parent[$changes][changeSetName]?.queueRootNode;
         if (!parentNode || parentNode === node) return;
 
+        // Use cached positions - no iteration needed!
+        const parentPosition = parentNode.position;
+        const childPosition = node.position;
+
+        // If child is already after parent, no need to move
+        if (childPosition > parentPosition) return;
+
+        // Child is before parent, so we need to move it after parent
+        // This maintains decoding order (parent before child)
+
         // Remove node from current position
         if (node.prev) {
             node.prev.next = node.next;
@@ -159,77 +169,10 @@ export class Root {
         }
 
         parentNode.next = node;
-    }
 
-    // moveSubtreeToEndOfChangeTreeList(changeSetName: ChangeSetName, changeTree: ChangeTree): void {
-    //     // Find the contiguous range of nodes that belong to this subtree
-    //     const subtreeRange = this.findSubtreeRange(changeTree, changeSetName);
-    //     if (!subtreeRange) return;
-
-    //     const changeSet = this[changeSetName];
-    //     const { firstNode, lastNode } = subtreeRange;
-
-    //     // If the last node is already at the tail, no need to move
-    //     if (lastNode === changeSet.tail) return;
-
-    //     // Remove the entire subtree range from current position
-    //     if (firstNode.prev) {
-    //         firstNode.prev.next = lastNode.next;
-    //     } else {
-    //         changeSet.next = lastNode.next;
-    //     }
-
-    //     if (lastNode.next) {
-    //         lastNode.next.prev = firstNode.prev;
-    //     } else {
-    //         changeSet.tail = firstNode.prev;
-    //     }
-
-    //     // Add the entire subtree to the end
-    //     firstNode.prev = changeSet.tail;
-    //     lastNode.next = undefined;
-
-    //     if (changeSet.tail) {
-    //         changeSet.tail.next = firstNode;
-    //     } else {
-    //         changeSet.next = firstNode;
-    //     }
-
-    //     changeSet.tail = lastNode;
-    // }
-
-    // private findSubtreeRange(changeTree: ChangeTree, changeSetName: ChangeSetName): { firstNode: ChangeTreeNode, lastNode: ChangeTreeNode } | null {
-    //     const rootNode = changeTree[changeSetName].queueRootNode;
-    //     if (!rootNode) return null;
-
-    //     // Collect all refIds that belong to this subtree
-    //     const subtreeRefIds = new Set<number>();
-    //     this.collectSubtreeRefIds(changeTree, subtreeRefIds);
-
-    //     // Find the first and last nodes in the linked list that belong to this subtree
-    //     let firstNode: ChangeTreeNode | null = null;
-    //     let lastNode: ChangeTreeNode | null = null;
-    //     let current = this[changeSetName].next;
-
-    //     while (current) {
-    //         if (subtreeRefIds.has(current.changeTree.refId)) {
-    //             if (!firstNode) firstNode = current;
-    //             lastNode = current;
-    //         }
-    //         current = current.next;
-    //     }
-
-    //     return firstNode && lastNode ? { firstNode, lastNode } : null;
-    // }
-
-    // private collectSubtreeRefIds(changeTree: ChangeTree, result: Set<number>): void {
-    //     result.add(changeTree.refId);
-
-    //     // Collect children recursively
-    //     changeTree.forEachChild((child, _) => {
-    //         this.collectSubtreeRefIds(child, result);
-    //     });
-    // }
+        // Update positions after the move
+        this.updatePositionsAfterMove(changeSet, node, parentPosition + 1);
+    }
 
     public enqueueChangeTree(
         changeTree: ChangeTree,
@@ -244,7 +187,12 @@ export class Root {
     }
 
     protected addToChangeTreeList(list: ChangeTreeList, changeTree: ChangeTree): ChangeTreeNode {
-        const node: ChangeTreeNode = { changeTree, next: undefined, prev: undefined };
+        const node: ChangeTreeNode = {
+            changeTree,
+            next: undefined,
+            prev: undefined,
+            position: list.tail ? list.tail.position + 1 : 0
+        };
 
         if (!list.next) {
             list.next = node;
@@ -255,16 +203,42 @@ export class Root {
             list.tail = node;
         }
 
-        list.length++;
-
         return node;
     }
 
+    protected updatePositionsAfterRemoval(list: ChangeTreeList, removedPosition: number) {
+        // Update positions for all nodes after the removed position
+        let current = list.next;
+        let position = 0;
+
+        while (current) {
+            if (position >= removedPosition) {
+                current.position = position;
+            }
+            current = current.next;
+            position++;
+        }
+    }
+
+    protected updatePositionsAfterMove(list: ChangeTreeList, node: ChangeTreeNode, newPosition: number) {
+        // Recalculate all positions - this is more reliable than trying to be clever
+        let current = list.next;
+        let position = 0;
+
+        while (current) {
+            current.position = position;
+            current = current.next;
+            position++;
+        }
+    }
+
     public removeChangeFromChangeSet(changeSetName: ChangeSetName, changeTree: ChangeTree) {
         const changeSet = this[changeSetName];
         const node = changeTree[changeSetName].queueRootNode;
 
         if (node && node.changeTree === changeTree) {
+            const removedPosition = node.position;
+
             // Remove the node from the linked list
             if (node.prev) {
                 node.prev.next = node.next;
@@ -278,7 +252,8 @@ export class Root {
                 changeSet.tail = node.prev;
             }
 
-            changeSet.length--;
+            // Update positions for nodes that came after the removed node
+            this.updatePositionsAfterRemoval(changeSet, removedPosition);
 
             // Clear ChangeTree reference
             changeTree[changeSetName].queueRootNode = undefined;

test/InstanceSharing.test.ts

@@ -587,7 +587,7 @@ describe("Instance sharing", () => {
         assertDeepStrictEqualEncodeAll(state);
     });
 
-    it.only("decoder: should increment refId count of deep shared instances", () => {
+    it("decoder: should increment refId count of deep shared instances", () => {
         class Position extends Schema {
             @type("number") x: number;
             @type("number") y: number;