Skip to content

Commit a2f8790

Browse files
committed
avoid calling .removeRef() on items already flagged for garbage collection.
1 parent 18d0b3b commit a2f8790

File tree

4 files changed

+45
-6
lines changed

4 files changed

+45
-6
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.17",
3+
"version": "3.0.18",
44
"description": "Binary state serializer with delta encoding for games",
55
"bin": {
66
"schema-codegen": "./bin/schema-codegen",

src/decoder/ReferenceTracker.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,20 @@ export class ReferenceTracker {
103103
for (const index in metadata) {
104104
const field = metadata[index as any as number].name;
105105
const childRefId = typeof(ref[field]) === "object" && this.refIds.get(ref[field]);
106-
if (childRefId) {
106+
if (childRefId && !this.deletedRefs.has(childRefId)) {
107107
this.removeRef(childRefId);
108108
}
109109
}
110110

111111
} else {
112112
if (typeof (Object.values(ref[$childType])[0]) === "function") {
113113
Array.from((ref as MapSchema).values())
114-
.forEach((child) => this.removeRef(this.refIds.get(child)));
114+
.forEach((child) => {
115+
const childRefId = this.refIds.get(child);
116+
if (!this.deletedRefs.has(childRefId)) {
117+
this.removeRef(childRefId);
118+
}
119+
});
115120
}
116121
}
117122

test/MapSchema.test.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as assert from "assert";
22

3-
import { State, Player, getCallbacks, createInstanceFromReflection, getDecoder, getEncoder, assertDeepStrictEqualEncodeAll } from "./Schema";
4-
import { MapSchema, type, Schema, ArraySchema, Reflection, $changes } from "../src";
3+
import { State, Player, getCallbacks, createInstanceFromReflection, getDecoder, getEncoder, assertDeepStrictEqualEncodeAll, assertRefIdCounts } from "./Schema";
4+
import { MapSchema, type, Schema, ArraySchema, Reflection, $changes, SetSchema, entity } from "../src";
55
import { nanoid } from "nanoid";
66

77
describe("Type: MapSchema", () => {
@@ -727,6 +727,40 @@ describe("Type: MapSchema", () => {
727727
assert.deepStrictEqual(map.toJSON(), previousMap.toJSON());
728728
});
729729

730+
it("should trigger warning: 'trying to remove refId with 0 refCount'", () => {
731+
enum Synergy { NORMAL = "NORMAL", GRASS = "GRASS", FIRE = "FIRE", WATER = "WATER", ELECTRIC = "ELECTRIC", FIGHTING = "FIGHTING", }
732+
class Pokemon extends Schema {
733+
@type("string") name: string;
734+
@type({ set: "string" }) types = new SetSchema<Synergy>();
735+
}
736+
@entity
737+
class Pikachu extends Pokemon {
738+
name = "Pikachu";
739+
types = new SetSchema<Synergy>([Synergy.ELECTRIC]);
740+
}
741+
class Player extends Schema {
742+
@type({ map: Pokemon }) board = new MapSchema<Pokemon>();
743+
}
744+
class State extends Schema {
745+
@type({ map: Player }) players = new MapSchema<Player>();
746+
}
747+
748+
const state = new State();
749+
const decodedState = createInstanceFromReflection(state);
750+
751+
const player = new Player();
752+
state.players.set("one", player);
753+
player.board.set("1", new Pikachu());
754+
755+
decodedState.decode(state.encode());
756+
757+
player.board.delete("1");
758+
decodedState.decode(state.encode());
759+
760+
assertRefIdCounts(state, decodedState);
761+
assertDeepStrictEqualEncodeAll(state);
762+
});
763+
730764
xit("move instance between keys in the same patch", () => {
731765
//
732766
// TODO: test $onEncodeEnd of MapSchema. Is $onEncodeEnd of MapSchema even necessary?

test/Schema.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function assertRefIdCounts(source: Schema, target: Schema) {
4848
for (const refId in encoder.root.refCount) {
4949
const encoderRefCount = encoder.root.refCount[refId];
5050
const decoderRefCount = decoder.root.refCounts[refId] ?? 0;
51-
assert.strictEqual(encoderRefCount, decoderRefCount, `refCount mismatch for ${refId}`);
51+
assert.strictEqual(encoderRefCount, decoderRefCount, `refCount mismatch for refId: ${refId}`);
5252
}
5353
}
5454

0 commit comments

Comments
 (0)