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

Updated to Socket.IO v1 api in case of active signalmaster's maxClients configuration #57

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

Conversation

scotu
Copy link

@scotu scotu commented Jan 5, 2016

Just a trivial fix for Socket.IO v1, I encountered the bug using maxClients in the config. I wasn't sure if it's good to return 0 if the room is not valid (because I'm not sure how it can happen that is not valid).

Let me know if I need to adjust anything.

(Thanks for signalmaster, btw :D)

return io.sockets.clients(name).length;
var room = io.sockets.adapter.rooms[name];
if (room && room.length) {
return room.length;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work for me:
room.length is always undefined.
You should use Object.keys(room).length instead of room.length

Otherwise clientsInRooms is always 0;

@scotu
Copy link
Author

scotu commented Jan 9, 2016

My "fix" was indeed wrong. I updated the pr, I also noticed that to get the room, in other functions in signalmaster, the namespace was selected manually, so I went that way also here.

Let me know any further problems or requirements

Thanks

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.

3 participants