battle calc bug with retreat after x rounds

[ZjelcoP]

My Operating System:

Windows 64

Engine version:

0.8304 at least up to 0.8659

Map name and version:

Any map

Can you describe how to trigger the error? (eg: what sequence of actions will recreate it?)

Start Battle Calc. Set retreat after X rounds. Average Attacker units left is completely off. Test 20 vs 20 tanks: Retreat after X Rounds: 1 - 2 - 3 - 4 - 5 Dice Units left: 20 - 18 - 9.5 - 5 - 3 Low Luck Units left: 20 - 20 - 20 - 15 - 3.5

Any additional information that may help:

Also posted here: https://forums.triplea-game.org/topic/563/possible-bug-with-battlecalc-since-0-8304-at-least https://forums.triplea-game.org/topic/520/battle-calc-bug-with-retreat

[DanVanAtta]

8304 was released not that long ago, this is a pretty big breakage. 8304 has been marked as a pre-release again, v7621 is available from website now. https://github.com/triplea-game/triplea/releases/tag/1.9.0.0.7621

Marking this as a release blocker, we should not mark a new latest until this is fixed.

[ron-murhammer]

@DanVanAtta Are you sure this works properly even in 7621?

[ssoloff]

Using the repro scenario described here, I see the same behavior on both 7621 and 8304. For example:

7621

8304

I went back to 3635, and it has significantly different behavior:

3635

Further testing shows that 7062 was the first stable release to exhibit the reported behavior. So unless we're willing to revert stable all the way back to 3635...?

[RoiEXLab]

@ssoloff @DanVanAtta + I'd argue this is a bug for a not-used-by-the-majority-of-people feature, so not game breaking.

[ron-murhammer]

Had a feeling that was the case. Let's stick with 8304 as I don't want to roll back that far. This only impacts a limited number of maps.

[prastle]

I think we should add @ZjelcoP to the testing group if he is interested 😄

[ZjelcoP]

@RoiEXLab @ron-murhammer Can only comment for myself, but I use the feature quite a lot, especially when playing Low Luck (strafing). Think for many it can be game breaking not to be able to calculate the possible outcomes of big stack battles (say NWO).

This only impacts a limited number of maps.

Is there any map in which it does work correctly? Doesn't seem to be map related to me.

@prastle Sure I'll help out testing. Put my battles with the AI to good use!

[RoiEXLab]

@ZjelcoP I'm not saying this doesn't need to be fixed, I'm just saying we shouldn't un-release versions because of that...

[prastle]

@ZjelcoP I rarely calc and never play NWO. I guess I use the force! 👍 I'll send ya the link in the forum thanks for the help!

[ZjelcoP]

@RoiEXLab Ah like that, get it now. Thx for clearing it up.

[DanVanAtta]

@DanVanAtta Are you sure this works properly even in 7621?

@ron-murhammer Looks like no - but I'm more confident in 7621 at this point than I am in 8304. 8304 never got more than one green light, it should never have been marked as a latest with just the one thumbs up. Our policy is to require multiple testers to give green light, not just one.

[DanVanAtta]

@ron-murhammer we also had a 2nd issue that impacts 8304, so even if it is not this, it's the other. That combined with not quite enough testing, it seems clear we should not have 8304 be a latest.

I'll note the benefit of constantly testing and releasing means we actually don't drop all that far back when we do.

[DanVanAtta]

@ssoloff @DanVanAtta

• I'd argue this is a bug for a not-used-by-the-majority-of-people feature, so not game breaking.

@RoiEXLab Battle calc is a very major feature in the game. Incorrect results can be very impactful. If it were just this it would be a tough call, we also had another bug in 8304 and so it does not seem that was such a good version.

Overall I do think there needs to be far less ceremony about going forward or backward in releases. We need more frequent testing and more upgrades, we've been suffering from not fixing our 💩 properly for a few months now, so much time between latest versions is bad as it means we can't stop regressing for even just a week or two.

[DanVanAtta]

@RoiEXLab recall we rolled back when we slowed battle calc down by about 30%, that is kittens compared to battle calc giving an incorrect result. We perhaps could use more standardization of what regressions we can live with and which ones we cannot.

[ron-murhammer]

@DanVanAtta Completely disagree. A significant BC slowdown impacts like every user and every time they use the calc vs this broken functionality for a max round setting that isn't used by most users or most calcs has been out for like 6 months and just being reported now. While I definitely think we should fix it, I don't think we should roll all the way back to say 3635.

Do you think we should revert back to 3635? Or something else before 7621?

[DanVanAtta]

@ron-murhammer we had another p1 out against that release, it became not a p1 only because we went to 7621. Is there any other problems in 7621? It's not a big deal, we will test latest and roll forward with hopefully not much time.

Regarding BC, I have not observed it running slower than 3s, typically it is about 1s. Adding 20% is not a huge impact, particularly when you can lower run counts. On the other hand, an incorrect result is broken. You may as well not run the calc if it tells you wrong result. First is correctness, then performance. Without correctness performance does not matter.

[ron-murhammer]

@DanVanAtta Play a large map vs the hard AI. Or play a 10 round game of TWW or WaW. Battle calc performance is in many cases more important than correctness. We actually had a 'more correct' casualty selection option which we decided to remove because it was too slow.

[prastle]

The simple truth is most rarely use battle calc. But when we do as Red pointed out, 60 sec to run it sucks. It does also need to be somewhat accurate.

[DanVanAtta]

The simple truth is most rarely use battle calc.

@prastle I'm really not sure about that, my observations have been otherwise. Regardless it is an important feature. At same time AI performance is also very important. The fact the two are one and the same is a (code/design) problem.

Play a large map vs the hard AI

During testing I noticed it is kinda broken @ron-murhammer : |

Or play a 10 round game of TWW or WaW

The actual calc is fast. I like WaW, my observation has that been the problem is that after round 15 the calc just loads too slow for a fast turn. The calc is used all the time for that map, it's game changing if the odds have shifted just slightly, with a 150+ unit stack, it can be hard to know sometimes and calc is key to not losing the game.

performance is in many cases more important than correctness

It's an interesting consideration @ron-murhammer , I think really the problem is that those use-cases were piggy-backed to the calculator. Some allowance can be made, but at some point it's frustrating to be frozen in the battle calculator because of other code that should have done something else. It may be worth considering forking a copy for the AI and then switching to a newer version one comes out. Otherwise in the forked version we can drop correctness.

Though a second point about correctness, it's one thing to be within 1% of the correct result - it's another when the result is completely wrong. Some approximations are fine for performance, it's a big problem though when the result is wrong

[OndisTripleA]

I wrote one of the posts that prompted this bugreport in the forum. I can confirm the error does not exist in 3627. Now what can you guys have been poking inbetween there to cause this?

[DanVanAtta]

@OndisTripleA for better or worse, quite a bit to poke. This should be fixed now courtesy Ron via #2970. (We introduced a regression while fixing #442, infrastructure units not retreating properly and dying instead)