Rantanen / node-mumble

Mumble client in Node.js
MIT License
155 stars 48 forks source link

In voice-start and voice-end events, user.name is null #66

Closed justinmchase closed 9 years ago

justinmchase commented 9 years ago

Maybe this isn't always true, but for our server it is. The code in question is:

https://github.com/EvolveLabs/electron-mumble/blob/master/lib/MumbleConnection.js#L715

if( packet.frame && !user.talking ) {
    user.talking = true;
    this.emit( 'voice-start', { session: user.session, name: user.name, talking: true } );
}

The packet shape is correct, it seems like mumble just doesn't send the user name anymore. Instead it gives you the session id and expects you to correlate it with a user I think. This could possibly be corrected by setting the name based on the users collection:

var name = user.name || this.users[ user.session ].name;
this.emit( 'voice-start', { session: user.session, name: name, talking: true } );

Our workaround is to essentially do this in the handler:

connection.on('voice-start', function (user) {
  user.name = connection.sessions[user.session].name;
  ...
});

I'll try to send a PR soon-ish if you don't do it sooner.

Rantanen commented 9 years ago

Hm. That's weird if the user.name || this.users[ user.session ].name works given the user in the packet is set from this.users[ user.session ] around line 800

Mumble never sent the name in voice packet, only the session ID. All the convenience bits, such as user name, are purely from node-mumble. Not sure when I've got time to check this - I've got the Node 4.0.0 compatibility stuff to check for lots of dependent modules still as well and a small hiking trip planned for the weekend. Might find time next week unless you come up with a PR before that. :)

justinmchase commented 9 years ago

I'm probably swamped until after next week. But I'm not blocked, just something interesting there.