CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
9.99k stars 4.09k forks source link

Gun accuracy calculation broken #21244

Closed Firestorm01X2 closed 6 years ago

Firestorm01X2 commented 7 years ago

Players reporting of massive reducing in firearm accuracy. Forum thread: http://smf.cataclysmdda.com/index.php?topic=14523.0

Looks like we have massive regression after that: https://github.com/CleverRaven/Cataclysm-DDA/pull/20941

This issue should be set as milestone to 0.D and probably marked as "Priority" because shooting is one of the core mechanics of the game.

kevingranade commented 7 years ago

Need expected behavior vs actual behavior, i.e. how accurate do you expect it to be?

Leland commented 7 years ago

Was #20941 expected to change accuracy? And if so, do we know to what degree it was changed?

I ask, as leading the discussion with a breakdown of how accurate it was prior to that PR vs. how accurate it is now should allow for a reasonable approach to how accurate firearms should be.

kevingranade commented 7 years ago

I did not expect #20941 to strictly maintain the previous level of accuracy, no.

There was no standard for accuracy in the first place, so while there might have been a change, you can't really call it a regression.

Again, give me a scenario, an expected result, and an actual result, and we can get this party started. Otherwise it's just going to go in circles indefinitely.

John-Candlebury commented 7 years ago

Just for reference #18943 (EDIT:mb I linked the wrong one) has the pre-revert unit tests. Which would serve as the previous standards players are comparing current dispersion against.

Someone would need to recreate them for the actual state of the game and make the comparison however.

kevingranade commented 7 years ago

Yea that's one if the incredibly frustrating things about that code. That unit test doesn't actually do anything. It added a "determine engagement range" method and tested that, not the actual accuracy of the weapons.

You may notice the test doesn't make any assertions about effective range or accuracy, because it isn't testing that, just that the function itself has certain properties.

AlecWhite commented 7 years ago

Again, give me a scenario, an expected result, and an actual result, and we can get this party started.

What do you mean by a scenario? Something like player skill/stats/modifier, range, gun stats, target skills/stats/modifier and whatever variables are needed to get the a shooting result?

I'm sorry, I don't understand what is the current problem with guns, only that they work "wrong". And if the shooting target formula is decide, and the code works, it would only need the tweaking of how much each variable weights for a successful shot.

kevingranade commented 7 years ago

scenario

yep, what you said.

only that they work "wrong".

People keep saying it's "wrong", but won't or can't say what's "right". If we don't have a way to check whether it works correctly, how do you expect to fix it?

Coolthulhu commented 7 years ago

Yea that's one if the incredibly frustrating things about that code. That unit test doesn't actually do anything. It added a "determine engagement range" method and tested that, not the actual accuracy of the weapons.

That method was a great measure of actual accuracy - simple, reliable, consistent, meaningful. It probably can't be accurately recreated for the much less natural sum of uniformly distributed variables, but if it could, it would be good enough.

The problem with the current system is not the numbers, it's the distributions and the way they're combined. Summing non-negative, uniformly distributed variables must weight the system towards expert, naturally gifted marksmen, creating a situation where guns require a lot of training to be useful and reward going from "good" to "a bit better than good" more than going from "average" to "competent". In most cases, Joe Average will be better off practicing with a knife than using a gun, even if the gun was fully silent.

But for the scenarios: I can't think of a better measure than just recreating the old tests, except by adding some extra upper caps to prevent the system from blowing up for skilled marksmen. Those tests already exist, are balanced, are consistent with what players were used to, can be understood without even learning what dispersion is in the current revision. The only thing they don't do is testing the implementation details - but I don't think they should even test that.

kevingranade commented 7 years ago

Those tests already exist

I'd like you to point them out, because I couldn't find them. I believe this is where they were supposed to go: https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L190

To break down the rest of that file:

This test: https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L147 merely asserts that for a given aim level and player/weapon/target, projectile_attack_roll() agrees with projectile_attack_chance().

This one: https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L34 asserts that for all guns at selected aim level/ player skill / range, gun_current_range() agrees with projectile_attack_chance()

This one https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L64 and this one https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L72 assert that a special overload of gun_engagement_range() reports a longer effectve range for higher aim levels.

This one https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L80 asserts that gun_effective_range() reports a longer range when dispersion is lower.

This one https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L93 and this one https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L108 assert that gun_current_range() scales range based on requested accuracy level.

This one https://github.com/CleverRaven/Cataclysm-DDA/pull/20941/files#diff-ae9fb308eca169e424e32c565235db95L108 asserts that all else being equal, smaller targets are harder to hit.

Except for the last test, they are all testing the methods themselves, not anything about game data. For the most part they're checking for consistency between two methods, which doesn't demonstrate correctness, just that if errors exist, they exist in both methods. None of these construct a scenario and assert an outcome.

Coolthulhu commented 7 years ago

OK, looks like I remembered wrong then - my bad. The dump.cpp data was compared manually, there was no complete regression test.

Still, the values themselves are good: https://github.com/CleverRaven/Cataclysm-DDA/pull/18020 - very good analysis of the math involved, with good sample numbers for a relatively broad set of weapons, at 3 different character skill levels Just approximating those numbers would be a really good starting point.

PlasmaChroma commented 7 years ago

From my current testing/play of the new system, I'd say what feels "wrong" to me is that skill levels seem to not have as big an impact as one would hope. Both gun-specific and general Marksmanship slightly move the actual performance in the players favor. Probably somewhere between 30% to 80% more from that would feel better.

Although in terms of balance, getting the actual skill levels is unnatural as well. If you're trying to train weapons via practice it seems to take excessive amount of time to skill up anywhere high. Whereas a lucky character can just read a few books or train via NPC and "instantly" be as good a shot as a player who has killed countless hordes over months and months of time to gain the exact same skill. The actual impact of skills in the case where they are gained "cheaply" seems like it would have about the improvement that it does now. But a hardened veteran with real combat experience is under appreciated.

Perception also helps a bit here, although I imagine a lot of players are likely running low-ish perception characters, at least I always have until now. Typically I wouldn't push it past 10, but on my latest run I'm experimenting with a natural 14 to see how big the impact is.

To just ballpark it, I'd think at 25 yards with a decent rifle and iron sights you would expect to hit fairly often even with low-ish skills and some steadying against normal sized target.

Coolthulhu commented 7 years ago

From my current testing/play of the new system, I'd say what feels "wrong" to me is that skill levels seem to not have as big an impact as one would hope.

They do have an effect on accuracy, it's just that now it's only really visible near level 10, with great equipment and high perception.

JacobKessler commented 7 years ago

I posted these in the forum thread and, while I didn't get any comment on the proposed scenarios, they seem reasonable to me. They obviously aren't exhaustive, but are at least a start toward some tests:

kevingranade commented 7 years ago

18020 - There are literally over a hundred comments in that PR, "do what that novel over there says" is not a valid bug report, especialy when various parts of the comments almost certainly contradict each other.

@JacobKessler that's the kind of thing I'm looking for, thank you.

guns 0 without steadying effectively cannot hit.

That's certainly testable, but I'm not totally sure it's accurate, what about a point-blank sawed-off shotgun blast? I could see just excepting e.g. spread weapons and calling it a day though. Actually lets just assume spread weapons aren't covered in general.

guns 10 without steadying shouldn't be able to hit often outside of 5 tiles, regardless of weapon

Needs to be a bit more specific, what's "often"? (1 time out of 10 would be a testable example). This scenario roughly matches effective quick-draw range for competitive shooters, so something like https://www.cowboyfastdraw.com/images/stories/pdf/Forms/GunslingersRules&Guidelines_9th_Final_11-28-16.pdf tl;dr page 19: 24" targets at 21', 17-3/16" target at 15' Could use a little more figuring, but after scaling that sounds reasonable-ish.

guns 5 should make shots 'effective', meaning a 75% chance of hitting at mid-steadiness

This middle part is SUPER arbitrary, for one thing, why mid-steadiness? Especially for the first batch of tests, its seems more productive to lock down the limits rather than test scenarios in the middle.

guns 10 with a single '.' of aiming should be able to reliably hit within 'close range', 5-7 tiles

It doesn't make much sense for this to be uniform across all guns. In the extreme case you're saying that the worst gun in the game can hit reliably at a range of 7 (~25') with a single aim action (which is itself fairly arbitrary). It makes sense to put limits on the best guns can be, but limiting how bad they can be doesn't make much sense.

guns 10 with mid-steadiness should give effective ranges of ~50% more than guns 5

Let's not get into scaling and distributions until we have limits locked down.

guns 10, with scope, a decent rifle, and max steadiness, should have a 75% hit range of at least 60 tiles.

Define "decent" :/ Also "at least" is pointless here since you can't shoot further than that. Otherwise though, sure, it makes sense to have a test around some whitelist of endgame-tier rifles being able to shoot things at the edge of the map.

Coolthulhu commented 7 years ago

OK then, breakdown:

Let's define 3 characters:

Novice character checks the lower bounds. Competent character and novice together ensure that the lower levels have sensible numbers. God character is only there to make sure the numbers don't blow up, meaning adequate formulas are used. For example, "5 dispersion for every point below cap" is a very bad formula for uniform variable summation, and the god character ranges should catch that.

In case you want to balance it with per-gun settings (you proposed something like this a ~year ago), let's also check different guns:

If not, just use the rifle numbers for everything. This will buff everything, but guns are currently very bad, so it's better to overbuff (except for free ammo ones, like pneumatics, and for endgame characters) than to nerf.

Character >50% good <10% good
Novice pistol 3 5
Novice SMG 5 8
Novice rifle 8 15
Competent pistol 7 10
Competent SMG 10 15
Competent rifle 15 45
God pistol 10 15
God SMG 20 30
God rifle 30 60

The numbers I picked were selected so that:

Firestorm01X2 commented 7 years ago

This issue should be set as milestone to 0.D. It will be bad to forget to fix this.

Firestorm01X2 commented 6 years ago

Temporary fix (fork containing non bugged aim) by Coolthulhu: http://smf.cataclysmdda.com/index.php?topic=14767.0 https://github.com/Coolthulhu/Cataclysm-DDA/releases/ Use that until it will be fixed in this repository.

kevingranade commented 6 years ago

That's a fork, not a fix. A fix is in progress at #21468

ghost commented 6 years ago

It's a fork containing a fix.

Coolthulhu commented 6 years ago

Note: the actually implemented tests and balance standards are significantly less strict than the ones I proposed. The guns should be useful now, but this doesn't mean they are balanced against other options or against each other.

Notable balance problems are: rifles are still just plain better than SMGs and pistols, "endgame" characters can easily reliably snipe much farther than they can see, perception got less useful despite already being much worse than dexterity.

TeMPOraL commented 6 years ago

Are those numbers based on gut feelings and back-of-the-napkin math, or did you have a mathematical model for them? It would be great if there was a well-defined one, which could later be enshrined in unit tests as well to safeguard against future balance-breaking changes.

Firestorm01X2 commented 6 years ago

Likely it is "game feeling by personal experience". But digits looks nice and there are no other balance outline so that is it. It was accepted ofically.

And unit tests already were impelemented here: https://github.com/CleverRaven/Cataclysm-DDA/pull/21468 Tests just were implemented not completely corectly based on that design.

kevingranade commented 6 years ago

As Coolthulu stated, the numbers are based on a number of game balance goals.

The only input from reality is the quickdraw based tests, whose upper bounds are based on the records established by actual quickdraw competitions.

Please open or comment on current issues for future discussion.