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

New CVars: mp_freezetime_duck and mp_freezetime_jump #886

Merged

Conversation

FEDERICOMB96
Copy link
Contributor

@FEDERICOMB96 FEDERICOMB96 commented Oct 28, 2023

  • Added cvar mp_freezetime_duck which allow ducking during freezetime.
    (0) disabled
    (1) enabled (default behaviour)

  • Added cvar mp_freezetime_jump which allow jumping during freezetime.
    (0) disabled
    (1) enabled (default behaviour)

Tested, work as intended 😄

PS: Cvar's names/description corrections are welcome

@abecee
Copy link

abecee commented Oct 28, 2023

That's a great idea. Good job.

@justgo97
Copy link

Instead of adding extra checks in PM_Duck & PM_Jump, It probably would be better to use PLAYER_PREVENT_DUCK
& PLAYER_PREVENT_JUMP flags on player when freezetime is on and to remove the flags once freezetime is over.

@dystopm
Copy link
Contributor

dystopm commented Oct 28, 2023

Instead of adding extra checks in PM_Duck & PM_Jump, It probably would be better to use PLAYER_PREVENT_DUCK & PLAYER_PREVENT_JUMP flags on player when freezetime is on and to remove the flags once freezetime is over.

Those are meant to be using by modders. By enabling/disabling you may interfere with 3rd party plugin behaviours. (I suggested it)

@dystopm
Copy link
Contributor

dystopm commented Oct 28, 2023

The cvar name is too long and literal. It should have a short name instead, there isn't a reason to explain the control behaviour in the name. Something like mp_freezetime_duck and mp_freezetime_jump, you can see all other added cvars in game.cfg which are mostly compact based on what they control.

@justgo97
Copy link

Instead of adding extra checks in PM_Duck & PM_Jump, It probably would be better to use PLAYER_PREVENT_DUCK & PLAYER_PREVENT_JUMP flags on player when freezetime is on and to remove the flags once freezetime is over.

Those are meant to be using by modders. By enabling/disabling you may interfere with 3rd party plugin behaviours. (I suggested it)

IMO Modders can enforce the cvars to be 0 or reset the flags on freezetime start/end so it won't really bother them much. It would be cleaner to not bloat PM_Duck & PM_Jump with these checks. Performance wise it's also better to avoid adding code in these functions that get called frequently. but it's fine to keep it the way it is now too

@dystopm
Copy link
Contributor

dystopm commented Oct 29, 2023

Instead of adding extra checks in PM_Duck & PM_Jump, It probably would be better to use PLAYER_PREVENT_DUCK & PLAYER_PREVENT_JUMP flags on player when freezetime is on and to remove the flags once freezetime is over.

Those are meant to be using by modders. By enabling/disabling you may interfere with 3rd party plugin behaviours. (I suggested it)

IMO Modders can enforce the cvars to be 0 or reset the flags on freezetime start/end so it won't really bother them much.

And yet, you're forcing modifications to handle this new behaviour which in the way it is now it wont hurt anybody. Instead, by using those custom flags, you can mess with more than one custom behaviour. That's why they are split

It would be cleaner to not bloat PM_Duck & PM_Jump with these checks.

Bloat why? They are two simple conditionals which are described in the comments, where one is a rule (cvar, like many others) and the other a custom flag. There are nastier conditions around the code, this one is completely sane

Performance wise it's also better to avoid adding code in these functions that get called frequently.

Hmm, performance card arguing on a simple statement? On C++? A machine-level interpreted source? 😛

@FEDERICOMB96
Copy link
Contributor Author

The cvar name is too long and literal. It should have a short name instead, there isn't a reason to explain the control behaviour in the name. Something like mp_freezetime_duck and mp_freezetime_jump, you can see all other added cvars in game.cfg which are mostly compact based on what they control.

Good point. Ty ❤️

@FEDERICOMB96 FEDERICOMB96 changed the title New CVars: mp_prevent_duck_during_freezetime and mp_prevent_jump_during_freezetime New CVars: mp_freezetime_duck and mp_freezetime_jump Oct 30, 2023
@Matiarguello
Copy link

The cvar name is too long and literal. It should have a short name instead, there isn't a reason to explain the control behaviour in the name. Something like mp_freezetime_duck and mp_freezetime_jump, you can see all other added cvars in game.cfg which are mostly compact based on what they control.

Good point. Ty ❤️

Screenshot_1

@justgo97
Copy link

justgo97 commented Nov 1, 2023

And yet, you're forcing modifications to handle this new behaviour which in the way it is now it wont hurt anybody. Instead, by using those custom flags, you can mess with more than one custom behaviour. That's why they are split

Actually this current approach is less flexible from a modder POV, let's say I want to make only a specific team able to duck/jump during freeze time, if the flags were used I would just change the flags on player spawn with my desired value but since the flags are overrided by the cvar value there is nothing I can do other than setting the cvars to 0 and making my own implementation.

Bloat why? They are two simple conditionals which are described in the comments, where one is a rule (cvar, like many others) and the other a custom flag. There are nastier conditions around the code, this one is completely sane

Performance wise it's also better to avoid adding code in these functions that get called frequently.

Hmm, performance card arguing on a simple statement? On C++? A machine-level interpreted source? 😛

It's better to sperate concerns when possible, things like freezetime is only related to the first 10-5 seconds of the round, keeping them in a code that runs every frame for every player is not the optimal way to do this, this is just a suggestion, I'm not arguing lol 😛

@dystopm
Copy link
Contributor

dystopm commented Nov 1, 2023

And yet, you're forcing modifications to handle this new behaviour which in the way it is now it wont hurt anybody. Instead, by using those custom flags, you can mess with more than one custom behaviour. That's why they are split

Actually this current approach is less flexible from a modder POV, let's say I want to make only a specific team able to duck/jump during freeze time, if the flags were used I would just change the flags on player spawn with my desired value but since the flags are overrided by the cvar value there is nothing I can do other than setting the cvars to 0 and making my own implementation.

Bloat why? They are two simple conditionals which are described in the comments, where one is a rule (cvar, like many others) and the other a custom flag. There are nastier conditions around the code, this one is completely sane

Performance wise it's also better to avoid adding code in these functions that get called frequently.

Hmm, performance card arguing on a simple statement? On C++? A machine-level interpreted source? 😛

It's better to sperate concerns when possible, things like freezetime is only related to the first 10-5 seconds of the round, keeping them in a code that runs every frame for every player is not the optimal way to do this, this is just a suggestion, I'm not arguing lol 😛

let's say I want to make only a specific team able to duck/jump during freeze time

Then don't use this cvar and code the rule? lmao, with or without this commit you'll be in need to do it

keeping them in a code that runs every frame for every player is not the optimal way to do this

Seriously? Are we in 2005's alliedmods forums?

@s1lentq s1lentq merged commit e3d70d2 into s1lentq:master Nov 26, 2023
4 checks passed
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.

6 participants