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

Skip Melee in Firing Mode #35132

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

LetterShapedGlyphs
Copy link
Contributor

@LetterShapedGlyphs LetterShapedGlyphs commented Oct 29, 2019

Summary

SUMMARY: Bugfixes "Fixes crashes involving bayonet type items"

Purpose of change

Fixes #35117 and Fixes #30676

Describe the solution

Reverted an erroneous change that broke the loading of gunmods.
Added extra logic to skip over melee gun modes when in the firing screen.

Describe alternatives you've considered

Automatically switch between reach_attack() targeting view and fire() targeting view.
Merge the two firing views.

Testing

Created a new character.
Spawned in a rifle.
Spawned in one of every bayonet type gunmod (combat knife, makeshift bayonet, etc.)
Tested that each can be installed as a gunmod on a rifle.
Spawned in ammo and a magazine for the rifle.
Tested switching firing modes both inside of and outside of the fire() targeting view.
Also tested that regular reach attacks still worked.

Additional context

@@ -1875,7 +1875,7 @@ std::string item::info( std::vector<iteminfo> &info, const iteminfo_query *parts
info.emplace_back( "GUNMOD", _( "Handling modifier: " ), "",
iteminfo::show_plus, mod.handling );
}
if( is_gun() && !type->mod->ammo_modifier.empty() && parts->test( iteminfo_parts::GUNMOD_AMMO ) ) {
if( !type->mod->ammo_modifier.empty() && parts->test( iteminfo_parts::GUNMOD_AMMO ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking if ammo_modifier is empty is sufficient for all gunmods now that all gunmod items actually have def.mod instantiated, and makes sure that any gunmod which isn't itself a gun, but still modifies ammo, will work in the future (none exist currently)

load_slot_optional( def.gunmod, jo, "gunmod_data", src );
load_slot_optional( def.mod, jo, "mod_data", src );
load_slot_optional( def.mod, jo, "gunmod_data", src );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

load_gunmod() Already calls both of these functions, as both are needed. This revert allows items which are not typed as GUNMOD to exhibit correct behavior.

@LetterShapedGlyphs
Copy link
Contributor Author

LetterShapedGlyphs commented Oct 29, 2019

As mentioned in my code comment, load_gunmod() already calls both load_slot( islot_mod [...] ) and load_slot( islot_gunmod [...] ) so items which can be optional gunmods need to call both of those with load_slot_optional().

The resulting 'unvisited JSON' errors either need to be ignored, the entire def.gunmod and def.mod dichotomy needs to be fixed for non GUNMOD typed items, or no items should be usable as GUNMODs unless they are explicitly that type (no more combat knife bayonet).

@AMurkin
Copy link
Contributor

AMurkin commented Oct 29, 2019

Fixes #35117 and #30676

Rewrite it like so:
Fixes #35117 and Fixes #30676

@LetterShapedGlyphs
Copy link
Contributor Author

LetterShapedGlyphs commented Oct 29, 2019

Rewrite it like so:
Fixes #35117 and Fixes #30676

Ah, didn't realize the word "Fixes" was important. Thank you.

@ifreund ifreund added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Items / Item Actions / Item Qualities Items and how they work and interact Items: Gunmod / Toolmod Weapon and tool attachments, and add-ons labels Oct 29, 2019
@snipercup
Copy link
Contributor

Tested:

  • spawned a m4a1 and a glock 17
  • spawned some gunmods
  • attached a combat knife to the m4a1
  • removed the combat knife and added a sword bayonet to the m4a1
  • attached pistol bayonet to the pistol
  • switched firing modes (auto, semi auto) when in firing mode using the m4a1
  • tested reach attack by spawning a blob and pressing tab while wielding my m4a1 2 tiles away
  • tested firing the m4a1 normally using semi-auto and full auto

no crashes or errors.

@ZhilkinSerg ZhilkinSerg merged commit f767f24 into CleverRaven:master Oct 30, 2019
@LetterShapedGlyphs LetterShapedGlyphs deleted the Gunmod-fix branch October 30, 2019 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Items: Gunmod / Toolmod Weapon and tool attachments, and add-ons Items / Item Actions / Item Qualities Items and how they work and interact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

non GUNMOD type gunmods with gunmod_data cannot be installed Combat Knife bayonet crashing
5 participants