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: Implement RG_PM_LadderMove hook #254

Merged
merged 5 commits into from
Jul 15, 2023

Conversation

ShadowsAdi
Copy link
Contributor

Related to #82

@RauliTop
Copy link
Contributor

RauliTop commented Mar 21, 2022

useful to create plugins like this: https://dev-cs.ru/threads/19404/

@WaLkZa
Copy link

WaLkZa commented Nov 18, 2022

@s1lentq merge it, please :)

@WaLkZa
Copy link

WaLkZa commented Mar 31, 2023

@dystopm
Copy link
Contributor

dystopm commented Jul 15, 2023

@ShadowsAdi @s1lentq I think this hook should keep the schema of PM_AirMove implementation, keeping physent parameter and an additional player index parameter

@s1lentq s1lentq merged commit d9b72bf into rehlds:master Jul 15, 2023
@dystopm
Copy link
Contributor

dystopm commented Jul 15, 2023

@s1lentq
CAPI_Impl.h

// PM_LadderMove hook
typedef IHookChainImpl<void, physent_t *, int> CReGameHook_PM_LadderMove;
typedef IHookChainRegistryImpl<void, physent_t *, int> CReGameHookRegistry_PM_LadderMove;

pm_shared.cpp

LINK_HOOK_VOID_CHAIN(PM_LadderMove, (physent_t *pLadder, int playerIndex = 0), pLadder, pmove->player_index + 1);

void EXT_FUNC __API_HOOK(PM_LadderMove)(physent_t *pLadder, int playerIndex)
{
	PM_LadderMove_internal(pLadder);
}

void PM_LadderMove_internal(physent_t *pLadder)
{
        // ...
}

regamedll_api.h

// PM_LadderMove hook
typedef IHookChain<void, physent_t *, int> IReGameHook_PM_LadderMove;
typedef IHookChainRegistry<void, physent_t *, int> IReGameHookRegistry_PM_LadderMove;

s1lentq added a commit that referenced this pull request Jul 15, 2023
@s1lentq
Copy link
Collaborator

s1lentq commented Jul 15, 2023

@dystopm
I thought I took a hasty merge, but this isn't the case, in my opinion, AirMove actually isn't a very good scheme, especially for modules where have a easy access to pmove.
I think it's better to use the wrapper with custom arguments for AMXX rather than doing it in the API.
Moreover, there are already a couple of such functions like PM_Move, PM_UpdateStepSound etc.
Don't you think?

I agree about the physent argument.

@dystopm
Copy link
Contributor

dystopm commented Jul 15, 2023

@s1lentq

I see you ended adding the physent argument which I think is fine guessing we can add logic in future to change physent structure values, so it's a yes, I agree with you.

In that case, shouldn't PM_AirMove be modified?

PD: there's a few of functions which I coded in my own fork which I find useful to be added, I'll be doing a commit taking advantage of you are merging

@s1lentq
Copy link
Collaborator

s1lentq commented Jul 15, 2023

In that case, shouldn't PM_AirMove be modified?

No, we should keep API compatibility at least until we update the major version

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.

5 participants