dreamstalker / rehlds

Reverse-engineered HLDS
GNU General Public License v3.0
625 stars 165 forks source link

Add `PF_Message(Begin|End)_I`, `PF_Write*_I` hooks #1019

Closed ShadowsAdi closed 1 month ago

ShadowsAdi commented 4 months ago

Closes #284

s1lentq commented 1 month ago

Close as fixes c9f9bbf

StevenKal commented 1 month ago

Could have still worth it to still have those hooks additionally to your new hooking system, in case of coders would like to build their own message manager with a few more features.

s1lentq commented 1 month ago

Could have still worth it to still have those hooks additionally to your new hooking system

It's very inefficient to have hook on every PF_Write*

in case of coders would like to build their own message manager with a few more features.

I see no reason not to add these more features directly to the current impl message manager

StevenKal commented 1 month ago

in case of coders would like to build their own message manager with a few more features.

I see no reason not to add these more features directly to the current impl message manager

If this feasible and you O.K. to do it, yeah, will worth more. About features, things like:

/* Message data types.
 *
 * Note: To use with the "is_msg_data_modified" native. */
enum MSG_DT {
    MSG_DT_Destination = 0,
    MSG_DT_Origin,
    MSG_DT_TargetID,
    MSG_DT_Parameter,
};
/* Check if the data of a message is currently modified (so if a message hook has used some of the previous natives in order to change an original data).
 * The "iMessageDataType" parameter is a MSG_DT_* from "message_const.inc".
 * The "iNumber" parameter is only used with the "MSG_DT_Parameter" type. */
native is_msg_data_modified(MSG_DT:iMessageDataType, iNumber = 0);

I think I thought on the past to one or two more things but I forgot about it, just some ideas I forgot to note when I was working on something over a year ago.

s1lentq commented 1 month ago
  • Ability to have PRE & POST on the "registerHook", where PRE acts like the current behavior of your system ("register_message" behavior), & POST is like "register_event", unalterable data (parameters & rest). Also be able to know on a POST hook if a PRE hook has been blocked (SUPERCEDE).

Message manager uses a hookchain, which means that both PRE and POST events can be used

  • Ability to retrieve original values passed (like on my own ReAPI module with my parameters system, I am guessing you probably already took a look at its include one day). So original parameters and destination (MSG_*) + origin + entity/edict.

I do not think it is possible to implement this in the current impl of the message manager, because I preferred to use method without allocating additional memory to store the original parameters in favor of quickly replacing data on the fly directly in the msg buffer, therefore, the bottleneck is that it will not be able to get the original parameters in the POST event after modifying. But you can still do it in your code, just register the message with highest priority hook and save the original parameters in the PRE event.

  • Ability to alter destination (MSG_*) + origin + entity/edict.

This looks very unsafe, how to handle cases when the vanilla code does a loop on the players with destination MSG_ONE, but the hook altered it to MSG_ALL?

  • More "enhanced" "isModified", like via sum of bits of the parameters or else

This can be added bool isParamModified(size_t index)

StevenKal commented 1 month ago

Message manager uses a hookchain, which means that both PRE and POST events can be used

Ah yeah you are right (the "SendUserMessageData" as chain), I forgot about the fact it uses a hook chain!

  • Ability to retrieve original values passed (like on my own ReAPI module with my parameters system, I am guessing you probably already took a look at its include one day). So original parameters and destination (MSG_*) + origin + entity/edict.

I do not think it is possible to implement this in the current impl of the message manager, because I preferred to use method without allocating additional memory to store the original parameters in favor of quickly replacing data on the fly directly in the msg buffer, therefore, the bottleneck is that it will not be able to get the original parameters in the POST event after modifying. But you can still do it in your code, just register the message with highest priority hook and save the original parameters in the PRE event.

Well, need to create extra data to copy/store it, sure this will use some "extra memory", but maybe it could only be allocated when we start to alter something on the message (for a lot of calls we will not do it), however, if not modified, redirect to actual data. Yes, your workaround solution can eventually do the job.

  • Ability to alter destination (MSG_*) + origin + entity/edict.

This looks very unsafe, how to handle cases when the vanilla code does a loop on the players with destination MSG_ONE, but the hook altered it to MSG_ALL?

Well, obviously we must use it in conjonction of the target/entity alteration to avoid crash etc. (in case we change from MSG_ONE to MSG_ALL & vice-versa, but in some situations it could be used. At least if you add it, people will have the possibility to use it if needed, after that, that is up to them to make a proper use of such feature, there are supposed to be "responsible" of that they are doing!

  • More "enhanced" "isModified", like via sum of bits of the parameters or else

This can be added bool isParamModified(size_t index)

Good! But if in case you implement modification of destination (MSG_*) + origin + entity/edict, add the related functions too!

StevenKal commented 1 month ago

Nice update, @s1lentq!