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

Remove Reporter/Message scheme #44

Merged
merged 18 commits into from May 8, 2023
Merged

Remove Reporter/Message scheme #44

merged 18 commits into from May 8, 2023

Conversation

mvanderkamp
Copy link
Contributor

I recall adding this in order to avoid some pitfalls that I was hitting early in development. It's a good idea to serialize, but this approach was over-engineered and made the code much more confusing than it needed to be, and was also ultimately limiting.

I've replaced it with simple toJSON methods on the base classes. Socket IO will call these methods automatically when it finds them.

Closes #16

Comment on lines -25 to +26
"socket.io": "^4.6.1",
"socket.io-client": "^4.6.1",
"socket.io": "^4.2.0",
"socket.io-client": "^4.2.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I downgrade socket.io back to 4.2.0 so that we can get debug messages in the browser if we want. See socketio/socket.io-client#1516

@mvanderkamp mvanderkamp merged commit b27df7d into main May 8, 2023
1 check passed
@mvanderkamp mvanderkamp deleted the remove-reporters-scheme branch May 8, 2023 06:25
@mvanderkamp mvanderkamp self-assigned this Jun 2, 2023
@mvanderkamp mvanderkamp added this to the Camera Ready milestone Jun 2, 2023
@mvanderkamp mvanderkamp added client Affects client deliverables server Affects server code cleanup Refactor or general cleanup that doesn't change behaviour but makes code easier to read and maintain labels Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Refactor or general cleanup that doesn't change behaviour but makes code easier to read and maintain client Affects client deliverables server Affects server code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor "Reporters" scheme to use decoupled serializers or simpler "toJson" approach
1 participant