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

API: Add hooks ED_Alloc() & ED_Free(). #867

Merged
merged 11 commits into from
Oct 20, 2021
Merged

API: Add hooks ED_Alloc() & ED_Free(). #867

merged 11 commits into from
Oct 20, 2021

Conversation

StevenKal
Copy link
Contributor

@StevenKal StevenKal commented Oct 19, 2021

Addition of new & useful API hooks related to the start & end points of the entity creation/removal.
Please add this QUICKLY! With the others of @francoromaniello at the same time! Hahaha!

@wopox1337
Copy link
Collaborator

I guess you have to add in RehldsFuncs_t g_RehldsApiFuncs = also &ED_Alloc_api, &ED_Free_api ?

Like here: 81fe334#diff-d65c6e4e25d9b3976051fc1605a2a6711248b2ee520dfc321967f472d2857329R518

also in class CRehldsHookchains : public IRehldsHookchains { ... }

@StevenKal
Copy link
Contributor Author

StevenKal commented Oct 19, 2021

Adding the API functions as "function call" is not needed, calling "pfnCreateEntity" & "pfnRemoveEntity" trigger them directly and so, the hooks will be.
Hook "ED_Alloc" was more to have "new entity creation" in the BSP loading process (ED_LoadFromFile), despite not fully needed, and "ED_Free" in order to catch the final call, since already existing "PF_Remove_I" hook is not triggered in all cases (FL_KILLME as example), and "pfnOnFreeEntPrivateData" was not good enough as it might be triggered on already existing entities where only the pvPrivateData is cleaned up before a new allocation right after (without entity removal).

The hooks are already in "class CRehldsHookchains : public IRehldsHookchains", I do not think I have missed something.

Thanks for having quickly reviewed & approved them, appreciated!
I hope you will merge all of this soon with a few others PRs being O.K. with a new release! Hehe!

PS: You can move it from "To do" list to "Done" in the "Projects"!

@DarthMan
Copy link

Now all that's left is for someone to implement them on ReAPI as well as to have the GetEntityInit hook available.

@StevenKal
Copy link
Contributor Author

Why you do not just merge the PR without s1lentq review, especially knowing you got the repository rights? The changes are not complexes & I think you know all is good (& understand the code).
Because if s1lentq is in hibernating process, this might take ages!

And one more question: Can you merge it in force even if you asked s1lentq's review and if he do not review it?
I am not asking to merge it immediately (with both franco's hooks & some other "O.K." PRs too [like your Sys_Error modification]) & make a new release (despite this will be appreciated), just asking if you have the possibility to "bypass his reviewing request" in case of he never reviews it, or, if this is "locked" and you absolutely need to wait for him (since you requested his review) in order to be able to merge to official. I am just asking as information about "that possibility" since I do not know much GitHub, and since you (wopox) already approved the changes.

The fact is, I have difficulties to understand why some PRs which seem good & "finalized" are often dormant, and sometimes for months, for all the Re* projects.

@wopox1337 wopox1337 merged commit 04ddafe into dreamstalker:master Oct 20, 2021
@wopox1337 wopox1337 changed the title API: Add hooks "ED_<Alloc|Free>". API: Add hooks ED_Alloc & ED_Free. Oct 20, 2021
@wopox1337 wopox1337 changed the title API: Add hooks ED_Alloc & ED_Free. API: Add hooks ED_Alloc() & ED_Free(). Oct 20, 2021
@StevenKal
Copy link
Contributor Author

StevenKal commented Oct 20, 2021

I guess I have my answer for my second question! It is "yes you can!" Hehehehehe!
Thank you very much!

Now you can merge a few others PRs ready, then make a release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants