Skip to content

Commit 5b17088

Browse files
endelendel
committed
ArraySchema: fixes when using consecutive .splice() calls.
1 parent ddbad34 commit 5b17088

File tree

10 files changed

+485
-66
lines changed

10 files changed

+485
-66
lines changed

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@colyseus/schema",
3-
"version": "3.0.26",
3+
"version": "3.0.27",
44
"description": "Binary state serializer with delta encoding for games",
55
"bin": {
66
"schema-codegen": "./bin/schema-codegen",

‎src/encoder/ChangeTree.ts

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import * as util from "util";
2+
13
import { OPERATION } from "../encoding/spec";
24
import { Schema } from "../Schema";
35
import { $changes, $childType, $decoder, $onEncodeEnd, $encoder, $getByIndex, $refTypeFieldIndexes, $viewFieldIndexes } from "../types/symbols";
@@ -39,7 +41,7 @@ export interface IndexedOperations {
3941
export interface ChangeSet {
4042
// field index -> operation index
4143
indexes: { [index: number]: number };
42-
operations: OPERATION[]
44+
operations: number[];
4345
queueRootIndex?: number; // index of ChangeTree structure in `root.changes` or `root.filteredChanges`
4446
}
4547

@@ -56,14 +58,43 @@ export function setOperationAtIndex(changeSet: ChangeSet, index: number) {
5658
}
5759
}
5860

59-
export function deleteOperationAtIndex(changeSet: ChangeSet, index: number) {
60-
const operationsIndex = changeSet.indexes[index];
61-
if (operationsIndex !== undefined) {
62-
changeSet.operations[operationsIndex] = undefined;
61+
export function deleteOperationAtIndex(changeSet: ChangeSet, index: number | string) {
62+
let operationsIndex = changeSet.indexes[index];
63+
if (operationsIndex === undefined) {
64+
//
65+
// if index is not found, we need to find the last operation
66+
// FIXME: this is not very efficient
67+
//
68+
// > See "should allow consecutive splices (same place)" tests
69+
//
70+
operationsIndex = Object.values(changeSet.indexes).at(-1);
71+
index = Object.entries(changeSet.indexes).find(([_, value]) => value === operationsIndex)?.[0];
6372
}
73+
changeSet.operations[operationsIndex] = undefined;
6474
delete changeSet.indexes[index];
6575
}
6676

77+
export function debugChangeSet(label: string, changeSet: ChangeSet) {
78+
let indexes: string[] = [];
79+
let operations: string[] = [];
80+
81+
for (const index in changeSet.indexes) {
82+
indexes.push(`\t${util.inspect(index, { colors: true })} => [${util.inspect(changeSet.indexes[index], { colors: true })}]`);
83+
}
84+
85+
for (let i = 0; i < changeSet.operations.length; i++) {
86+
const index = changeSet.operations[i];
87+
if (index !== undefined) {
88+
operations.push(`\t[${util.inspect(i, { colors: true })}] => ${util.inspect(index, { colors: true })}`);
89+
}
90+
}
91+
92+
console.log(`${label} =>\nindexes (${Object.keys(changeSet.indexes).length}) {`);
93+
console.log(indexes.join("\n"), "\n}");
94+
console.log(`operations (${changeSet.operations.filter(op => op !== undefined).length}) {`);
95+
console.log(operations.join("\n"), "\n}");
96+
}
97+
6798
export function enqueueChangeTree(
6899
root: Root,
69100
changeTree: ChangeTree,
@@ -292,15 +323,24 @@ export class ChangeTree<T extends Ref=any> {
292323

293324
private _shiftAllChangeIndexes(shiftIndex: number, startIndex: number = 0, changeSet: ChangeSet) {
294325
const newIndexes = {};
295-
326+
let newKey = 0;
296327
for (const key in changeSet.indexes) {
297-
const index = changeSet.indexes[key];
298-
if (index > startIndex) {
299-
newIndexes[Number(key) + shiftIndex] = index;
300-
} else {
301-
newIndexes[key] = index;
302-
}
328+
newIndexes[newKey++] = changeSet.indexes[key];
303329
}
330+
331+
// const newIndexes = {};
332+
// let newKey = 0;
333+
// for (const key in changeSet.indexes) {
334+
// const index = changeSet.indexes[key];
335+
// newIndexes[newKey++] = changeSet.indexes[key];
336+
// console.log("...shiftAllChangeIndexes", { index: key, targetIndex: index, startIndex, shiftIndex });
337+
// if (index > startIndex) {
338+
// newIndexes[Number(key) + shiftIndex] = index;
339+
// } else {
340+
// newIndexes[Number(key)] = index;
341+
// }
342+
// }
343+
304344
changeSet.indexes = newIndexes;
305345

306346
for (let i = 0; i < changeSet.operations.length; i++) {

‎src/encoder/EncodeOperation.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,8 @@ export const encodeArray: EncodeOperation = function (
217217
const type = changeTree.getType(field);
218218
const value = changeTree.getValue(field, isEncodeAll);
219219

220+
// console.log({ type, field, value });
221+
220222
// console.log("encodeArray -> ", {
221223
// ref: changeTree.ref.constructor.name,
222224
// field,

‎src/encoder/Encoder.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Root } from "./Root";
1010

1111
import type { StateView } from "./StateView";
1212
import type { Metadata } from "../Metadata";
13-
import type { ChangeTree } from "./ChangeTree";
13+
import type { ChangeSetName, ChangeTree } from "./ChangeTree";
1414

1515
export class Encoder<T extends Schema = any> {
1616
static BUFFER_SIZE = (typeof(Buffer) !== "undefined") && Buffer.poolSize || 8 * 1024; // 8KB
@@ -48,7 +48,7 @@ export class Encoder<T extends Schema = any> {
4848
it: Iterator = { offset: 0 },
4949
view?: StateView,
5050
buffer = this.sharedBuffer,
51-
changeSetName: "changes" | "allChanges" | "filteredChanges" | "allFilteredChanges" = "changes",
51+
changeSetName: ChangeSetName = "changes",
5252
isEncodeAll = changeSetName === "allChanges",
5353
initialOffset = it.offset // cache current offset in case we need to resize the buffer
5454
): Buffer {
@@ -70,11 +70,11 @@ export class Encoder<T extends Schema = any> {
7070
}
7171
}
7272

73-
const operations = changeTree[changeSetName];
73+
const changeSet = changeTree[changeSetName];
7474
const ref = changeTree.ref;
7575

7676
// TODO: avoid iterating over change tree if no changes were made
77-
const numChanges = operations.operations.length;
77+
const numChanges = changeSet.operations.length;
7878
if (numChanges === 0) { continue; }
7979

8080
const ctor = ref.constructor;
@@ -90,7 +90,7 @@ export class Encoder<T extends Schema = any> {
9090
}
9191

9292
for (let j = 0; j < numChanges; j++) {
93-
const fieldIndex = operations.operations[j];
93+
const fieldIndex = changeSet.operations[j];
9494

9595
const operation = (fieldIndex < 0)
9696
? Math.abs(fieldIndex) // "pure" operation without fieldIndex (e.g. CLEAR, REVERSE, etc.)

‎src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export { getRawChangesCallback } from "./decoder/strategy/RawChanges";
5050

5151
export { Encoder } from "./encoder/Encoder";
5252
export { encodeSchemaOperation, encodeArray, encodeKeyValueOperation } from "./encoder/EncodeOperation";
53-
export { ChangeTree, Ref } from "./encoder/ChangeTree";
53+
export { ChangeTree, Ref, type ChangeSetName, type ChangeSet} from "./encoder/ChangeTree";
5454
export { StateView } from "./encoder/StateView";
5555

5656
export { Decoder } from "./decoder/Decoder";

‎src/types/custom/ArraySchema.ts

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { $changes, $childType, $decoder, $deleteByIndex, $onEncodeEnd, $encoder, $filter, $getByIndex, $onDecodeEnd } from "../symbols";
22
import type { Schema } from "../../Schema";
3-
import { ChangeTree, setOperationAtIndex } from "../../encoder/ChangeTree";
3+
import { ChangeTree, debugChangeSet, deleteOperationAtIndex, enqueueChangeTree, setOperationAtIndex } from "../../encoder/ChangeTree";
44
import { OPERATION } from "../../encoding/spec";
55
import { registerType } from "../registry";
66
import { Collection } from "../HelperTypes";
@@ -229,9 +229,6 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
229229

230230
this[$changes].delete(index, undefined, this.items.length - 1);
231231

232-
// this.tmpItems[index] = undefined;
233-
// this.tmpItems.pop();
234-
235232
this.deletedIndexes[index] = true;
236233

237234
return this.items.pop();
@@ -412,38 +409,63 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
412409
*/
413410
splice(
414411
start: number,
415-
deleteCount: number = this.items.length - start,
412+
deleteCount?: number,
416413
...insertItems: V[]
417414
): V[] {
418415
const changeTree = this[$changes];
419416

417+
const itemsLength = this.items.length;
420418
const tmpItemsLength = this.tmpItems.length;
421419
const insertCount = insertItems.length;
422420

423421
// build up-to-date list of indexes, excluding removed values.
424422
const indexes: number[] = [];
425423
for (let i = 0; i < tmpItemsLength; i++) {
426-
// if (this.tmpItems[i] !== undefined) {
427424
if (this.deletedIndexes[i] !== true) {
428425
indexes.push(i);
429426
}
430427
}
431428

432-
// delete operations at correct index
433-
for (let i = start; i < start + deleteCount; i++) {
434-
const index = indexes[i];
435-
changeTree.delete(index);
436-
// this.tmpItems[index] = undefined;
437-
this.deletedIndexes[index] = true;
429+
if (itemsLength > start) {
430+
// if deleteCount is not provided, delete all items from start to end
431+
if (deleteCount === undefined) {
432+
deleteCount = itemsLength - start;
433+
}
434+
435+
//
436+
// delete operations at correct index
437+
//
438+
for (let i = start; i < start + deleteCount; i++) {
439+
const index = indexes[i];
440+
changeTree.delete(index, OPERATION.DELETE);
441+
this.deletedIndexes[index] = true;
442+
}
443+
444+
} else {
445+
// not enough items to delete
446+
deleteCount = 0;
438447
}
439448

440-
// force insert operations
441-
for (let i = 0; i < insertCount; i++) {
442-
const addIndex = indexes[start] + i;
443-
changeTree.indexedOperation(addIndex, OPERATION.ADD);
449+
// insert operations
450+
if (insertCount > 0) {
451+
if (insertCount > deleteCount) {
452+
console.error("Inserting more elements than deleting during ArraySchema#splice()");
453+
throw new Error("ArraySchema#splice(): insertCount must be equal or lower than deleteCount.");
454+
}
455+
456+
for (let i = 0; i < insertCount; i++) {
457+
const addIndex = (indexes[start] ?? itemsLength) + i;
458+
459+
changeTree.indexedOperation(
460+
addIndex,
461+
(this.deletedIndexes[addIndex])
462+
? OPERATION.DELETE_AND_ADD
463+
: OPERATION.ADD
464+
);
444465

445-
// set value's parent/root
446-
insertItems[i][$changes]?.setParent(this, changeTree.root, addIndex);
466+
// set value's parent/root
467+
insertItems[i][$changes]?.setParent(this, changeTree.root, addIndex);
468+
}
447469
}
448470

449471
//
@@ -452,6 +474,17 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
452474
//
453475
if (deleteCount > insertCount) {
454476
changeTree.shiftAllChangeIndexes(-(deleteCount - insertCount), indexes[start + insertCount]);
477+
// debugChangeSet("AFTER SHIFT indexes", changeTree.allChanges);
478+
}
479+
480+
//
481+
// FIXME: this code block is duplicated on ChangeTree
482+
//
483+
if (changeTree.filteredChanges !== undefined) {
484+
enqueueChangeTree(changeTree.root, changeTree, 'filteredChanges');
485+
486+
} else {
487+
enqueueChangeTree(changeTree.root, changeTree, 'changes');
455488
}
456489

457490
return this.items.splice(start, deleteCount, ...insertItems);
@@ -767,10 +800,6 @@ export class ArraySchema<V = any> implements Array<V>, Collection<number, V> {
767800
: this.deletedIndexes[index]
768801
? this.items[index]
769802
: this.tmpItems[index] || this.items[index];
770-
771-
// return (isEncodeAll)
772-
// ? this.items[index]
773-
// : this.tmpItems[index] ?? this.items[index];
774803
}
775804

776805
protected [$deleteByIndex](index: number) {

0 commit comments

Comments
 (0)