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

Critical hit coverage #52060

Merged
merged 6 commits into from Oct 14, 2021
Merged

Conversation

dseguin
Copy link
Member

@dseguin dseguin commented Oct 3, 2021

Summary

None

Purpose of change

This is part of a more comprehensive coverage balance. This adds coverage values for additional categories: melee, ranged and vitals. Certain armour pieces would be more effective at protecting vitals (a.k.a. protection from critical hits).

Describe the solution

Currently, monster attacks do not generate critical hits. Characters can crit using either melee attacks or projectile attacks. The implementation of critical hits is a bit scattered around the codebase, so some minor adjustments were made:

  • The target body part of a melee attack is determined after rolling damage. This was moved into the damage roll.
  • Crit mitigation reduces additional damage from critical hits, which affects:
    • projectile_attack_results::damage_mult for projectile attacks (see Creature::select_body_part_projectile_attack())
    • Bash, cut, and stab multipliers and armour penetration for melee attacks (see Character::roll_all_damage() and sub-functions)

The optional fields "cover_melee", "cover_ranged" and "cover_vitals" are used to affect the damage received from different sources.

    "armor": [
      {
        "covers": [ "torso" ],
        "coverage": 95,
        "cover_melee": 80,
        "cover_ranged": 50,
        "cover_vitals": 5,
        "encumbrance": [ 1, 2 ]
      },
      {
        "covers": [ "arm_l", "arm_r" ],
        "coverage": 50,
        "cover_melee": 45,
        "cover_ranged": 30,
        "cover_vitals": 5,
        "encumbrance": [ 1, 1 ]
      },
      {
        "covers": [ "leg_l", "leg_r" ],
        "coverage": 80,
        "cover_melee": 70,
        "cover_ranged": 45,
        "cover_vitals": 5,
        "encumbrance": [ 1, 2 ]
      }
    ],

iteminfo_cover2
armor_menu2

TODO:

  • Read new coverage fields into armor_portion_data
  • Display extra coverage values in-game
  • Process the correct type of coverage depending on the damage/effect
  • Reduce critical hit damage based on vitals coverage
  • Update coverage values in json This should be left to the experts
  • Fix tests in iteminfo_test.cpp to accomodate the new format

The following areas of code may be affected:

Source file Areas affected
armor_layers.cpp coverage display information
character.cpp infections from filthy clothes
character_armor.cpp damage absorption
creature.cpp new function get_crit_factor() to multiply crit effects, crit modifiers in select_body_part_projectile_attack()
dump.cpp output extra armour fields
item.cpp coverage display info, get_coverage() and get_avg_coverage()
item.h new item::cover_type enum for identifying coverage type
item_factory.cpp process new fields from json
itype.h new coverage fields in armor_portion_data
melee.cpp crit modifiers in roll_<type>_damage() functions
iteminfo_test.cpp coverage info display tests

Describe alternatives you've considered

Critical hit protection could also be derived from the clothing's materials.

Testing

  1. Spawn dozens of NPCs, make both hostile and friendly
  2. Spawn dozens of monsters of different factions
  3. Observe hits:
  • Character -> Character (crit mitigation based on "cover_vitals")
  • Character -> monster (no crit mitigation)
  • monster -> Character (no critical hits)
  • monster -> monster (no critical hits)

Additional context

@Venera3
Copy link
Contributor

Venera3 commented Oct 3, 2021

Unless I missed something major critical hits are a strictly player-only (or character-only at most) system, monsters can't crit at all. Including a vitals coverage is at least futureproofing for when we do wounds/vital hits, but until then it will be of limited relevance.

Also, what's the melee/ranged split supposed to represent?

@dseguin
Copy link
Member Author

dseguin commented Oct 3, 2021

Unless I missed something major critical hits are a strictly player-only (or character-only at most) system, monsters can't crit at all

Thanks for confirming, I though I was going crazy! For now, crit mitigation will only be applied to hits from Characters like you suggest, but I suspect that monster crits will be a thing very soon.

Also, what's the melee/ranged split supposed to represent?

@I-am-Erk would have more details about this, but I figure that different armour pieces would effectively cover you differently based on the kind of hit you're taking. Right now it affects armour damage absorption and infection risk from hits to filthy clothing, but also different effects could be added based on different coverage types.

@dseguin dseguin marked this pull request as ready for review October 4, 2021 03:30
@dseguin dseguin changed the title [WIP] Critical hit coverage Critical hit coverage Oct 4, 2021
@kevingranade
Copy link
Member

Gunplay and the newer throwing attacks monsters can make detour through the player code, and can indeed trigger critical hits.

Melee attacks are handled directly in the monster code and do not trigger crits for now, but lacking a way to mitigate them is precisely the reason they aren't there...

@dseguin
Copy link
Member Author

dseguin commented Oct 4, 2021

Gunplay and the newer throwing attacks monsters can make detour through the player code, and can indeed trigger critical hits.

I see that monster::deal_projectile_attack() basically wraps Creature::deal_projectile_attack(). That's convenient because all the crit handling is taken care of there for projectiles.

Melee attacks are handled directly in the monster code and do not trigger crits for now, but lacking a way to mitigate them is precisely the reason they aren't there...

I think it would make sense to do something similar for melee attacks: migrate Character::melee_attack_abstract() to Creature::deal_melee_attack() and have monster::deal_melee_attack() wrap it. Maybe something for a future PR...

@Maleclypse Maleclypse added Melee Melee weapons, tactics, techniques, reach attack Ranged Ranged (firearms, bows, crossbows, throwing), balance, tactics labels Oct 5, 2021
@I-am-Erk
Copy link
Contributor

I-am-Erk commented Oct 6, 2021

I want to make sure before diving into review... You list a lot of things that could be affected, but I would think that as long as "coverage" the base stat is unchanged, adding more stats that are independent of it shouldn't have any impact on stuff like wind protection

@dseguin
Copy link
Member Author

dseguin commented Oct 6, 2021

That's right, those are just areas that make use of coverage in general. I made that list before I started on this to figure out the scope of coverage. I'll update the list to avoid confusion.

@Night-Pryanik
Copy link
Contributor

There is a lot of space in Armor Sort window. Please expand all these M, R, and V, they could be confusing for players.

@dseguin
Copy link
Member Author

dseguin commented Oct 12, 2021

I expanded the coverage names in the armour menu. Hopefully this is more clear:
armor_menu

@kevingranade kevingranade merged commit 452a0c6 into CleverRaven:master Oct 14, 2021
@dseguin dseguin deleted the crit_protection branch November 7, 2021 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Melee Melee weapons, tactics, techniques, reach attack Ranged Ranged (firearms, bows, crossbows, throwing), balance, tactics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants