Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MapSchema string field undefined set error #143

Open
NoahHahm opened this issue Nov 2, 2022 · 3 comments
Open

MapSchema string field undefined set error #143

NoahHahm opened this issue Nov 2, 2022 · 3 comments

Comments

@NoahHahm
Copy link

NoahHahm commented Nov 2, 2022

An error occurs when inserting undefined into a string field in MapSchema.
There was no problem until 1.0.38, but after the latest update, a refId error occurred in the client;
If you simply insert undefined into the string field in the Schema Object when using MapSchema, an error occurs.

items: MapSchema<Expedition>;

class Expedition {
  pet1: string;
}

expedition.pet1 =  undefined;

items.set('test', expedition);
@NoahHahm NoahHahm changed the title MapSchema string field bug MapSchema string field undefined set error Nov 2, 2022
@NoahHahm
Copy link
Author

NoahHahm commented Nov 7, 2022

Error: a 'string' was expected, but 'undefined' was provided in current#pet1
    at new EncodeSchemaError (/usr/src/app/node_modules/@colyseus/schema/build/cjs/index.js:2232:42)
    at assertType (/usr/src/app/node_modules/@colyseus/schema/build/cjs/index.js:2266:15)
    at encodePrimitiveType (/usr/src/app/node_modules/@colyseus/schema/build/cjs/index.js:2275:5)
    at current.Schema.encode (/usr/src/app/node_modules/@colyseus/schema/build/cjs/index.js:2686:21)
    at SchemaSerializer.applyPatches (/usr/src/app/lib/applications/colyseus/Serializer.js:41:40)

@endel are there any recent undefined issues?
there were no problems until version 1.0.38.

@hunkydoryrepair
Copy link

this seems related to our issue. undefined gets changed to {} This test fails

    test('set schema and change', () => {
        class OptionalSubScheam extends Schema {
            constructor() {
                super();
                this.index = 200;
                this.my_string = 'a good string';
            }
            @type('number')  index: number;
            @type('string')  my_string: string;
        }

        class Test extends Schema {
            constructor() {
                super();
                this.size = 0;
            }
            @type('number') size: number; // total number of storage slots in this container.
                    
            @type('boolean') transient?: boolean;
          
            @type(OptionalSubScheam) sub?: OptionalSubScheam;
        }
        
        const testobj = new Test();
        const encoded = testobj.encodeAll(false);
        const handshake = Reflection.encode(testobj);


        // serializer from client libs
        const serializer = new SchemaSerializer<Test>();
        serializer.handshake(handshake);
        serializer.setState(encoded);

        const clientobj = serializer.getState();
        expect(clientobj.sub).toBeUndefined();
    });

@endel
Copy link
Member

endel commented Jul 13, 2023

Hi @hunkydoryrepair, your scenario is an intended behavior, in order to simplify listening on direct references on the root structure, such as: room.state.sub.listen("index", () => {}).

When structures are "reflected" in the client, every non-primitive structure in the root level is auto-initialized, see:

schema/src/Reflection.ts

Lines 143 to 155 in 185e332

/**
* auto-initialize referenced types on root type
* to allow registering listeners immediatelly on client-side
*/
for (let fieldName in rootType._definition.schema) {
const fieldType = rootType._definition.schema[fieldName];
if (typeof(fieldType) !== "string") {
rootInstance[fieldName] = (typeof (fieldType) === "function")
? new (fieldType as any)() // is a schema reference
: new (getType(Object.keys(fieldType)[0])).constructor(); // is a "collection"
}
}

I agree that at the data level, it would be more intuitive if it starts as undefined - the problem is we don't have an API for listening to direct schema "additions" like this. I'm open to a proposal here.


@NoahHahm your case seems different but the reproduction scenario seems incomplete. I'm not sure how to reproduce it. Could you provide a full test scenario like @hunkydoryrepair did? Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants