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 a new parameter to OnRconLoginAttempt callback #892

Open
adib-yg opened this issue Mar 8, 2024 · 5 comments
Open

Add a new parameter to OnRconLoginAttempt callback #892

adib-yg opened this issue Mar 8, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@adib-yg
Copy link
Member

adib-yg commented Mar 8, 2024

Add the playerid parameter after the success parameter to OnRconLoginAttempt callback.

public OnRconLoginAttempt(ip[], password[], success, playerid)
{
    if (!success)
    {
        Kick(playerid);
    }
}

I know we can get the player id by loop through players' ip

@adib-yg adib-yg added the enhancement New feature or request label Mar 8, 2024
@NexiusTailer
Copy link
Contributor

NexiusTailer commented Mar 8, 2024

I suggested something similar earlier, but with separate callbacks OnInGameRconLoginAttempt (and OnInGameRconCommand for already logined rcon admins, as OnRconCommand also doen't pass player ID). If opinion about adding this was changed and it could be added, then I still think a separate variants will be much better than adding a playerid parameter to the existing ones, because:

  1. If I'm not wrong, it is called both from remote rcon (without connecting to a server) and in-game rcon, so passing playerid as 65535 half of all cases seems redundant.
  2. Already declared callbacks must be changed and recompiled due to new arguments in them, separate callbacks won't produce such problems.

UPD: Oh btw,

I know we can get the player id by loop through players' ip

actually we cannot be sure which playerid it is even using a loop because if the server allow many players from the same IP, a loop will find only one of the players on this IP address. That's why I also think about adding "playerid"-ful analogue of those callbacks for in-game rcon manipulations is a not so bad idea.

@Y-Less
Copy link
Collaborator

Y-Less commented Mar 11, 2024

Unfortunately adding new parameters to existing callbacks is a compiler error. Weirdly even just changing the names (not types) of existing ones is an error.

The other option is a function like GetRconAttemptPlayer(), returning INVALID_PLAYER_ID when there isn't one. The question with a new callback is how do the semantics work? Do we call only OnInGameRconLoginAttempt for in-game calls? Or both? What about in old scripts with only one? And any attempt to determine which to call could break expectations if a library hooks one or both - you think your script doesn't have the new variant when it does in a library, the server can't tell the difference.

I'm sure there are good answers to all these questions, they just need working out. However, whatever the solution there's still going to be duplicated code too. A Get function would allow people to update exsting scripts at their own pace.

@NexiusTailer
Copy link
Contributor

NexiusTailer commented Mar 11, 2024

Well it's also a good variant. Just doing it like with OnPlayerWeaponShot and GetPlayerLastShotVectors, where the second is also updated every shot, being useful inside OPWS and keep containing extended data about the last bullet further (until the new shot rewrite it). So, this seems a good idea for me too.

If considering both OnRconLoginAttempt and OnRconCommand, the naming can be related to "interaction", something like "get player interacted with rcon" (just the idea, not literally this name).

@ARKANANTAATAYA
Copy link

Pintar

@Y-Less
Copy link
Collaborator

Y-Less commented Mar 12, 2024

If considering both OnRconLoginAttempt and OnRconCommand, the naming can be related to "interaction", something like "get player interacted with rcon" (just the idea, not literally this name).

Good point. GetRconPlayer?

Edit: GetLastRconPlayer if we want to be more consistent with GetPlayerLastShotVectors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants