Add "Auto Switch Weapon on Pickup" option #541
Conversation
| @@ -187,43 +190,47 @@ static dboolean P_GiveAmmo(player_t *player, ammotype_t ammo, int num) | |||
| { | |||
There was a problem hiding this comment.
You can put if (!autoswitch) return true; before if (heretic) and remove these 2 conditions
There was a problem hiding this comment.
Ah great catch! Also simplifies this section much more nicely :)
prboom2/src/p_inter.c
Outdated
| @@ -2298,7 +2307,8 @@ dboolean Heretic_P_GiveWeapon(player_t * player, weapontype_t weapon) | |||
| player->bonuscount += BONUSADD; | |||
| player->weaponowned[weapon] = true; | |||
| P_GiveAmmo(player, wpnlev1info[weapon].ammo, GetWeaponAmmo[weapon]); | |||
| player->pendingweapon = weapon; | |||
| if (autoswitch) | |||
| player->pendingweapon = weapon; | |||
There was a problem hiding this comment.
The condition below is using brackets, so lets follow the style. Ik its not consistent at all, but its better if new code tries to match whats already there.
And maybe you should just extract this if into a new P_AutoSwitchWeapon
There was a problem hiding this comment.
Hmmm we could add a P_AutoSwitchWeapon, although not all of these cases are player->pendingweapon = weapon...
The player->pendingweapon is the same, but the result is variable. So doing a P_AutoSwitchWeapon() would require running a variable through, which at that point idk if it's worth the hassle.
That is unless you were thinking of replacing all autoswitch cases with a function... although the conditions around each autoswitch seem quite variable, so it doesn't seem like a major boon.
Actually regarding the brackets style, I'm fine with adding brackets. Although I usually tend to avoid using brackets for 1 line conditions, as I usually find it looks cleaner.
There was a problem hiding this comment.
Ignore my thoughts on P_AutoSwitchWeapon... I changed my mind
| @@ -108,6 +108,8 @@ static weapontype_t GetAmmoChange[] = { | |||
| wp_mace | |||
| }; | |||
|
|
|||
| #define autoswitch (allow_incompatibility && !deathmatch && !netgame ? dsda_IntConfig(dsda_config_switch_weapon_on_pickup) : true) | |||
|
|
|||
There was a problem hiding this comment.
Put () around allow_incompatibility && !deathmatch && !netgame. I wouldnt think the ternary captures all those conditions
There was a problem hiding this comment.
So in my test, this does actually work correctly. (i.e. I tested putting allow_incompatiblity at the end) and it still worked as intended.
Lol I will admit it is probably better practice to put () around the conditions though, so I'll do it anyway. :P
|
Implementing |
Took a stab at implementing an Auto Switch option asked for in (#511).
It's a rather simple casual only setting that's disabled in strict mode, demo recording/playing, netgame, and deathmatch.
Let me know if there's better precedent when it comes to netgame / deathmatch compat for options.