[RDY] [CR] item: point to mod item (not the base firearm item) from mod's "gun mode"

[keyspace]

Fixes #20214.

Summary

PR #19515 correctly changed a second if to an else-if, but incorrectly pointed to the base firearm in the gun mode's constructor args EDIT: instead of leaving as it was - pointing to the mod item.

The doc line got me confused at first, requiring a bisect to confirm this pointer is the culprit. So, tried to clarify it.

Details

In current git master only bashing damage is applied (and displayed in item::info()). The bashing comes from the base gun's melee quality. The cutting/piercing of bayonets is added to referenced from the firearm's "gun modes" (the ones cycled with F), but damage from mods on the firearm isn't applied.

Happens because during gun_mode init, this is erroneously passed, which in the context of item::gun_all_modes() means the "base gun" item.

What the gun_mode struct constructor expects is the "part" from which the "gun mode" originates (base gun or some attached mod) - so that a mapping can be created:

• auto -> base gun
• single -> base gun
• bayonet -> attached bayonet mod

In current git master, the latter is instead:

• bayonet -> base gun

In theory, when item::damage_melee() is invoked, melee damage of all "gun modes" is considered, and then std::max_element() of them is returned.

However, in current git master, e.second.target != this && e.second.melee() (here) is always false (inspected manually), since there is never a gun_mode that doesn't have the base gun as its target. Therefore the fallback default of base gun's bashing damage is used.

This PR properly sets the "bayonet" gun_mode's target to the bayonet item, so that the cut/pierce damage is again available. It is my belief that this change in PR #19515 was erroneous - to achieve its goal of fixing #19492 only the addition of an else was required.

---

P.S. Perhaps target in gun_mode is somewhat confusingly named. Or the code lacks a comment here and there.

[Coolthulhu]

Are you sure that is the correct fix? I don't mean it isn't, it's just that I can't tell from the comments for the relevant structures what exactly are the fields supposed to mean.
If you decoded it, it could be a good idea to add comments regarding what is what in the structure (not necessary here).

[keyspace]

I'm pretty sure, but will add more doc comments in a second 20 minutes.

[keyspace]

That is, I'm pretty sure this is the correct fix for issue #20214. I've tried firing on a few targets, to check see if issue #19492 (that PR #19515 addressed) would reappear. Didn't get a debugmsg().

Actually, I'll test some more, to make sure modes don't change "by themselves", or some other weirdness doesn't happen again.

---

EDIT: clarify

[keyspace]

Firing an AK-74M in "auto" mode does not always result in the maximum 8 shots. Not sure of the cause - not enough moves? Distance to target? Target dies before burst over? (I've not used automatic mode enough to know.) Seems to be "pre-existing" - tried on git master, get about the same.

Didn't notice anything else unexpected.

[keyspace]

Re-read forum post. Got wondering:

I think the ability to use bayonets in hand to hand combat was removed in favor of reach combat.

#15034 mentions this - but it's not yet implemented; there's also PR #11665... Need to review - didn't really check if only one of the damage types is applied, namely cut/pierce of the bayonet, but not bashing damage. EDIT: Was confused, see discussion below.

[Coolthulhu]

I specifically did test that and it looks like only bashing is applied.
It doesn't really make sense to special-case it like that. From balance side, bayonets are underpowered. From realism side, spears are some of the most versatile weapons in terms of reach and rifle+bayonet is essentially a spear.

[keyspace]

I mean after applying this PR.

Started writing detailed description of what has been changed here - will move to preample preamble.

What is the expected behaviour? As I understand, the intent of current code was to apply bayonet's damage when "fired" in reach mode, and highest possible of melee damages from either the base gun (bashing) or any of the mods (cut/pierce).

[Coolthulhu]

The intent is using the best damage of each type.
The old behavior was that gun+bayonet deals as much damage as bashing with the gun and stabbing with the bayonet at the same time. For as long as this doesn't get better than dropping the rifle and shanking with a knife, there is no reason to nerf that further.

There is an assumption that no gun will have both cutting and piercing damage at the same time, but there is no need to enforce that at the moment.

[keyspace]

Checked in this PR - whether using the bayonet for a reach attack or regular next-tile melee, both bash and cut are applied. E.g., a AK-74M with attached sword bayonet deals ~11 bash and ~25 cut for an all-skills 5 and STR 11.

It is unexpected to me, but that's tangent.

The old behavior was that gun+bayonet deals as much damage as bashing with the gun and stabbing with the bayonet at the same time. For as long as this doesn't get better than dropping the rifle and shanking with a knife, there is no reason to nerf that further.

Okay. Then it works as expected?

Tested by adding:

add_msg( "DEBUG b: %i c: %i p: %i",
         dealt_dam.type_damage( DT_BASH ),
         dealt_dam.type_damage( DT_CUT ),
         dealt_dam.type_damage( DT_STAB ) );

at this line.

[Coolthulhu]

Checked in this PR - whether using the bayonet for a reach attack or regular next-tile melee, both bash and cut are applied. E.g., a AK-74M with attached sword bayonet deals ~11 bash and ~25 cut for an all-skills 5 and STR 11.

Looks OK.
Though the giant discrepancy between bash and cut damage is pretty alarming. After all the buffs bashing has gotten, it still sucks.