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.

Sign up for GitHub

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

Jump to bottom

Add "Auto Switch Weapon on Pickup" option #541

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+38 −16
Open

Add "Auto Switch Weapon on Pickup" option #541

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2678c49
Add "Auto Switch Weapon on Pickup" option
andrikpowell Nov 27, 2024
8505bd5
Implement `P_AutoSwitchWeapon`
andrikpowell Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions prboom2/src/dsda/configuration.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,10 @@ dsda_config_t dsda_config[dsda_config_count] = {
"dsda_switch_when_ammo_runs_out", dsda_config_switch_when_ammo_runs_out,
CONF_BOOL(1)
},
[dsda_config_switch_weapon_on_pickup] = {
"dsda_switch_weapon_on_pickup", dsda_config_switch_weapon_on_pickup,
CONF_BOOL(1), NULL, STRICT_INT(1)
},
[dsda_config_viewbob] = {
"dsda_viewbob", dsda_config_viewbob,
CONF_BOOL(1)
Expand Down
1 change: 1 addition & 0 deletions prboom2/src/dsda/configuration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ typedef enum {
dsda_config_parallel_sfx_window,
dsda_config_movement_toggle_sfx,
dsda_config_switch_when_ammo_runs_out,
dsda_config_switch_weapon_on_pickup,
dsda_config_viewbob,
dsda_config_weaponbob,
dsda_config_quake_intensity,
Expand Down
1 change: 1 addition & 0 deletions prboom2/src/m_menu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3087,6 +3087,7 @@ setup_menu_t misc_settings[] = {
{ "Skip Quit Prompt", S_YESNO, m_conf, G_X, dsda_config_skip_quit_prompt },
{ "Death Use Action", S_CHOICE, m_conf, G_X, dsda_config_death_use_action, 0, death_use_strings },
{ "Boom Weapon Auto Switch", S_YESNO, m_conf, G_X, dsda_config_switch_when_ammo_runs_out },
{ "Auto Switch Weapon on Pickup", S_YESNO, m_conf, G_X, dsda_config_switch_weapon_on_pickup },
{ "Parallel Same-Sound Limit", S_NUM, m_conf, G_X, dsda_config_parallel_sfx_limit },
{ "Parallel Same-Sound Window", S_NUM, m_conf, G_X, dsda_config_parallel_sfx_window },
{ "Play SFX For Movement Toggles", S_YESNO, m_conf, G_X, dsda_config_movement_toggle_sfx },
Expand Down
1 change: 1 addition & 0 deletions prboom2/src/m_misc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ cfg_def_t cfg_defs[] =
MIGRATED_SETTING(dsda_config_parallel_sfx_window),
MIGRATED_SETTING(dsda_config_movement_toggle_sfx),
MIGRATED_SETTING(dsda_config_switch_when_ammo_runs_out),
MIGRATED_SETTING(dsda_config_switch_weapon_on_pickup),
MIGRATED_SETTING(dsda_config_viewbob),
MIGRATED_SETTING(dsda_config_weaponbob),
MIGRATED_SETTING(dsda_config_quake_intensity),
Expand Down
88 changes: 51 additions & 37 deletions prboom2/src/p_inter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put () around allow_incompatibility && !deathmatch && !netgame. I wouldnt think the ternary captures all those conditions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

//
// P_GiveAmmo
// Num is the number of clip loads,
Expand All @@ -134,7 +136,8 @@ static dboolean P_GiveAmmoAutoSwitch(player_t *player, ammotype_t ammo, int olda
weaponinfo[i].ammopershot <= player->ammo[ammo]
)
{
player->pendingweapon = i;
if (autoswitch)
player->pendingweapon = i;
break;
}
}
Expand Down Expand Up @@ -187,43 +190,47 @@ static dboolean P_GiveAmmo(player_t *player, ammotype_t ammo, int num)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can put if (!autoswitch) return true; before if (heretic) and remove these 2 conditions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah great catch! Also simplifies this section much more nicely :)

if (player->weaponowned[GetAmmoChange[ammo]])
{
player->pendingweapon = GetAmmoChange[ammo];
if (autoswitch)
player->pendingweapon = GetAmmoChange[ammo];
}
}

return true;
}

switch (ammo)
{
case am_clip:
if (player->readyweapon == wp_fist) {
if (player->weaponowned[wp_chaingun])
player->pendingweapon = wp_chaingun;
else
player->pendingweapon = wp_pistol;
}
break;

case am_shell:
if (player->readyweapon == wp_fist || player->readyweapon == wp_pistol)
if (player->weaponowned[wp_shotgun])
player->pendingweapon = wp_shotgun;
break;
if (autoswitch)
{
switch (ammo)
{
case am_clip:
if (player->readyweapon == wp_fist) {
if (player->weaponowned[wp_chaingun])
player->pendingweapon = wp_chaingun;
else
player->pendingweapon = wp_pistol;
}
break;

case am_cell:
case am_shell:
if (player->readyweapon == wp_fist || player->readyweapon == wp_pistol)
if (player->weaponowned[wp_plasma])
player->pendingweapon = wp_plasma;
if (player->weaponowned[wp_shotgun])
player->pendingweapon = wp_shotgun;
break;

case am_misl:
if (player->readyweapon == wp_fist)
if (player->weaponowned[wp_missile])
player->pendingweapon = wp_missile;
default:
break;
}
case am_cell:
if (player->readyweapon == wp_fist || player->readyweapon == wp_pistol)
if (player->weaponowned[wp_plasma])
player->pendingweapon = wp_plasma;
break;

case am_misl:
if (player->readyweapon == wp_fist)
if (player->weaponowned[wp_missile])
player->pendingweapon = wp_missile;
default:
break;
}
}
return true;
}

Expand All @@ -249,8 +256,8 @@ dboolean P_GiveWeapon(player_t *player, weapontype_t weapon, dboolean dropped)
player->weaponowned[weapon] = true;

P_GiveAmmo(player, weaponinfo[weapon].ammo, deathmatch ? 5 : 2);

player->pendingweapon = weapon;
if (autoswitch)
player->pendingweapon = weapon;
/* cph 20028/10 - for old-school DM addicts, allow old behavior
* where only consoleplayer's pickup sounds are heard */
// displayplayer, not consoleplayer, for viewing multiplayer demos
Expand All @@ -276,7 +283,8 @@ dboolean P_GiveWeapon(player_t *player, weapontype_t weapon, dboolean dropped)
{
gaveweapon = true;
player->weaponowned[weapon] = true;
player->pendingweapon = weapon;
if (autoswitch)
player->pendingweapon = weapon;
}
return gaveweapon || gaveammo;
}
Expand Down Expand Up @@ -628,7 +636,8 @@ void P_TouchSpecialThing(mobj_t *special, mobj_t *toucher)
return;
dsda_AddPlayerMessage(s_GOTBERSERK, player);
if (player->readyweapon != wp_fist)
player->pendingweapon = wp_fist;
if (autoswitch)
player->pendingweapon = wp_fist;
sound = sfx_getpow;
break;

Expand Down Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@andrikpowell andrikpowell Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore my thoughts on P_AutoSwitchWeapon... I changed my mind

if (player == &players[consoleplayer])
{
S_StartVoidSound(heretic_sfx_wpnup);
Expand All @@ -2317,7 +2327,8 @@ dboolean Heretic_P_GiveWeapon(player_t * player, weapontype_t weapon)
player->weaponowned[weapon] = true;
if (WeaponValue[weapon] > WeaponValue[player->readyweapon])
{ // Only switch to more powerful weapons
player->pendingweapon = weapon;
if (autoswitch)
player->pendingweapon = weapon;
}
}
return (gaveWeapon || gaveAmmo);
Expand Down Expand Up @@ -2843,7 +2854,8 @@ void TryPickupWeapon(player_t * player, pclass_t weaponClass,
{
P_GiveMana(player, MANA_2, 25);
}
player->pendingweapon = weaponType;
if (autoswitch)
player->pendingweapon = weaponType;
remove = false;
}
else
Expand All @@ -2866,7 +2878,8 @@ void TryPickupWeapon(player_t * player, pclass_t weaponClass,
player->weaponowned[weaponType] = true;
if (weaponType > player->readyweapon)
{ // Only switch to more powerful weapons
player->pendingweapon = weaponType;
if (autoswitch)
player->pendingweapon = weaponType;
}
}
if (!(gaveWeapon || gaveMana))
Expand Down Expand Up @@ -3002,7 +3015,8 @@ static void TryPickupWeaponPiece(player_t * player, pclass_t matchClass,
{
gaveWeapon = true;
player->weaponowned[wp_fourth] = true;
player->pendingweapon = wp_fourth;
if (autoswitch)
player->pendingweapon = wp_fourth;
}
}

Expand Down
Loading