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.29k stars 381 forks source link

Casualty selection problem in 2.6+ #12672

Closed WCSumpton closed 1 day ago

WCSumpton commented 1 week ago

Please review:

@zlefin describes a problem in selecting the default casualties, and is demonstrated in bug1.zip.

The player cannot select 3 units to receive one hit and one of those units to receive a second hit. In 2.5 the player is able to select the default casualties.

Cheers...

asvitkine commented 2 days ago

Looks like it's caused by this code:

    casualtyDetails.ensureUnitsAreDamagedFirst(
        sortedTargetsToPickFrom,
        Matches.unitIsAir(),
        Comparator.comparing(Unit::getMovementLeft).reversed());

Which results in the damaged list to no longer have the duplicate unit list.

asvitkine commented 2 days ago

So looks like the problem was introduced by: https://github.com/triplea-game/triplea/commit/20835c42b272bcacb3adc6a175e038015948ea2e

What happens is that redistributeHits() ends up overriding what the player chose (i.e. 2 dragons taking 1 dmg and 1 dragon taking 2 dmg) and instead changing it internally to (2 dragons taking 2 dmg each). But then in ensureUnitsAreDamagedFirst(), the logic does this:

    damaged.addAll(
        targetsHitWithCorrectOrder.stream()
            .filter(unit -> !damaged.contains(unit))
            .collect(Collectors.toList()));
    damaged.removeIf(matcher.and(not(targetsHitWithCorrectOrder::contains)));

The first is a no-op, because damaged already contains the two dragons that redistributeHits() chose. (But doesn't add any new entries, so we don't get the 2nd entry for the dragon that redistributeHits() decided to give 2 hits to.) But the second line ends up removing the third dragon that now isn't in the list.

I think this logic is just wrong when considering the >= 3 hp case. redistributeHits() should not be changing the user's selection here in terms of how many 3->2 and 2->1 hits were chosen.

@RaiNova - any chance you're around to look at fixing this?