-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix: RPC events sync. #67
Conversation
- RPC events name sync between players. - Made Players dictionary private #66
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either my eyes are deceiving me, or I can't see the private Players
change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add description comments
- Write dry runs on all cases in mind (no need to actually build and run)
I have a hunch this methodology might cause Race conditions upon high load. Needs testing on high performance calls between multiple players, with multiple RPCs. Instead of saving a state, maybe send some metadata alongside each RPC call? That way, it's guaranteed to reach along with the RPC call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What scenarios has this been tested on?
Merging in favor of a working solution instead of over engineering. Noted down problems in Refactor Umbrella issue. |
Temporary fix, RPC part needs to be refactored, might be better to handle within JS. #40