search

greggman / HappyFunTimes

A System for creating 10-100+ player local games
http://greggman.github.io/HappyFunTimes
BSD 3-Clause "New" or "Revised" License
381 stars 55 forks source link

Minor clean up and added node environment for jslint #7

Closed formula1 closed 9 years ago

formula1 commented 9 years ago

I would have liked to do however, RelayServer was hampering me from seperating the ip logic.

Additionally, I'm not exactly sure if you want to keep the event emitter private or not as that is the benefit of doing things the way your doing. However, it also adds redundant features

The Relay server itself could probably be more abstract

greggman commented 9 years ago

Thanks for this. One question, I don't understand the 

+/*jslint node: true */

stuff. I'm using eslint. Type

grunt eslint

And it's configured to use node mode for those files already so are those jslint lines needed?

formula1 commented 9 years ago

More importantly its messy and you don't need to see it. I'm using atom which is helping me lint on the fly. But I'll run eslint as well. Why not use all the linters amirite?

formula1 commented 9 years ago

For what its worth, eslint seems quite draconian. That may be a good thing though there are times such as here it wants you to split up the variable statements which is purely a personal preference issue (which may actually prefer commas for performance, [though its impact is debatable]).

That being said, there is so much linting to do T__T

greggman commented 9 years ago

eslint is not draconian, my configuration of it is :P I had forgotten to check in the files that skip checking semver etc...

But, that said, I manually merged most of your changes but I couldn't use body-parser as is. It changes the behavior. Specifically if you pass bad json it just barfs, there's no catching the error. I added 2 unit tests to test post handling which caught the change in behavior. The issue is documented on SO and the body-parser repo but I looked around and didn't see a solution.

As an aside this is one of those things though that seems like overkill. body-parser is ~6000 lines of code to replace 20 lines of code. Sure it's supposed to handle some special cases but I'm not sure where any of those special cases would come up in HFT so for now I just refactored the old json parsing code into express middleware.

formula1 commented 9 years ago

Understandable :) I fully and respect your decisions. BodyParser certianly has a lot in it, though in other cases people. I'll try my best to do more tests/create more in case I submit to your repo. This is very far down the line, so the last thing I want to do break what isn't broken.

I'm sincerely excited to see this project continue to evolve!