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

refactoring of terminator strings; http endpoint to list clients per channel #34

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

JosePedroDiasSky
Copy link

@JosePedroDiasSky JosePedroDiasSky commented Jun 16, 2020

I felt the need to list number of clients per channel in noobhub to provide some sort of lobby feature in game. I initially spiked it as messages in the protocol. I got it to work, but found that approach would:

  • introduce a super basic action schema in the payloads (unlike now where it's freeform)
  • kinda breaks the overall client in a channel at a time idea, as this command would be god mode.

Attempted this very simple satellite HTTP endpoint instead. It asks for totals from TCP sockets (or compounds both TCP and WS). While the bootstrap/hooks logic in node.js isn't particularly readable, there's a clear separation of concerns and this way both features remain optional, in the spirit of the initial PR. No breakage with original project as well.

I suggest you look at this PR one commit at a time: the first one refactors terminator strings (more lines changed but linear to analyze in isolation I hope), the second one adds the http endpoint and hook counting function on both existing protocols to report back to it.

Tell me if you:

  • find this useless (and remains happily in my fork)
  • have had this necessity before and have an alternative design to suggest

Cheers!

@JosePedroDiasSky JosePedroDiasSky changed the title http endpoint to list clients per channel; support for ws bridge to be accounted for in the feature refactoring of terminator strings; http endpoint to list clients per channel Jun 16, 2020
@Overtorment
Copy link
Owner

typically, to find how many people are in lobby you just post a message to lobby, something like WHO_WANTS_TO_PLAY and see who responds. this way you'll get only alive clients who are not just connected, but also respond

@Overtorment Overtorment requested a review from dr3am3r June 16, 2020 10:50
@JosePedroDiasSky
Copy link
Author

But how do you address every channel? Or you make everyone go to a looby channel?
The scenario I'm thinking is joining an ongoing game, I guess.

@Overtorment
Copy link
Owner

yep, there should be a dedicated lobby channel that is called, you know, lobby

@dr3am3r
Copy link
Collaborator

dr3am3r commented Jun 22, 2020

looks good, please fix conflicts

@JosePedroDias
Copy link
Contributor

@dr3am3r but I failed to confirm whether you're interested in applying the 2 commits and their features. Are you?

@dr3am3r
Copy link
Collaborator

dr3am3r commented Jun 22, 2020

well, I would approach getting number of people in the room by extending the messaging protocol, not by creating a new endpoint. Also, regarding protocol configuration, it should be separated from the codebase with some sort of .json file and shared between all services, which use it.

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

Successfully merging this pull request may close these issues.

5 participants