Skip to content

Server does not sync state with clients on the latest version #510

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

Closed
jog1t opened this issue Mar 9, 2022 · 10 comments
Closed

Server does not sync state with clients on the latest version #510

jog1t opened this issue Mar 9, 2022 · 10 comments
Labels
🧮 @colyseus/schema Problems related to Schema Serializer 💅 enhancement
Milestone

Comments

@jog1t
Copy link

jog1t commented Mar 9, 2022

Steps to Reproduce

Example CodesSandbox: https://codesandbox.io/s/dazzling-mountain-hwzfxh

  1. Open CodeSandbox
  2. Preview of HTML shows client's state (check console for more details).
  3. Client sends ping message to server.
  4. Server every 1s modifies the state.

Context

Server receives ping messages, but client is not updated with the latest state. Checked with DevTools, server is not sending any message (sends only two at the beginning, I think they are schema types related).

Adding @colyseus/monitor, and inspecting room's state also shows empty state.

Your Environment

@jog1t jog1t changed the title Bug: Server does not sync state with clients on the latest version Server does not sync state with clients on the latest version Mar 9, 2022
@endel
Copy link
Member

endel commented Mar 9, 2022

Hi @jog1t,

Interesting find. It seems that defineTypes() and defining plain JavaScript properties directly don't play nicely together:

class State extends Schema {
  number = 0;
  property = "";
}
defineTypes(State, {
  property: "string",
  number: "number"
}); 

I've just tried commenting out the property initializer and then your example works: (See sandbox link)

class State extends Schema {
  // number = 0;
  // property = "";
}
defineTypes(State, {
  property: "string",
  number: "number"
}); 

For TypeScript this is not an issue. We need to investigate why this is happening, if you have any clues, I highly appreciate it!

@endel endel added the 🧮 @colyseus/schema Problems related to Schema Serializer label Mar 9, 2022
@endel endel added this to the 0.15 milestone Mar 9, 2022
@jog1t
Copy link
Author

jog1t commented Mar 9, 2022

@endel thanks for quick reply! 🙌🏻 Didn't have too much time today to look properly at this, but it looks like getters/setters are not set properly for properties with initial values. Will try to look at it in the following days.

@jog1t
Copy link
Author

jog1t commented Mar 13, 2022

It's looks like TypeScript's type decorator also does not work 😢. I've tried also using different versions, but without any luck.
Here's the same example from CodeSandbox, but uses ts-node or vite-node to run the server - https://github.com/jog1t/colyseus-issue-510.

🎉 What has worked for me is to define a schema class similar to this (see working-solution branch):

class State extends Schema {
  constructor() {
    super();
    this.property = "";
    this.number = 0;
  }
}

defineTypes(State, {
  property: "string",
  number: "number",
});

interface State extends Schema {
  property: string;
  number: number;
}

@WesleyClements
Copy link

WesleyClements commented Apr 8, 2022

This seems to only be a problem for TypeScript if you are compiling to ES2022/ESNext. Public instance fields are added to subclass instances using Object.defineProperty immediately after the super() call returns so the property defined in the Schema constructor is being overwritten. I don't think there is an elegant solution to fix this currently. Could move the getters and setters definition to outside the constructor but that's kind of wonky.

@devlsh
Copy link
Contributor

devlsh commented Dec 18, 2022

This seems to only be a problem for TypeScript if you are compiling to ES2022/ESNext

Just upgraded my entire stack to compile to ESM and this is the only major issue I've had so far. Would be great to have some movement on this!

@justin-m-lacy
Copy link

justin-m-lacy commented Jan 5, 2023

This seems to only be a problem for TypeScript if you are compiling to ES2022/ESNext. Public instance fields are added to subclass instances using Object.defineProperty immediately after the super() call returns so the property defined in the Schema constructor is being overwritten. I don't think there is an elegant solution to fix this currently. Could move the getters and setters definition to outside the constructor but that's kind of wonky.

If this is as you say would the solution be to abandon subclassing of schemas entirely; instead turning schemas into typed data containers? const playerSchema = new Schema<PlayerType>({ ...playerData });

It feels similar to what Pinia does for reactive data stores.

@khoakomlem
Copy link

khoakomlem commented Feb 20, 2023

It's looks like TypeScript's type decorator also does not work 😢. I've tried also using different versions, but without any luck. Here's the same example from CodeSandbox, but uses ts-node or vite-node to run the server - https://github.com/jog1t/colyseus-issue-510.

🎉 What has worked for me is to define a schema class similar to this (see working-solution branch):

class State extends Schema {
  constructor() {
    super();
    this.property = "";
    this.number = 0;
  }
}

defineTypes(State, {
  property: "string",
  number: "number",
});

interface State extends Schema {
  property: string;
  number: number;
}

Any solution for using @decorators defining schema? My current solution for this is just change target from esnext to es2018.

khoakomlem added a commit to gunsurvival/server that referenced this issue Feb 21, 2023
khoakomlem added a commit to gunsurvival/server that referenced this issue Feb 21, 2023
@endel
Copy link
Member

endel commented Apr 14, 2023

Just found a workaround for this: setting "useDefineForClassFields": false on the tsconfig.json file:

  "compilerOptions": {
    "useDefineForClassFields": false,
    // ...
  }

This setting may conflict with projects that rely on it being true (which is the new default for ES2022+ and the upcoming ECMA runtime behavior)

More info:

@endel endel modified the milestones: 0.15, 1.0 Apr 14, 2023
@SIGSTACKFAULT
Copy link

SIGSTACKFAULT commented Apr 30, 2023

works:

export class MySchema extends Schema {
    constructor(){
        super();
        this.foo = 3;
    }
    @type("int32") foo: number;
}

doesn't work:
but does work with "useDefineForClassFields": false

export class MySchema extends Schema {
    @type("int32") foo: number = 3;
    // presumably the getters and setters are clobbered?
    // at any rate the setter is never called if you do this
}

@endel
Copy link
Member

endel commented May 5, 2023

Closing this as it seems a non-issue for the moment. #510 (comment)

@endel endel closed this as completed May 5, 2023
Pkmmte added a commit to Wave-Play/robo.js that referenced this issue Apr 24, 2024
… and Colyseus

Let's be friendlier towards Colyseus, shall we?

Apparently, `useDefineForClassFields` being `true` by default prevented states from working properly. Because we intend on encouraging Colyseus usage, we should be sure our Robo Compiler defaults play well with it!

Ref: colyseus/colyseus#510
chukmunnlee added a commit to chukmunnlee/colyseus_experiments that referenced this issue Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧮 @colyseus/schema Problems related to Schema Serializer 💅 enhancement
Projects
None yet
Development

No branches or pull requests

7 participants