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

Documentation for Command Pattern is lacking in justification #59

Open
emilytocin opened this issue May 4, 2021 · 1 comment
Open

Comments

@emilytocin
Copy link

https://docs.colyseus.io/best-practices/command-pattern/

> Why?

Models (@colyseus/schema) should contain only data, without game logic.
Rooms should have as little code as possible, and forward actions to other structures

The 'Why' section does not sufficiently explain to me why I should care about this. I see no issue with my schema containing game logic, as you have not told me why that is a bad thing. I also haven't been convinced that rooms should have as little code as possible. Why? Is the code all sent to and from the clients in every patch update? That seems impossible, and easy to fix deeper in the code.

Given no real justification for why to bother with the command pattern, and no real examples showing how it improves code cleanliness, this recommendation just seems like an opinionated way to structure your code.

If these are just Colyseus best practices, they should be better justified to the people the documentation is asking to do extra work. If these are meant to be "Accepted and understood best practices for Typescript in General", please link to external documentation supporting and further explaining the concepts.

FWIW I believe that I want the command pattern, and that I should bother with it eventually, but I've read this doc five times now and am still fighting with myself over what benefit I'm getting from it. Trying to implement it from the start has just confused my Colyseus adoption. I recommend either expanding this doc to better explain the reasoning, and then give better examples. OR to remove this doc from 'best practices' and put it in a more specific 'code cleanliness / Command Pattern' section until it is complete enough to not confuse new users.

@endel
Copy link
Member

endel commented May 7, 2021

Hi @emilytocin,

Thanks for your feedback! :)

I agree with you, I also don't see a problem with having a little bit of logic inside the schemas, I often do that myself too. The problem is when it gets overused, from a maintenance perspective (code cleanliness as you mentioned)

Also, if you import a node-specific module inside a schema class, it is not going to be possible to import that schema class from the front-end, for example. (if you're using javascript/browser in the frontend; you may also not care about re-usage and that's fine too!)

The command pattern is just one way (maybe opinionated indeed, as you mentioned) to separate the "controller" (room class) from small particular features. It relates to SOLID's Single-responsibility principle.

Take brawlball.io for example, these are the commands the game has so far:

Screen Shot 2021-05-06 at 23 11 58

The GameOverCommand for example has ~80 LOC. It is a piece of functionality that can be called from either the room or other commands. When reading the sources, you don't need to scroll through the 80 LOC to understand that it is "game over" related code.

We're going to rewrite some bits of the documentation soon, will keep your feedback in mind! Thanks a lot for that!

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

2 participants