getUserHash returns an empty hash in a onUserAdded callback

[j-mie]

Description

If you try to call getUserHash on the plugin API in a onUserAdded callback you'll get MUMBLE_EC_OK with an empty hash.

Steps to reproduce

Try to call getUserHash on the plugin API in a onUserAdded callback you'll get MUMBLE_EC_OK with an empty hash.

Mumble version

No response

Mumble component

Client

OS

Windows

Reproducible?

Yes

Additional information

@Krzmbrzl linked this code snippet on Matrix:

mumble/src/mumble/Messages.cpp

Lines 354 to 365 in 815e8b1

	pDst = pmModel->addUser(msg.session(), u8(msg.name()));
	connect(pDst, &ClientUser::talkingStateChanged, Global::get().talkingUI, &TalkingUI::on_talkingStateChanged);
	connect(pDst, &ClientUser::muteDeafStateChanged, Global::get().talkingUI, &TalkingUI::on_muteDeafStateChanged);
	if (channel && channel != pDst->cChannel) {
	pmModel->moveUser(pDst, channel);
	}
	if (msg.has_hash()) {
	pmModel->setHash(pDst, u8(msg.hash()));
	}

Relevant log output

No response

Screenshots

No response

[Krzmbrzl]

Reason being: The event is fired inside the pmModel->addUser call, but some properties (most notably: the hash) of a user are set only in the code that follows the mentioned function call.

[Krzmbrzl]

Fixing this probably requires changing some of the event function's guarantees. The statements about when these events are triggered outside of the obvious case would have to be removed:

mumble/plugins/MumblePlugin_v_1_1_x.h

Lines 398 to 422 in cc5a570

	/// Called when a new channel gets added to the user model. This is the case when a new channel is created on the server
	/// the local user is on but also when the local user connects to a server that contains channels other than the
	/// root-channel (in this case this method will be called for ever non-root channel on that server).
	///
	/// @param connection An object used to identify the current connection
	/// @param channelID The ID of the channel that has been added
	PLUGIN_EXPORT void PLUGIN_CALLING_CONVENTION mumble_onChannelAdded(mumble_connection_t connection,
	mumble_channelid_t channelID);
	/// Called when a channel gets removed from the user model. This is the case when a channel is removed on the server the
	/// local user is on but also when the local user disconnects from a server that contains channels other than the
	/// root-channel (in this case this method will be called for ever non-root channel on that server).
	///
	/// @param connection An object used to identify the current connection
	/// @param channelID The ID of the channel that has been removed
	PLUGIN_EXPORT void PLUGIN_CALLING_CONVENTION mumble_onChannelRemoved(mumble_connection_t connection,
	mumble_channelid_t channelID);
	/// Called when a channel gets renamed. This also applies when a new channel is created (thus assigning it an initial
	/// name is also considered renaming).
	///
	/// @param connection An object used to identify the current connection
	/// @param channelID The ID of the channel that has been renamed
	PLUGIN_EXPORT void PLUGIN_CALLING_CONVENTION mumble_onChannelRenamed(mumble_connection_t connection,
	mumble_channelid_t channelID);

mumble/plugins/MumblePlugin_v_1_1_x.h

Lines 277 to 303 in cc5a570

	/// Called whenever any user on the server enters a channel
	/// This function will also be called when freshly connecting to a server as each user on that
	/// server needs to be "added" to the respective channel as far as the local client is concerned.
	///
	/// @param connection The ID of the server-connection this event is connected to
	/// @param userID The ID of the user this event has been triggered for
	/// @param previousChannelID The ID of the chanel the user is coming from. Negative IDs indicate that there is no
	/// previous channel (e.g. the user freshly connected to the server) or the channel isn't available because of any other
	/// reason.
	/// @param newChannelID The ID of the channel the user has entered. If the ID is negative, the new channel could not be
	/// retrieved. This means that the ID is invalid.
	PLUGIN_EXPORT void PLUGIN_CALLING_CONVENTION mumble_onChannelEntered(mumble_connection_t connection,
	mumble_userid_t userID,
	mumble_channelid_t previousChannelID,
	mumble_channelid_t newChannelID);
	/// Called whenever a user leaves a channel.
	/// This includes a client disconnecting from the server as this will also lead to the user not being in that channel
	/// anymore.
	///
	/// @param connection The ID of the server-connection this event is connected to
	/// @param userID The ID of the user that left the channel
	/// @param channelID The ID of the channel the user left. If the ID is negative, the channel could not be retrieved.
	/// This means that the ID is invalid.
	PLUGIN_EXPORT void PLUGIN_CALLING_CONVENTION mumble_onChannelExited(mumble_connection_t connection,
	mumble_userid_t userID,
	mumble_channelid_t channelID);

mumble/plugins/MumblePlugin_v_1_1_x.h

Lines 380 to 396 in cc5a570

	/// Called when a new user gets added to the user model. This is the case when that new user freshly connects to the
	/// server the local user is on but also when the local user connects to a server other clients are already connected to
	/// (in this case this method will be called for every client already on that server).
	///
	/// @param connection An object used to identify the current connection
	/// @param userID The ID of the user that has been added
	PLUGIN_EXPORT void PLUGIN_CALLING_CONVENTION mumble_onUserAdded(mumble_connection_t connection, mumble_userid_t userID);
	/// Called when a user gets removed from the user model. This is the case when that user disconnects from the server the
	/// local user is on but also when the local user disconnects from a server other clients are connected to (in this case
	/// this method will be called for every client on that server).
	///
	/// @param connection An object used to identify the current connection
	/// @param userID The ID of the user that has been removed
	PLUGIN_EXPORT void PLUGIN_CALLING_CONVENTION mumble_onUserRemoved(mumble_connection_t connection,
	mumble_userid_t userID);

However, this would essentially break backwards compatibility with (potentially) existing plugins 👀
I'll have to think about this some more