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 client API for interacting with rooms, user lists, etc #40

Closed
incompl opened this issue Oct 30, 2013 · 12 comments
Closed

Remove client API for interacting with rooms, user lists, etc #40

incompl opened this issue Oct 30, 2013 · 12 comments
Labels

Comments

@incompl
Copy link
Owner

incompl commented Oct 30, 2013

The following client methods should be removed:

  • listRooms
  • joinLobby
  • joinRoom
  • getRoomMembers
  • leaveRoom
  • listUsers
  • registerUsername

All of these are things that clients should not necessarily be given free reign to do in all games. They make assumptions about what is allowed in the game. For example, a game may have private rooms, so a client listRooms method would not be appropriate.

Right now rooms have shouldAllowUser to deal with this problem, but this is not a sustainable approach. This type of logic should go in a custom message handler rather than cluttering the config with new settings.

I realize this makes getting started with Cloak a little harder, but it greatly increases the general usefulness of the library. Most of these methods could be implemented as a message handlers within a handful of lines of code.

The following client functions are not effected.

  • configure
  • run
  • end
  • connected
  • message

Since this is not a backward-compatible change this will likely go in a 1.0.0 release.

Interested in your thoughts @dariusk @Cheeseen. Especially since some of these methods came from your work on Grow21, @dariusk .

@dariusk
Copy link
Collaborator

dariusk commented Oct 31, 2013

I don't know. The thing I like best about Cloak is that it's ready-to-go for a basic setup and then you can remove that basic functionality if you want.

That said: I still want to make my grunt-init template, and this could be a good place to put all this stuff! You'd run

$ grunt-init cloak-scaffold

and you'd be asked a bunch of questions like:

  • Do you want the client to be able to list all rooms?
  • Do you want the client to able to join a specific room?
  • etc etc

Then it would generate a client.js and a server.js with the basic custom messages required for this.

@incompl
Copy link
Owner Author

incompl commented Oct 31, 2013

The problem is that you can't remove the basic functionality you want.

Your idea for the scaffolding is the perfect solution.

On Wed, Oct 30, 2013 at 8:13 PM, dariusk notifications@github.com wrote:

I don't know. The thing I like best about Cloak is that it's ready-to-go
for a basic setup and then you can remove that basic functionality if you
want.

That said: I still want to make my grunt-init template, and this could be
a good place to put all this stuff! You'd run

$ grunt-init cloak-scaffold

and you'd be asked a bunch of questions like:

  • Do you want the client to be able to list all rooms?
  • Do you want the client to able to join a specific room?
  • etc etc

Then it would generate a client.js and a server.js with the basic custom
messages required for this.


Reply to this email directly or view it on GitHubhttps://github.com//issues/40#issuecomment-27451430
.

@incompl
Copy link
Owner Author

incompl commented Oct 31, 2013

Also, we will want to optimize the server API to make it easy to make messages for common tasks.

@aychtang
Copy link
Collaborator

It's a shame we have to sacrifice ease of use but on balance I agree with removing official public client API methods.

I think that a good scaffolding system would help with that and maybe we should start collecting commonly found patterns to be put into a snippets/pattern collection for cloak.

What kind of time-frame are you planning for the road to 1.0? Should I hold off a full tutorial until 1.0?

@dariusk
Copy link
Collaborator

dariusk commented Oct 31, 2013

I had another idea this morning. What if we just provide switches on the server config to disregard the "core" client messages on a message by message basis? It's not elegant but I'm also really, really uncomfortable telling developers, "If you want a full 50% of the features we promise you need to install this new repo and this thing called grunt-init and set up your ~/.grunt-init directory and answer a bunch of questions."

@incompl
Copy link
Owner Author

incompl commented Oct 31, 2013

We'll make sure the server retains all these features. Effectively implementing any of them as a pass-through to the client is trivial, and that leaves you with easily extensible code rather than a black box.

Also, it seems useful to not have two places to look for features you might want.

I really think this is critical for making cloak useful for real games and not just quick prototyping.

@aychtang
Copy link
Collaborator

I guess the point is instead of providing promised features cloak becomes more of a backbone for realtime game apps. Provide better/easier ways of dealing with messaging and handling socket connections, and leave business logic as the responsibility of the user.

@incompl
Copy link
Owner Author

incompl commented Oct 31, 2013

Well it's more than that, it's just that I think the domain logic should be
on the server since it easier to customize / control it that way since it
would be accessed only through messaging.

On Thu, Oct 31, 2013 at 1:38 PM, Howard Tang notifications@github.comwrote:

I guess the point is instead of providing promised features cloak becomes
more of a backbone for realtime game apps. Provide better/easier ways of
dealing with messaging and handling socket connections, and leave business
logic as the responsibility of the user.


Reply to this email directly or view it on GitHubhttps://github.com//issues/40#issuecomment-27508115
.

@incompl
Copy link
Owner Author

incompl commented Nov 13, 2013

I created a branch to start exploring this issue. Here is a commit where I move listRooms and joinRoom from the client to server

36afc8b

This includes updating grow21 and the unit tests. I would definitely appreciate input on how this affects Cloak's usability!

incompl added a commit that referenced this issue Nov 15, 2013
incompl pushed a commit that referenced this issue Nov 25, 2013
@incompl
Copy link
Owner Author

incompl commented Nov 25, 2013

The main PR for this is merged but I'm still updating docs and will probably make further API changes. After this I will tag a 1.0.0 release (major revision number change for backward-breaking API changes).

@aychtang
Copy link
Collaborator

Ok, building a pong clone with cloak now so will check out the changes and move toward using the new API.

@incompl
Copy link
Owner Author

incompl commented Nov 25, 2013

Calling this done, after everything is tidied up more I will tag a 1.0.0 so this can hit npm

@incompl incompl closed this as completed Nov 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants