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

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

Closed
wants to merge 1 commit into from

Conversation

ShadowsAdi
Copy link
Contributor

@ShadowsAdi ShadowsAdi commented Feb 22, 2024

Closes #284

@s1lentq
Copy link
Collaborator

s1lentq commented May 13, 2024

Close as fixes c9f9bbf

@s1lentq s1lentq closed this May 13, 2024
@StevenKal
Copy link
Contributor

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
Copy link
Collaborator

s1lentq commented May 19, 2024

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
Copy link
Contributor

StevenKal commented May 19, 2024

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:

  • 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).
  • 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.
  • Ability to alter destination (MSG_*) + origin + entity/edict.
  • Ability to "quickly" reset the parameters (and possibly the dest/origin/entity) to their original values.
  • More "enhanced" "isModified", like via sum of bits of the parameters or else. I have on an old dev release something 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
Copy link
Collaborator

s1lentq commented May 20, 2024

  • 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
Copy link
Contributor

StevenKal commented May 20, 2024

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
Copy link
Contributor

Nice update, @s1lentq!

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.

Is it possible to add hooks for pfnMessageBegin, pfnMessageEnd, etc...
3 participants