sourcemod icon indicating copy to clipboard operation
sourcemod copied to clipboard

Post UserMessageHook sent param bugged

Open Kenzzer opened this issue 6 years ago • 3 comments

Help us help you

  • [x] I have checked that my issue doesn't exist yet.
  • [x] I have tried my absolute best to reduce the problem-space and have provided the absolute smallest test-case possible.
  • [x] I can always reproduce the issue with the provided description below.

Environment

  • Operating System version: Windows 10
  • Game/AppID (with version if applicable): 440 but should affect all games
  • Current SourceMod version: 1.9.0.6275
  • Current SourceMod snapshot: N/A
  • Current Metamod: Source snapshot:
  • [x] I have updated SourceMod to the latest version and it still happens.
  • [x] I have updated SourceMod to the latest snapshot and it still happens.
  • [x] I have updated SourceMM to the latest snapshot and it still happens.

Description

It seems the sent param in the post UserMessageHook to be always true. And I believe this is the reason but I'd rather not open a PR if there's an explanation for it. https://github.com/alliedmodders/sourcemod/blob/aae71612731eaa6771067a31f3a7ad34bdb0df28/core/UserMessages.cpp#L810-L812 https://github.com/alliedmodders/sourcemod/blob/aae71612731eaa6771067a31f3a7ad34bdb0df28/core/UserMessages.cpp#L543-L545 https://github.com/alliedmodders/sourcemod/blob/aae71612731eaa6771067a31f3a7ad34bdb0df28/core/UserMessages.cpp#L644

Problematic Code (or Steps to Reproduce)

public void OnPluginStart()
{
  HookUserMessage(GetUserMessageId("SayText2"), Hook_SayText2, true, Hook_SayText2Post);
}
public void Hook_SayText2Post(UserMsg msg_id, bool sent)
{
  PrintToChatAll("sent %i", sent);
}
public Action Hook_SayText2(UserMsg msg_id, BfRead message, const int[] players, int playersNum, bool reliable, bool init)
{
  return Plugin_Handled;
}

Logs

  • None

Kenzzer avatar Feb 06 '19 19:02 Kenzzer

You're right, we only run set m_BlockEndPost when Plugin_Stop is set. However, the documentation we have is...

MsgHook
Syntax:
functag public Action:MsgHook(UserMsg:msg_id, Handle:msg, const players[], playersNum, bool:reliable, bool:init);

Usage:
 msg_id		Message index.
 msg			Handle to the input bit buffer or protobuf.
 players		Array containing player indexes.
 playersNum	Number of players in the array.
 reliable		True if message is reliable, false otherwise.
 init			True if message is an initmsg, false otherwise.
Notes:
Called when a message is hooked

Return:
Ignored for normal hooks. For intercept hooks, Plugin_Handled blocks the message from being sent, and Plugin_Continue resumes normal functionality.

Version Added:
1.0.0.1946

Unfortunately I think it's too late to fix this now. UserMessages have always had bizarre semantics, I think the best we can do here is fix-up the docs.

KyleSanderson avatar May 10 '19 18:05 KyleSanderson

Wouldn't it be fine to have the sent param set to false for Plugin_Handled as well?

I know I'm going from an assumption and it's bad to think that way. But since the param never worked as intended (except for Plugin_Stop) I doubt any plugins made use of it?

If the documentation is fixed to: Ignored for normal hooks. For intercept hooks, **Plugin_Stop** blocks the message from being sent, and Plugin_Continue resumes normal functionality.

We would also have to mention Plugin_Handled has the ""same"" behaviour as Plugin_Stop except it doesn't set the sent param to false? Or just clearly state not to use but that would be weird no?

Kenzzer avatar Aug 02 '19 08:08 Kenzzer

I've just come across this myself. One of the solutions above should absolutely get implemented.

Having the correct value in "sent" is important for modifying (=recreating) user messages because you get an error when starting a new message and the original was not blocked (sent=true).

Alienmario avatar Nov 23 '22 22:11 Alienmario