triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.32k stars 392 forks source link

Battle Calc not producing correct result in old LotR map #9447

Open zlefin opened 3 years ago

zlefin commented 3 years ago

This is about the old lotr map, the one titled "Lord of the Rings: Middle Earth"

Reproduction Steps

map is lowluck by default, and that setting is used for this.

  1. Open up a battle calc screen for province Lorien
  2. set goblins to 10 stabbers, 10 shooters, 9 wargs, 2 trolls, 2 bats
  3. set elves to 6 hunters, 2 wizards, 3 towers, 4 fortifications
  4. click calculate odds

It yields: average rounds 3 which is incorrect. The first round goblins will inflict 8 cas, and elves 7, by default goblins will lose a mix of their infantry and end up with 41 pips worth of power, so goblins should inflict 6-7 cas, probably the 7, on round 2, thus killing off the elves. So the average rounds should be ~2.17 this also causes some of the other numbers like estimated tuv changes to be incorrect.

Based on trying differing mixes of troops, and setting battle to retreat after round 2 for some other test cases; I think the problem is that the battle calc sometimes uses an incorrect method of selecting the casualties, so that it does not optimize for the mix of shooters and stabbers which; whereas the actual default casualty selector does correctly optimize for that. The battle calc is not using the current default casualty selection rules; it seems to be using some older and less thorough one.

Engine Version

Engine Version: tested and verified to occur in both 2.5.22294 and 2.6.something (I'm having trouble finding an exact version number, it's one I cloned to use in IDEA a month or two ago)

Is there anything else we should know?

I'm gonna poke around the code a bit and see if I Can find something

zlefin commented 3 years ago

note: I might be misreading parts of the code, as it's in a style I'm not used to that makes it hard to read.

battle calculators main class at game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/BattleCalculator.java

the method used by the above class is called getUnitListByOrderOfLoss in game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/OrderOfLossesInputPanel.java

this one does not handle interleaving fully, it seems to just select the first one in an interleave pattern and then make all the others of the same type, rather than actually interleaving. ie if you need to take 4 casualties of an interleaved unit set of which you have 7 units of type A and 5 units of type B, and AB is a support/supportable pair, it will choose all 4 casualties as being of type A, rather than the optimal 3 of A and 1 of B

However this code does handle the choices players can input for choosing alternate casualty orders.

The code that selects casualties in the actual game and properly handles interleaving: sortUnitsForCasualtiesWithSupport in game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/casualty/CasualtyOrderOfLosses.java

solutions: (I'd like to hear thoughts on which solution would be best)

option 1: Rework getUnitListByOrderOfLoss to fully account for interleaving while also following explicitly specified loss orders. roughly to be a few or several hours of work to get correct.

option 2: a kludgy fix, if there are no explicit orders adjusting the order of loss, then have battle calc call sortUnitsForCasualtiesWithSupport if there are explicit orders adjusting the order of loss, then just call getUnitListByOrderOfLoss under this system, the battle calc will correctly handle cases with Interleaving, exclusive or, Explicit orders of loss. but it will not always accurately handle cases where both are present. The upside is that it would be very easy to code and thus much less work for a partial fix.

option 3? I'm not sure what the schedule is for various 3.0 stuff, and whether it would cover this area of the code or not. If this area is expected to be covered in some upcoming rework anyways, it may make more sense to simply wait until then so it all gets done together.