Skip to content

Commit 273baf9

Browse files
endelendel
committed
Reflection: fix encode order - extending types should come after base type.
1 parent 20943a8 commit 273baf9

File tree

3 files changed

+138
-48
lines changed

3 files changed

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

‎src/Reflection.ts

Lines changed: 79 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -44,71 +44,104 @@ export class Reflection extends Schema {
4444
const rootType = context.schemas.get(encoder.state.constructor);
4545
if (rootType > 0) { reflection.rootType = rootType; }
4646

47-
const buildType = (currentType: ReflectionType, metadata: Metadata) => {
48-
for (const fieldIndex in metadata) {
49-
const index = Number(fieldIndex);
50-
const fieldName = metadata[index].name;
51-
52-
// skip fields from parent classes
53-
if (!Object.prototype.hasOwnProperty.call(metadata, fieldName)) {
54-
continue;
47+
const includedTypeIds = new Set<number>();
48+
const pendingReflectionTypes: { [typeid: number]: ReflectionType[] } = {};
49+
50+
// add type to reflection in a way that respects inheritance
51+
// (parent types should be added before their children)
52+
const addType = (type: ReflectionType) => {
53+
if (type.extendsId === undefined || includedTypeIds.has(type.extendsId)) {
54+
includedTypeIds.add(type.id);
55+
56+
reflection.types.push(type);
57+
58+
const deps = pendingReflectionTypes[type.id];
59+
if (deps !== undefined) {
60+
delete pendingReflectionTypes[type.id];
61+
deps.forEach((childType) => addType(childType));
5562
}
63+
} else {
64+
if (pendingReflectionTypes[type.extendsId] === undefined) {
65+
pendingReflectionTypes[type.extendsId] = [];
66+
}
67+
pendingReflectionTypes[type.extendsId].push(type);
68+
}
69+
};
5670

57-
const field = new ReflectionField();
58-
field.name = fieldName;
71+
context.schemas.forEach((typeid, klass) => {
72+
const type = new ReflectionType();
73+
type.id = Number(typeid);
5974

60-
let fieldType: string;
75+
// support inheritance
76+
const inheritFrom = Object.getPrototypeOf(klass);
77+
if (inheritFrom !== Schema) {
78+
type.extendsId = context.schemas.get(inheritFrom);
79+
}
6180

62-
const type = metadata[index].type;
81+
const metadata = klass[Symbol.metadata];
6382

64-
if (typeof (type) === "string") {
65-
fieldType = type;
83+
//
84+
// FIXME: this is a workaround for inherited types without additional fields
85+
// if metadata is the same reference as the parent class - it means the class has no own metadata
86+
//
87+
if (metadata !== inheritFrom[Symbol.metadata]) {
88+
for (const fieldIndex in metadata) {
89+
const index = Number(fieldIndex);
90+
const fieldName = metadata[index].name;
6691

67-
} else {
68-
let childTypeSchema: typeof Schema;
92+
// skip fields from parent classes
93+
if (!Object.prototype.hasOwnProperty.call(metadata, fieldName)) {
94+
continue;
95+
}
96+
97+
const reflectionField = new ReflectionField();
98+
reflectionField.name = fieldName;
99+
100+
let fieldType: string;
69101

70-
//
71-
// TODO: refactor below.
72-
//
73-
if (Schema.is(type)) {
74-
fieldType = "ref";
75-
childTypeSchema = type as typeof Schema;
102+
const field = metadata[index];
103+
104+
if (typeof (field.type) === "string") {
105+
fieldType = field.type;
76106

77107
} else {
78-
fieldType = Object.keys(type)[0];
108+
let childTypeSchema: typeof Schema;
79109

80-
if (typeof(type[fieldType]) === "string") {
81-
fieldType += ":" + type[fieldType]; // array:string
110+
//
111+
// TODO: refactor below.
112+
//
113+
if (Schema.is(field.type)) {
114+
fieldType = "ref";
115+
childTypeSchema = field.type as typeof Schema;
82116

83117
} else {
84-
childTypeSchema = type[fieldType];
118+
fieldType = Object.keys(field.type)[0];
119+
120+
if (typeof (field.type[fieldType]) === "string") {
121+
fieldType += ":" + field.type[fieldType]; // array:string
122+
123+
} else {
124+
childTypeSchema = field.type[fieldType];
125+
}
85126
}
127+
128+
reflectionField.referencedType = (childTypeSchema)
129+
? context.getTypeId(childTypeSchema)
130+
: -1;
86131
}
87132

88-
field.referencedType = (childTypeSchema)
89-
? context.getTypeId(childTypeSchema)
90-
: -1;
133+
reflectionField.type = fieldType;
134+
type.fields.push(reflectionField);
91135
}
92-
93-
field.type = fieldType;
94-
currentType.fields.push(field);
95136
}
96137

97-
reflection.types.push(currentType);
98-
}
99-
100-
for (let typeid in context.types) {
101-
const klass = context.types[typeid];
102-
const type = new ReflectionType();
103-
type.id = Number(typeid);
104-
105-
// support inheritance
106-
const inheritFrom = Object.getPrototypeOf(klass);
107-
if (inheritFrom !== Schema) {
108-
type.extendsId = context.schemas.get(inheritFrom);
109-
}
138+
addType(type);
139+
});
110140

111-
buildType(type, klass[Symbol.metadata]);
141+
// in case there are types that were not added due to inheritance
142+
for (const typeid in pendingReflectionTypes) {
143+
pendingReflectionTypes[typeid].forEach((type) =>
144+
reflection.types.push(type))
112145
}
113146

114147
const buf = reflectionEncoder.encodeAll(it);

‎test/Reflection.test.ts

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as util from "util";
22
import * as assert from "assert";
3-
import { Reflection, Schema, type, MapSchema, ArraySchema, $changes, TypeContext } from "../src";
3+
import { Reflection, Schema, type, MapSchema, ArraySchema, $changes, TypeContext, Decoder, entity } from "../src";
44
import { createInstanceFromReflection, getEncoder } from "./Schema";
55

66
/**
@@ -265,4 +265,61 @@ describe("Reflection", () => {
265265
assert.deepStrictEqual(state.toJSON(), reflected3.toJSON());
266266
assert.deepStrictEqual(state.toJSON(), reflected4.toJSON());
267267
});
268+
269+
it("order of decoded types must follow inheritance order", () => {
270+
class Vector2D extends Schema {
271+
@type("number") x: number;
272+
@type("number") y: number;
273+
}
274+
class Health extends Schema {
275+
@type("number") max: number;
276+
@type("number") current: number;
277+
}
278+
class Entity extends Schema {
279+
@type("string") uuid: string;
280+
@type(Vector2D) position = new Vector2D();
281+
}
282+
class DynamicEntity extends Entity {
283+
@type(Health) health = new Health();
284+
}
285+
class Player extends DynamicEntity {
286+
@type("string") name: string;
287+
}
288+
289+
@entity class SlimeEntity extends DynamicEntity { }
290+
291+
abstract class EntitiesMap<T extends Entity = Entity> extends Schema {
292+
@type({ map: Entity }) entities = new MapSchema<T>();
293+
}
294+
295+
class StaticEntitiesMap extends EntitiesMap {}
296+
class DynamicEntitiesMap<T extends DynamicEntity = DynamicEntity> extends EntitiesMap<T> {}
297+
class PlayerMap extends DynamicEntitiesMap<Player> {}
298+
299+
class State extends Schema {
300+
@type(StaticEntitiesMap) staticEntities = new StaticEntitiesMap();
301+
@type(DynamicEntitiesMap) dynamicEntities = new DynamicEntitiesMap();
302+
@type(PlayerMap) players = new PlayerMap();
303+
}
304+
305+
const state = new State();
306+
const encoded = Reflection.encode(getEncoder(state));
307+
308+
const reflected = new Reflection();
309+
const decoder = new Decoder(reflected);
310+
decoder.decode(encoded);
311+
312+
const types = reflected.types.toArray();
313+
assert.strictEqual(11, types.length)
314+
315+
const addedTypeIds = new Set<number>();
316+
317+
types.forEach((type) => {
318+
addedTypeIds.add(type.id);
319+
320+
if (type.extendsId !== undefined) {
321+
assert.ok(addedTypeIds.has(type.extendsId), "Base type must be added before its children");
322+
}
323+
});
324+
});
268325
});

0 commit comments

Comments
 (0)