-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Characters automatically reload gas masks when filters run low #66913
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of automating character actions but we should also make it as non-intrusive as possible, so here are some suggestions.
src/character_management.cpp
Outdated
// No valid targets? Tell the player, and don't try to autoreload for a bit | ||
if( mags.empty() ) { | ||
if( !is_npc && info.warn_no_ammo ) { | ||
popup( "You don't have anything to reload your %s with!", it_name ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a message to be consistent with other warnings.
Also, it seems this message and the query below will repeat itself every minute after the player's only magazine is nearly depleted or they choose not to reload, which I think is not good user expereience. I suggest to only warn once when no magazine is found, and add an option to never ask again to RELOAD_CHOICE
until the next auto-reload opportunity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather this be a popup to ensure I know I need to get out before my gas mask goes, but I agree it's not the best experience.
add_msg_if_player( m_good, _( "You notice your %s is running low and reload it." ), it_name ); | ||
|
||
// TODO: NPCs complain about running out of reloads | ||
if( mags.size() == 1 && info.warn_on_last && !is_npc ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this handle reloading with non-magazine items (such as bullets) which may only be partially consumed?
@@ -4270,6 +4271,7 @@ std::optional<int> iuse::gasmask( Character *p, item *it, bool t, const tripoint | |||
_( "<npcname> needs new gas mask filters!" ) | |||
, it->tname() ); | |||
} | |||
p->try_autoreload( *it, char_autoreload::params() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes more sense to reload with the fullest magazine so it lasts longer.
Also, how does this solve the problem that the player may not want to reload at a specific moment? For example, if the item charge happens to run out when the player is facing a much more serious threat, automatically reloading may cause the player to become more seriously harmed.
How does this handle the case where the player wants to specifically use a magazine with low charges? Some items may accept different types of ammo so the player may want to specifically use one with some special property that is important to their task.
An option in the auto-features option group to override the default reload types would be nice too (although adding more options may not be desired due to the concern about option bloat)
} | ||
|
||
// Try to reload and see how it goes | ||
bool status = it.reload( *this, mags[picked], 1 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this reduce the move points of the reloader? Also, some reloading actions takes a long time to complete so it might make sense to make it an activity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, and it can be a significant number of moves. This will take a fair bit of work to address (with the above concerns).
// The gas mask | ||
static const itype_id itype_mask_gas( "mask_gas" ); | ||
// Flag identifying cartridges | ||
static const flag_id json_flag_GASFILTER_MED( "GASFILTER_MED" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to add test-specific items to avoid changes to the stats of in-game items causing test regressions.
src/character_management.cpp
Outdated
continue; | ||
} | ||
// Only mags with sufficient ammo to avoid immediate reload, too! | ||
if( loc->ammo_remaining() < 2 ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might also make sense to auto-reload with the fullest magazine if the current magazine is depleted but all the remaining magazines have less than this many remaining charges.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered. |
I'd really like to merge this, are you still around to check out qrox' suggestions @ehughsbaird ? |
Yes, but I would not expect significant movement very soon. |
Allows storing a time value on an item without converting to turns and then converting back.
I recently visited a collapsed tower, and as I fought my way through the dungeon, I faced a consistent issue: my gas mask would run out, and I would not notice, becoming poisoned. This was, needless to say, quite annoying. So, let's have the character manage the changing of filters, instead of the player. The solution is simple. Add a function to Character that will will take an item and look at it's remaining ammo. If the item is low, but not quite out, the function will warn every 8 turns about the item being low. When the item runs out, it will scan the player inventory and reload with appropriate items. Then, this function simply needs to be called wherever the item consumes charges, and the player will automatically handle reloading. A variety of behavior tweaks are available, but by default (and always, as there is no way to change them), a popup shown when the last suitable item has been reloaded (next reload will fail) and when a reload is attempted but fails. Several tests have been added to ensure this behavior works for both players and npcs.
b7c77df
to
23e724e
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered. |
Summary
Features "Characters automatically reload gasmasks when they run low"
Purpose of change
I recently visited a collapsed tower, and as I fought my way through the dungeon, I faced a consistent issue: my gas mask would run out, and I would not notice, becoming poisoned.
This was, needless to say, quite annoying. So, let's have the character manage the changing of filters, instead of the player.
Describe the solution
The solution is simple. Add a function to Character that will will take an item and look at it's remaining ammo. If the item is low, but not quite out, the function will warn every 8 turns about the item being low. When the item runs out, it will scan the player inventory and reload with appropriate items.
Then, this function simply needs to be called wherever the item consumes charges, and the player will automatically handle reloading.
A variety of behavior tweaks are available, but by default (and always, as there is no way to change them), a popup shown when the last suitable item has been reloaded (next reload will fail) and when a reload is attempted but fails.
Describe alternatives you've considered
I couldn't figure out another way to solve this problem short of only adding more messages when the gas mask is running low. I think this is preferable.
Testing
Several tests have been added to ensure this behavior works for both players and npcs.
For manual testing, equip a gas mask, grab some cartridges, and expose yourself to a lot of gas. (Going underground makes the gas dissipate slower).
The screenshots below show what this will be like to the player with the current settings
Default Settings
Start:
Reload 1:
Reload 2:
Gas mask runs out:
The following sections show the three different reload methods. All start with this inventory:
Least First reload
Most First reload
Player choose reload
Additional context
I'd obviously like to let the player choose between these three options, as well as expand this to more things, but I figured I should get this out there first.