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 separate keybind for selecting which gun ammo is used by default #1660

Merged
merged 5 commits into from
Jul 6, 2022

Conversation

olanti-p
Copy link
Contributor

Summary

SUMMARY: Interface "Add separate keybind for selecting which gun ammo is used by default"

Purpose of change

Fixes #1352
When pressing F to change gun mode if the gun has only 1 mode, and it's either RELOAD_AND_SHOOT (slings, bows) or RELOAD_ONE (shotguns, revolvers), the game will instead try to select default ammo to use.

There are a couple problems with this:

  1. If none of the loading options are available (e.g. shotgun is loaded with regular shot, but you only have dragon breath left), the game will show a erroneous "You don't have any X to reload your Y" message.
  2. If the shotgun has a second magazine (e.g. Kel-Tec KSG), it has a second firing mode, so the player loses ability to select default ammo.

Describe the solution

  1. Add a separate keybind for changing default gun ammo, and remove default ammo selection fallback from F key.
    Now, if the player tries to switch firing mode, and the gun has only 1 mode, it'll say so.
  2. Make "select default ammo" menu ignore current state of the gun and allow any compatible ammo to be marked as default.

Describe alternatives you've considered

Not adding yet another keybind, and instead do either of:

  1. Restrict default ammo feature to RELOAD_AND_SHOOT guns, as they are the main use case.
    • Might be inconvenient when (or if) weapons start supporting mixed ammo as the player would need to re-select which shot to load each time they want to load one.
    • Might be inconvenient for people that rely on this feature to skip ammo selection when loading in first shot.
  2. Don't change firing mode for RELOAD_ONE guns and instead go straight to default ammo selection menu.
    • Will make it impossible to switch firing mode for these guns outside target selection ui, which would be inconsistent with many other guns (rifles, pistols).
    • Will need to allow entering target selection ui with currently selected gun mode being empty (it's disallowed right now).

Testing

Marked some ammo as default using the new key or from target ui, tried reloading various shotguns and firing bows.

@Coolthulhu Coolthulhu self-assigned this Jun 28, 2022
src/player.cpp Outdated
@@ -2451,32 +2451,35 @@ bool player::list_ammo( const item &base, std::vector<item::reload_option> &ammo
bool ammo_match_found = false;
int ammo_search_range = is_mounted() ? -1 : 1;
for( const auto e : opts ) {
for( item_location &ammo : find_ammo( *e, empty, ammo_search_range ) ) {
for( item_location &ammo : find_ammo( *e, empty_mags, ammo_search_range ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, it would be good to make const item_location&.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh, looks like it was non-const to allow move-initialization of item_location for item::reload_option a few lines down, except item::reload_option's constructor takes a const item_location&, so the move does nothing.
Removed that move and changed ammo here to const item_location&.

src/player.cpp Outdated
// don't try to unload frozen liquids
if( ammo->is_watertight_container() && ammo->contents_made_of( SOLID ) ) {
continue;
}
auto id = ( ammo->is_ammo_container() || ammo->is_container() )
? ammo->contents.front().typeId()
: ammo->typeId();
if( e->can_reload_with( id ) ) {
bool can_reload_with = e->can_reload_with( id );
Copy link
Contributor

Choose a reason for hiding this comment

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

const?

Copy link
Member

Choose a reason for hiding this comment

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

Consting primitives is a bit of an overkill, except where it really needs to be clear that this variable won't change.
Ideally, all primitives would be treated as const, except in cases where they obviously have to change (iteration, accumulator variable etc.).

@Coolthulhu Coolthulhu merged commit 55b0730 into cataclysmbnteam:upload Jul 6, 2022
@olanti-p olanti-p deleted the fix-firing-mode-msg branch July 17, 2022 20:24
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.

Trying to change firing mode on a gun that you don't have more ammo for, produces misleading message
3 participants