Update aiming [RDY]

[mugling]

Overview

Subset of #17712 that provides:

• A unit test validating player::gun_engagement_range
• R script for producing aiming graphs
• Initial algorithm changes

The below is produced using the R graphing script:

• Uses the S1 definition for a survivor (all stats 8, all skills 4) wearing light survivor gloves + mask.
• Weapons are loaded with default ammunition (9mm, 223 and 270)
• Effective range is defined as 50% chance of accuracy_goodhit
• Chance and accuracy scale linearly with range (at half the range you have twice the accuracy)

Initial status

• Aiming is irrelevant (slope of curve)
  a rifle scope reaches full aim after 2 turns and gains you only a single extra tile
• Weapon dispersion is irrelevant (y-intercept)
  a highly accurate rifle (σ=30) and an inaccurate pistol have (σ=380) differ by only two tiles
• Maximum range is both too low and too uniform (zenith of slopes)
  rifle with scope is only 50% better than pistol and both are useless apart from at point-blank range

Increase effect of aiming

• A 400% increase in MIN_RECOILto 600 results in a proportional increase in aiming time
  eg. turns required to achieve maximum aim with ar_15 increases by 450% from 1 to 4
• Initial range (y-intercept) is worse, maximum range (slope zenith) is unaffected
• Weapon aim_speed is irrelevant
  glock_19 has half the aim_speed (4) of ar15 (8) but their curves are identical

Aim speed differs by weapon

aim_speed has only trivial effect in Character::aim_per_time. The following implementation is both simpler and produces much better results.

int Character::aim_per_time( const item& gun, int recoil ) const {
    int aim = std::max( 1, 10 - gun.aim_speed( recoil ) ) * 3.2;
    return std::max( std::min( aim, recoil - gun.sight_dispersion( recoil ) ), 0 )
}

• Pistols and SMG's now aim faster than rifles
• Further improvements would require JSON changes (probably rescaling aim_speed)

Increase effect of weapon stats

• Player skills have too much effect
  from nil to maximum skill is Δσ=600, whereas from a pistol to a rifle is typically Δσ=300.
• markmanship has too little effect
  only 25% of player dispersion results from marksmanship

Considering few players will reach marksmanship 10 and even fewer will max out a ranged skill we need to reduce the effect whilst still granting a bonus for each successive level. The following works well reducing both the overall effect and making marksmanship twice as important as weapon skill:

int Character::skill_dispersion( const item& gun ) const
{
    if( weapon_lvl < 10 ) {
        dispersion += 5 * ( 10 - lvl );
    }
    if( marksmanship_lvl < 10 ) {
        dispersion += 10 * ( 10 - marksmanship_lvl );
    }
}

Drop excessive penalty from stats

• Stat penalties via ranged_dex_mod() and ranged_per_mod() were implemented when character generation encouraged Min-Maxing and most characters had stats of 12
• Currently an average survivor has a penalty of σ=120 with the lowest possible penalty being σ=90
• If we drop the penalty level to 8 a typical players doesn't receive a penalty except when subject to pain or other stat modifying effects.
• Later we could consider applying a non-uniform penalty so more difficult shots required better stats

Summary

• Although later JSON changes will be required the major problem is the algorithms. Fortunately the above fixes are not especially intrusive and significant gains can be made without rewriting the aiming subsystem.
• Rescaling of JSON data can be done once the above changes are made and after further unit tests are devised. These should specify the expected range for suitable test items under specific circumstances. Suggestions as to such tests are welcomed

[mugling]

Effectively this PR leaves effective_min unchanged but markedly increases effective_max:

engagement::effective_min

50% chance of good hit with no aiming

Item	before	after	change
Glock 19	3.1	2.9	-6.5%
MP5	3.3	3.1	-6.1%
AR-15	4.1	3.8	-7.3%
R700+scope	4.4	4.1	-6.8%

engagement::effective_max

50% chance of good hit at maximum aim

Item	before	after	change
Glock 19	3.2	5.1	+59%
MP5	3.6	6.0	+67%
AR-15	4.6	9.7	+110%
R700+scope	5.5	14.3	+160%

Further combinations could be tested by updating dump.cpp and then running
./cataclysm-tiles --dump-stats AIMING

[Coolthulhu]

Stat penalties via ranged_dex_mod() and ranged_per_mod() were implemented when character generation encouraged Min-Maxing and most characters had stats of 12

Rebalancing them around 8 instead of 12 only makes the characters blander: lowering dexterity and perception is now even more punishing than before.

It should be rebalanced around high numbers (14, 16 or 20) or removed completely.

[mugling]

@Coolthulhu the penalty remains unchagned at σ=15 per level so low DEX and PER are now less punishing; previously DEX 7 and PER 7 was a penalty of σ=150, now it is σ=30.

Whats changed is that DEX > 8 or PER > 8 no longer provide a bonus. Previously the system was 'balanced' for typical stats to be 12 and this assumption is no longer true. Before this PR all players using new character generation rules were guaranteed a penalty of at least σ=90.

The current check also ensures that pain or status effects that adjust stats reduce accuracy.

or removed completely.

The main priority for this PR is the graphing component and the changes to aiming so I'm not averse to dropping stat penalty altogether and coming back to it later if you are otherwise happy with the PR.

[mugling]

Latest push (94f6400) also includes partial fix for #17878

[Coolthulhu]

Whats changed is that DEX > 8 or PER > 8 no longer provide a bonus.

While 12 wasn't perfect, since it resulted in a threshold, it offered some sort of a direction.

Now you removed both aiming speed adjustment from stats and dispersion modifier from stats.
Meaning that:

• Perception is no longer relevant, except when stacking it for Niten. That dispersion adjustment protected it from becoming a total dump stat
• Dexterity has no effect on ranged combat
• The system, now balanced for default stats, will result in no more feedback from characters with subpar stats and will result in even more breaking apart due to pain. Pain is already a major problem with balance, since it mandates boring, conservative play, as stacked pain can easily take out even a strong character

[mugling]

Pain is already a major problem with balance, since it mandates boring, conservative play, as stacked pain can easily take out even a strong character

Lets go with removal then as that was my only reason for wanting to retain it. We can devise some better effects from stats in a later PR - possibly DEX should help with aiming speed and PER should somehow affect maximum range.

[Coolthulhu]

Taking out an entire stat is a major change.

I'd be fine with making dexterity not have much of a role in ranged combat, since it is already good enough in melee, but perception was nearly a dump stat even with the impact on ranged combat.

This is not a thing that can wait for a later PR.

I see the following options:

• Perception penalty is brought back as it was, the PR fulfills only part of its purpose, but doesn't make perception even more worthless
• Perception penalty is rebalanced here and taken into account. Preferably in such a way that it is both important and makes it a good idea to level perception above 12 (preferably capping at 20). For example, by scaling effective dispersion of all sights by (20 - perception) / 10.0.
• Some new mechanic is added to perception that makes it worth getting it. Must be actually useful and not just convenience. Must be added or at least PR'd before this PR is merged. Good example: nightvision range scaling factor, making Night Vision trait just count as 5 perception for purpose of NV. Bad example (ie. utility that isn't actually useful): something that helps identifying monsters, shows their exact hp, reveals their stats.

[mugling]

Good example: nightvision range scaling factor, making Night Vision trait just count as 5 perception for purpose of NV.

Whether or not PER has additional ranged effects that's a good idea. It should also affect overmap range and possibly sight range?

[Zireael07]

Good example: nightvision range scaling factor, making Night Vision trait just count as 5 perception for purpose of NV.

Does that mean perception affects NV in master?

[mugling]

Ready

[cainiaowu]

Why time instead of moves? will melee weapon also change to time?

[mugling]

Why time instead of moves?

It's far more intuitive than moves and isn't later broken if we adjust moves/turn (which has been previously discussed). Lots of other code expresses time as opposed to moves/turns for this reason.

will melee weapon also change to time?

Not as of this PR. Those values are typically of the same magnitude making comparison easier.

[mugling]

@Coolthulhu can we have a go at merge testing this. I have more code to commit but there is too work here to justify a further dependent branch

[mugling]

Also fixes #14938

[mugling]

Additionally fixes #14928

[Coolthulhu]

Not sure if caused here, but I can't aim at friends. Also, aiming at a non-friend monster then aiming at a friend causes a segfault.

[Coolthulhu]

In character creation, perception doesn't have any mechanical advantages listed. If it affects sight dispersion, it should say so there, since it doesn't seem to say it anywhere else.

[mugling]

Not sure if caused here, but I can't aim at friends. Also, aiming at a non-friend monster then aiming at a friend causes a segfault.

Existing bug

[mugling]

In character creation, perception doesn't have any mechanical advantages listed. If it affects sight dispersion, it should say so there, since it doesn't seem to say it anywhere else.

It has doxygen comments via ///EFFECT_PER but I'll add some to chargen

[Coolthulhu]

"aim_speed" is still used in some places, most notably the outdated blazemod, but also obsolete.json and one underbarrel mod. Blazemod doesn't need to be updated here as long as it won't produce debugmsgs on load/use, but core files should.

[mugling]

Blazemod doesn't need to be updated

Have updated everything else

[mugling]

In character creation, perception doesn't have any mechanical advantages listed. If it affects sight dispersion, it should say so there, since it doesn't seem to say it anywhere else.

It has doxygen comments via ///EFFECT_PER but I'll add some to chargen

Actually I might update chargen for all stats as a separate PR using the doxygen comments as a reference

[Coolthulhu]

I tested it, but as a significant mechanical change it still may have some problems, so look out for reports.
Would be good if you also made a forum account or at least scanned the forums once in a while.

[mugling]

Cross posts are appreciated but I have as of recent begun to skim for the recent posts section