Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue a cause of "Could not find step name" (2.6 bug). #11773

Merged
merged 11 commits into from
Jul 15, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,7 @@ private List<Unit> removeNonCombatants(
final boolean attacking,
final boolean removeForNextRound) {
int battleRound = (removeForNextRound ? round + 1 : round);
// Note: Also done in FiringGroupSplitterGeneral when determining step names. They must match.
return CollectionUtils.getMatches(
units,
Matches.unitCanParticipateInCombat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,28 @@ public enum Type {

@Override
public List<FiringGroup> apply(final BattleState battleState) {
final Collection<Unit> enemyUnits =
CollectionUtils.getMatches(
battleState.filterUnits(ALIVE, side.getOpposite()),
PredicateBuilder.of(Matches.unitIsNotInfrastructure())
.andIf(side == DEFENSE, Matches.unitIsSuicideOnAttack().negate())
.andIf(side == OFFENSE, Matches.unitIsSuicideOnDefense().negate())
.build());

// Filter participants (same as is done in MustFightBattle.removeNonCombatants()), so that we
asvitkine marked this conversation as resolved.
Show resolved Hide resolved
// don't end up generating combat step names for units that will be excluded.
final Predicate<Unit> canParticipateInCombat =
Matches.unitCanParticipateInCombat(
side == OFFENSE,
battleState.getPlayer(OFFENSE),
battleState.getBattleSite(),
1,
enemyUnits);
final Collection<Unit> canFire =
CollectionUtils.getMatches(
battleState.filterUnits(ACTIVE, side),
PredicateBuilder.of(getFiringUnitPredicate(battleState))
.and(canParticipateInCombat)
// Remove offense allied units if allied air can not participate
.andIf(
side == OFFENSE
Expand All @@ -68,14 +86,6 @@ public List<FiringGroup> apply(final BattleState battleState) {
Matches.unitIsOwnedBy(battleState.getPlayer(side)))
.build());

final Collection<Unit> enemyUnits =
CollectionUtils.getMatches(
battleState.filterUnits(ALIVE, side.getOpposite()),
PredicateBuilder.of(Matches.unitIsNotInfrastructure())
.andIf(side == DEFENSE, Matches.unitIsSuicideOnAttack().negate())
.andIf(side == OFFENSE, Matches.unitIsSuicideOnDefense().negate())
.build());

final List<FiringGroup> firingGroups = new ArrayList<>();

final List<TargetGroup> targetGroups = TargetGroup.newTargetGroups(canFire, enemyUnits);
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import static games.strategy.triplea.Constants.SUB_RETREAT_BEFORE_BATTLE;
import static games.strategy.triplea.Constants.TRANSPORT_CASUALTIES_RESTRICTED;
import static games.strategy.triplea.Constants.WW2V2;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand All @@ -30,9 +32,11 @@
import games.strategy.engine.data.ResourceList;
import games.strategy.engine.data.TechnologyFrontier;
import games.strategy.engine.data.Territory;
import games.strategy.engine.data.UnitType;
import games.strategy.engine.data.UnitTypeList;
import games.strategy.engine.data.properties.GameProperties;
import games.strategy.triplea.delegate.TechTracker;
import java.util.List;
import java.util.Set;

public class MockGameData {
Expand Down Expand Up @@ -102,6 +106,11 @@ public MockGameData withTechnologyFrontier() {
return this;
}

public MockGameData withLenientProperties() {
lenient().when(gameProperties.get(anyString(), anyBoolean())).thenReturn(false);
return this;
}

public MockGameData withTransportCasualtiesRestricted(final boolean value) {
when(gameProperties.get(TRANSPORT_CASUALTIES_RESTRICTED, false)).thenReturn(value);
return this;
Expand Down Expand Up @@ -189,4 +198,14 @@ public MockGameData withLowLuck(final boolean value) {
when(gameProperties.get(LOW_LUCK, false)).thenReturn(value);
return this;
}

public MockGameData withUnitTypeList(final List<UnitType> types) {
UnitTypeList unitTypeList = new UnitTypeList(gameData);
for (var unitType : types) {
lenient().when(unitType.getData()).thenReturn(gameData);
unitTypeList.addUnitType(unitType);
}
when(gameData.getUnitTypeList()).thenReturn(unitTypeList);
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import static games.strategy.triplea.delegate.battle.BattleState.Side.OFFENSE;
import static games.strategy.triplea.delegate.battle.FakeBattleState.givenBattleStateBuilder;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenAnyUnit;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitFirstStrikeSuicideOnAttack;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitFirstStrikeSuicideOnDefense;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenSeaUnitFirstStrikeSuicideOnAttack;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenSeaUnitFirstStrikeSuicideOnDefense;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
Expand Down Expand Up @@ -34,8 +34,8 @@ public class RemoveFirstStrikeSuicideTest {
void suicideUnitsRemoved() {
when(delegateBridge.getDisplayChannelBroadcaster()).thenReturn(mock(IDisplay.class));

final List<Unit> attackers = List.of(givenAnyUnit(), givenUnitFirstStrikeSuicideOnAttack());
final List<Unit> defenders = List.of(givenAnyUnit(), givenUnitFirstStrikeSuicideOnDefense());
final List<Unit> attackers = List.of(givenAnyUnit(), givenSeaUnitFirstStrikeSuicideOnAttack());
final List<Unit> defenders = List.of(givenAnyUnit(), givenSeaUnitFirstStrikeSuicideOnDefense());
final MockGameData gameData = MockGameData.givenGameData();
final BattleState battleState =
givenBattleStateBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import static games.strategy.triplea.delegate.battle.FakeBattleState.givenBattleStateBuilder;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenAnyUnit;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitCanNotBeTargetedBy;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenSeaUnitCanNotBeTargetedBy;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitDestroyer;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitIsAir;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -35,7 +35,7 @@ static List<Arguments> stepName() {
"Attacker has air units and no destroyers vs Defender subs",
givenBattleStateBuilder()
.attackingUnits(List.of(givenAnyUnit(), givenUnitIsAir()))
.defendingUnits(List.of(givenUnitCanNotBeTargetedBy(mock(UnitType.class))))
.defendingUnits(List.of(givenSeaUnitCanNotBeTargetedBy(mock(UnitType.class))))
.build(),
true),
Arguments.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import static games.strategy.triplea.delegate.battle.FakeBattleState.givenBattleStateBuilder;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenAnyUnit;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitCanNotBeTargetedBy;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenSeaUnitCanNotBeTargetedBy;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitDestroyer;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitIsAir;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -34,7 +34,7 @@ static List<Arguments> stepName() {
Arguments.of(
"Defender has air units and no destroyers vs Attacker subs",
givenBattleStateBuilder()
.attackingUnits(List.of(givenUnitCanNotBeTargetedBy(mock(UnitType.class))))
.attackingUnits(List.of(givenSeaUnitCanNotBeTargetedBy(mock(UnitType.class))))
.defendingUnits(List.of(givenAnyUnit(), givenUnitIsAir()))
.build(),
true),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@
import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import games.strategy.engine.data.GameData;
import games.strategy.engine.data.GamePlayer;
import games.strategy.engine.data.Unit;
import games.strategy.engine.data.UnitType;
import games.strategy.triplea.attachments.UnitAttachment;
import games.strategy.triplea.delegate.battle.steps.fire.FiringGroup;
import java.util.List;
import java.util.Set;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
Expand All @@ -40,6 +40,16 @@ class FiringGroupSplitterGeneralTest {
@Mock GamePlayer attacker;
@Mock GamePlayer defender;

@BeforeEach
void setUp() {
final GameData gameData = new GameData();

lenient().when(attacker.getName()).thenReturn("attacker");
lenient().when(attacker.getData()).thenReturn(gameData);
lenient().when(defender.getName()).thenReturn("defender");
lenient().when(defender.getData()).thenReturn(gameData);
}

@Test
void oneFiringUnitVsOneTargetableUnitMakesOneFiringGroup() {
final Unit targetUnit = givenAnyUnit();
Expand Down Expand Up @@ -216,14 +226,6 @@ void doNotExcludeUnitsOfAlliesIfAlliedAirIndependentIsFalseButItIsDefense() {
assertThat(firingGroups.get(0).getFiringUnits(), contains(fireUnit, fireUnit2));
assertThat(firingGroups.get(0).getTargetUnits(), contains(targetUnit));
assertThat(firingGroups.get(0).isSuicideOnHit(), is(false));

verify(
fireUnit,
never()
.description(
"Units on defense with AlliedAirIndependent == false"
+ "should never call getOwner"))
.getOwner();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import static games.strategy.triplea.delegate.battle.BattleState.Side.OFFENSE;
import static games.strategy.triplea.delegate.battle.FakeBattleState.givenBattleStateBuilder;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenAnyUnit;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitCanEvadeAndCanNotBeTargetedByRandomUnit;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenSeaUnitCanEvadeAndCanNotBeTargetedByRandomUnit;
import static games.strategy.triplea.delegate.battle.steps.BattleStepsTest.givenUnitIsAir;
import static games.strategy.triplea.delegate.battle.steps.MockGameData.givenGameData;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -90,15 +90,15 @@ static List<Arguments> stepName() {
Arguments.of(
"Attacking evaders vs ALL air",
givenBattleStateBuilder()
.attackingUnits(List.of(givenUnitCanEvadeAndCanNotBeTargetedByRandomUnit()))
.attackingUnits(List.of(givenSeaUnitCanEvadeAndCanNotBeTargetedByRandomUnit()))
.defendingUnits(List.of(givenUnitIsAir(), givenUnitIsAir()))
.build(),
true),
Arguments.of(
"Defending evaders vs ALL air",
givenBattleStateBuilder()
.attackingUnits(List.of(givenUnitIsAir(), givenUnitIsAir()))
.defendingUnits(List.of(givenUnitCanEvadeAndCanNotBeTargetedByRandomUnit()))
.defendingUnits(List.of(givenSeaUnitCanEvadeAndCanNotBeTargetedByRandomUnit()))
.build(),
true));
}
Expand Down