prestonp / simple-rcon

Simple, painless node RCON client for Source servers
MIT License
14 stars 4 forks source link

cleanup #8

Closed skibz closed 9 years ago

skibz commented 9 years ago

wanted to focus on adding some tests and getting a steady build process in place. this is your baby, if you'd prefer changes to my additions to meet your standards/requirements, let me know :stuck_out_tongue:

prestonp commented 9 years ago

@skibz This is awesome work! I will take a closer look later today, thank you! :dancer:

skibz commented 9 years ago

i still need to read the rcon documentation so i can write more complex tests for the packet and protocol stuff, but i really hate the valve dev documentation :pensive:

also, please note that i don't have access to a rcon game server for live testing. so, until this is run "for reals", not safe to merge.

edit: and hey, wouldn't it be cool to have this library listed on valve rcon docs?

prestonp commented 9 years ago

Yes once these changes get merged I'd love to add it on the wiki.

I would like to test these changes, I'll ask some friends if I can test on a server

prestonp commented 9 years ago

@skibz If you have a steam account you can rent servers for free for an hour at a time with RCON access.

http://na.serveme.tf/

prestonp commented 9 years ago

How come flush, listen, connect, and dequeue were removed from the Rcon prototype? I think it would be more semantic than using call(this) if we kept it on the prototype

skibz commented 9 years ago

How come flush, listen, connect, and dequeue were removed from the Rcon prototype? I think it would be more semantic than using call(this) if we kept it on the prototype

i moved them into private functions to make them inaccessible to the user. those functions manipulate the command buffers, net socket, and such, so i thought it'd be safe to keep them hidden away and not on the prototype. and invoking them using call or bind helps to avoid doing var self = this; too much.

one of my goals was to remove functions from the prototype that a user wouldn't typically use and those functions seemed to be that. the prototype now only exposes connect, exec and close (i think) which is a much more focused public api for users.

but all in all, it's a pretty opinionated change. so if you'd prefer it the old way, let me know. :stuck_out_tongue:

@skibz If you have a steam account you can rent servers for free for an hour at a time with RCON access.

thanks, man. i'll figure out a testing solution asap!

prestonp commented 9 years ago

Hey @skibz sorry I took a mental vacation from github for a bit, I'm gonna test out your branch with a test server, it looks good so far! Also, I agree with hiding all of that network stuff from the user.

prestonp commented 9 years ago

:+1: Thank you!!!! This is fantastic

skibz commented 9 years ago

i'm glad you're happy, dude. :D going forward, i still need to improve the test coverage a bit. did your testing on the test server work as expected?

prestonp commented 9 years ago

Yup, I rented a temp server from na.serveme.tf, was able to run commands successfully