Skip to content

Commit

Permalink
Add a test for "requires units" for sea and cleanup code. (#12385)
Browse files Browse the repository at this point in the history 
No logic changes in this CL.

Code cleanup consists of inverting a boolean param for better readability and adding a helper function.
  • Loading branch information
@asvitkine
asvitkine authored Mar 3, 2024
1 parent ce22bf4 commit 4c5c6bf
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 39 deletions.
5 changes: 5 additions & 0 deletions game-app/game-core/src/main/java/games/strategy/triplea/delegate/AbstractDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import games.strategy.engine.data.GameData;
import games.strategy.engine.data.GamePlayer;
import games.strategy.engine.data.properties.GameProperties;
import games.strategy.engine.delegate.IDelegate;
import games.strategy.engine.delegate.IDelegateBridge;
import games.strategy.net.websocket.ClientNetworkBridge;
Expand Down Expand Up @@ -88,4 +89,8 @@ public IDelegateBridge getBridge() {
protected GameData getData() {
return bridge.getData();
}

protected GameProperties getProperties() {
return getData().getProperties();
}
}
69 changes: 32 additions & 37 deletions game-app/game-core/src/main/java/games/strategy/triplea/delegate/AbstractPlaceDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,9 @@ public PlaceableUnits getPlaceableUnits(final Collection<Unit> units, final Terr
unitsCanBePlacedByThisProducer = new ArrayList<>(unitsLeftToPlace);
} else {
unitsCanBePlacedByThisProducer =
(Properties.getUnitPlacementRestrictions(getData().getProperties())
(Properties.getUnitPlacementRestrictions(getProperties())
? CollectionUtils.getMatches(
unitsLeftToPlace, unitWhichRequiresUnitsHasRequiredUnits(producer, true))
unitsLeftToPlace, unitWhichRequiresUnitsHasRequiredUnits(producer, false))
: new ArrayList<>(unitsLeftToPlace));
}

Expand Down Expand Up @@ -389,7 +389,7 @@ private void freePlacementCapacity(
// around at all
if (placeTerritory.isWater()
&& !placeTerritory.equals(producer)
&& (!Properties.getUnitPlacementRestrictions(getData().getProperties())
&& (!Properties.getUnitPlacementRestrictions(getProperties())
|| placement.getUnits().stream()
.noneMatch(Matches.unitRequiresUnitsOnCreation()))) {
// found placement move of producer that can be taken over
Expand Down Expand Up @@ -514,8 +514,8 @@ private void freePlacementCapacity(
if (!at.isWater()) {
return null;
}
if (!Properties.getMoveExistingFightersToNewCarriers(getData().getProperties())
|| Properties.getLhtrCarrierProductionRules(getData().getProperties())) {
if (!Properties.getMoveExistingFightersToNewCarriers(getProperties())
|| Properties.getLhtrCarrierProductionRules(getProperties())) {
return null;
}
if (units.stream().noneMatch(Matches.unitIsCarrier())) {
Expand Down Expand Up @@ -697,14 +697,14 @@ private String isValidPlacement(
return null;
}
// make sure some unit has fulfilled requiresUnits requirements
if (Properties.getUnitPlacementRestrictions(getData().getProperties())
if (Properties.getUnitPlacementRestrictions(getProperties())
&& !testUnits.isEmpty()
&& testUnits.stream().noneMatch(unitWhichRequiresUnitsHasRequiredUnits(producer, true))) {
&& testUnits.stream().noneMatch(unitWhichRequiresUnitsHasRequiredUnits(producer, false))) {
return "You do not have the required units to build in " + producer.getName();
}
if (to.isWater()
&& (!Properties.getWW2V2(getData().getProperties())
&& !Properties.getUnitPlacementInEnemySeas(getData().getProperties()))
&& (!Properties.getWW2V2(getProperties())
&& !Properties.getUnitPlacementInEnemySeas(getProperties()))
&& to.anyUnitsMatch(Matches.enemyUnit(player))) {
return "Cannot place sea units with enemy naval units";
}
Expand Down Expand Up @@ -857,7 +857,7 @@ private List<Territory> getAllProducers(
return "Not Enough Units To Upgrade or Be Consumed";
}
// now return null (valid placement) if we have placement restrictions disabled in game options
if (!Properties.getUnitPlacementRestrictions(getData().getProperties())) {
if (!Properties.getUnitPlacementRestrictions(getProperties())) {
return null;
}
// account for any unit placement restrictions by territory
Expand Down Expand Up @@ -888,7 +888,7 @@ private List<Territory> getAllProducers(

protected @Nullable Collection<Unit> getUnitsToBePlaced(
final Territory to, final Collection<Unit> allUnits, final GamePlayer player) {
final GameProperties properties = getData().getProperties();
final GameProperties properties = getProperties();
final boolean water = to.isWater();
if (water
&& (!Properties.getWW2V2(properties) && !Properties.getUnitPlacementInEnemySeas(properties))
Expand Down Expand Up @@ -969,7 +969,7 @@ private List<Territory> getAllProducers(
&& ua.getCanOnlyBePlacedInTerritoryValuedAtX() > territoryProduction) {
continue;
}
if (unitWhichRequiresUnitsHasRequiredUnits(to, false).negate().test(currentUnit)) {
if (unitWhichRequiresUnitsHasRequiredUnits(to, true).negate().test(currentUnit)) {
continue;
}
if (Matches.unitCanOnlyPlaceInOriginalTerritories().test(currentUnit)
Expand Down Expand Up @@ -1115,11 +1115,13 @@ protected int getMaxUnitsToBePlacedFrom(
final boolean countSwitchedProductionToNeighbors,
final Collection<Territory> notUsableAsOtherProducers,
final Map<Territory, Integer> currentAvailablePlacementForOtherProducers) {
final GameProperties properties = getProperties();
final boolean unitPlacementRestrictions = Properties.getUnitPlacementRestrictions(properties);
// we may have special units with requiresUnits restrictions
final Collection<Unit> unitsCanBePlacedByThisProducer =
(Properties.getUnitPlacementRestrictions(getData().getProperties())
(unitPlacementRestrictions
? CollectionUtils.getMatches(
units, unitWhichRequiresUnitsHasRequiredUnits(producer, true))
units, unitWhichRequiresUnitsHasRequiredUnits(producer, false))
: new ArrayList<>(units));
if (unitsCanBePlacedByThisProducer.isEmpty()) {
return 0;
Expand All @@ -1134,7 +1136,7 @@ units, unitWhichRequiresUnitsHasRequiredUnits(producer, true))
final Collection<Unit> factoryUnits = producer.getUnitCollection().getMatches(factoryMatch);
// boolean placementRestrictedByFactory = isPlacementRestrictedByFactory();
final boolean unitPlacementPerTerritoryRestricted =
Properties.getUnitPlacementPerTerritoryRestricted(getData().getProperties());
Properties.getUnitPlacementPerTerritoryRestricted(properties);
final boolean originalFactory = (ta != null && ta.getOriginalFactory());
final boolean playerIsOriginalOwner =
!factoryUnits.isEmpty() && this.player.equals(getOriginalFactoryOwner(producer));
Expand Down Expand Up @@ -1180,12 +1182,7 @@ units, unitWhichRequiresUnitsHasRequiredUnits(producer, true))
// maxConstructions
int production =
UnitUtils.getProductionPotentialOfTerritory(
unitsAtStartOfStepInTerritory(producer),
producer,
player,
getData().getProperties(),
true,
true);
unitsAtStartOfStepInTerritory(producer), producer, player, properties, true, true);
// increase the production by the number of constructions allowed
if (maxConstructions > 0) {
production += maxConstructions;
Expand Down Expand Up @@ -1219,7 +1216,7 @@ units, unitWhichRequiresUnitsHasRequiredUnits(producer, true))
// TODO: Units which have the unit attachment property, requiresUnits, are too difficult
// to mess with logically, so we ignore them for our special 'move shit around' methods.
if (!placeTerritory.isWater()
|| (Properties.getUnitPlacementRestrictions(getData().getProperties())
|| (Properties.getUnitPlacementRestrictions(properties)
&& unitsPlacedByCurrentPlacementMove.stream()
.anyMatch(Matches.unitRequiresUnitsOnCreation()))) {
productionCanNotBeMoved += unitsPlacedByCurrentPlacementMove.size();
Expand Down Expand Up @@ -1320,7 +1317,7 @@ IntegerMap<String> howManyOfEachConstructionCanPlace(
final IntegerMap<String> unitMapHeld = new IntegerMap<>();
final IntegerMap<String> unitMapMaxType = new IntegerMap<>();
final IntegerMap<String> unitMapTypePerTurn = new IntegerMap<>();
final int maxFactory = Properties.getFactoriesPerCountry(getData().getProperties());
final int maxFactory = Properties.getFactoriesPerCountry(getProperties());
// Can be null!
final TerritoryAttachment terrAttachment = TerritoryAttachment.get(to);
int toProduction = 0;
Expand All @@ -1330,15 +1327,15 @@ IntegerMap<String> howManyOfEachConstructionCanPlace(
for (final Unit currentUnit : CollectionUtils.getMatches(units, Matches.unitIsConstruction())) {
final UnitAttachment ua = currentUnit.getUnitAttachment();
// account for any unit placement restrictions by territory
if (Properties.getUnitPlacementRestrictions(getData().getProperties())) {
if (Properties.getUnitPlacementRestrictions(getProperties())) {
if (ua.unitPlacementRestrictionsContain(to)) {
continue;
}
if (ua.getCanOnlyBePlacedInTerritoryValuedAtX() != -1
&& ua.getCanOnlyBePlacedInTerritoryValuedAtX() > toProduction) {
continue;
}
if (unitWhichRequiresUnitsHasRequiredUnits(to, false).negate().test(currentUnit)) {
if (unitWhichRequiresUnitsHasRequiredUnits(to, true).negate().test(currentUnit)) {
continue;
}
}
Expand All @@ -1358,11 +1355,9 @@ IntegerMap<String> howManyOfEachConstructionCanPlace(
}
}
final boolean moreWithoutFactory =
Properties.getMoreConstructionsWithoutFactory(getData().getProperties());
final boolean moreWithFactory =
Properties.getMoreConstructionsWithFactory(getData().getProperties());
final boolean unlimitedConstructions =
Properties.getUnlimitedConstructions(getData().getProperties());
Properties.getMoreConstructionsWithoutFactory(getProperties());
final boolean moreWithFactory = Properties.getMoreConstructionsWithFactory(getProperties());
final boolean unlimitedConstructions = Properties.getUnlimitedConstructions(getProperties());
final boolean wasFactoryThereAtStart =
wasOwnedUnitThatCanProduceUnitsOrIsFactoryInTerritoryAtStartOfStep(to, player);
// build an integer map of each construction unit in the territory
Expand Down Expand Up @@ -1442,11 +1437,11 @@ static int howManyOfConstructionUnit(final Unit unit, final IntegerMap<String> c
* adjacent land territory has one of the required combos as well).
*
* @param to - Territory we are testing for required units
* @param doNotCountNeighbors If false, and 'to' is water, then we will test neighboring land
* @param countNeighbors If true, and 'to' is water, then we will test neighboring land
* territories to see if they have any of the required units as well.
*/
private Predicate<Unit> unitWhichRequiresUnitsHasRequiredUnits(
final Territory to, final boolean doNotCountNeighbors) {
final Territory to, final boolean countNeighbors) {
return unitWhichRequiresUnits -> {
if (!Matches.unitRequiresUnitsOnCreation().test(unitWhichRequiresUnits)) {
return true;
Expand All @@ -1458,7 +1453,7 @@ private Predicate<Unit> unitWhichRequiresUnitsHasRequiredUnits(
.test(unitWhichRequiresUnits)) {
return true;
}
if (!doNotCountNeighbors && to.isWater()) {
if (countNeighbors && to.isWater()) {
for (final Territory current :
getAllProducers(to, player, List.of(unitWhichRequiresUnits), true)) {
final Collection<Unit> unitsAtStartOfTurnInCurrent =
Expand All @@ -1480,7 +1475,7 @@ private boolean getCanAllUnitsWithRequiresUnitsBePlacedCorrectly(

private Collection<Unit> getUnitsThatCantBePlacedThatRequireUnits(
final Collection<Unit> units, final Territory to) {
if (!Properties.getUnitPlacementRestrictions(getData().getProperties())
if (!Properties.getUnitPlacementRestrictions(getProperties())
|| units.stream().noneMatch(Matches.unitRequiresUnitsOnCreation())) {
return List.of();
}
Expand All @@ -1498,7 +1493,7 @@ private Collection<Unit> getUnitsThatCantBePlacedThatRequireUnits(
final int productionHere = producersMap.getInt(t);
final List<Unit> canBePlacedHere =
CollectionUtils.getMatches(
unitsLeftToPlace, unitWhichRequiresUnitsHasRequiredUnits(t, true));
unitsLeftToPlace, unitWhichRequiresUnitsHasRequiredUnits(t, false));
if (productionHere == -1 || productionHere >= canBePlacedHere.size()) {
unitsLeftToPlace.removeAll(canBePlacedHere);
continue;
Expand Down Expand Up @@ -1680,15 +1675,15 @@ protected boolean wasConquered(final Territory t) {
}

private boolean isPlayerAllowedToPlacementAnyTerritoryOwnedLand(final GamePlayer player) {
if (Properties.getPlaceInAnyTerritory(getData().getProperties())) {
if (Properties.getPlaceInAnyTerritory(getProperties())) {
final RulesAttachment ra = player.getRulesAttachment();
return ra != null && ra.getPlacementAnyTerritory();
}
return false;
}

private boolean isPlayerAllowedToPlacementAnySeaZoneByOwnedLand(final GamePlayer player) {
if (Properties.getPlaceInAnyTerritory(getData().getProperties())) {
if (Properties.getPlaceInAnyTerritory(getProperties())) {
final RulesAttachment ra = player.getRulesAttachment();
return ra != null && ra.getPlacementAnySeaZone();
}
Expand Down
2 changes: 1 addition & 1 deletion ...ame-core/src/main/java/games/strategy/triplea/delegate/remote/IAbstractPlaceDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ default String placeUnits(final Collection<Unit> units, final Territory at) {
return placeUnits(units, at, BidMode.NOT_BID);
}

/** Indicates whether or not bidding is enabled during placement. */
/** Indicates whether bidding is enabled during placement. */
enum BidMode {
BID,
NOT_BID
Expand Down
23 changes: 23 additions & 0 deletions game-app/game-core/src/test/java/games/strategy/triplea/delegate/PlaceDelegateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,29 @@ void testCanNotProduceThatManyUnitsDueToRequiresUnits() {
assertThat(response.getUnits(), hasSize(3));
}

@Test
void testRequiresUnitsSea() {
gameData.getProperties().set(UNIT_PLACEMENT_RESTRICTIONS, true);
// Needed for canProduceXUnits to work. (!)
gameData.getProperties().set(DAMAGE_FROM_BOMBING_DONE_TO_UNITS_INSTEAD_OF_TERRITORIES, true);
final var factory2 = unitType("factory2", gameData);
final var sub2 = unitType("submarine2", gameData);

final var threeSub2 = create(british, sub2, 3);
final var fourSub2 = create(british, sub2, 4);

uk.getUnitCollection().clear();
northSea.getUnitCollection().clear();
assertError(delegate.canUnitsBePlaced(northSea, threeSub2, british));
uk.getUnitCollection().addAll(create(british, factory2, 1));
assertValid(delegate.canUnitsBePlaced(northSea, threeSub2, british));
assertError(delegate.canUnitsBePlaced(northSea, fourSub2, british));
final PlaceableUnits response = delegate.getPlaceableUnits(fourSub2, northSea);
assertThat(response.getUnits(), hasSize(3));
// We also can't place the subs in UK since they're sea units. :)
assertError(delegate.canUnitsBePlaced(uk, threeSub2, british));
}

@Test
void testAlreadyProducedUnits() {
delegate.setProduced(Map.of(westCanada, create(british, infantry, 2)));
Expand Down
10 changes: 9 additions & 1 deletion game-app/game-core/src/test/resources/DelegateTest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@
<unit name="aaGun"/>
<unit name="factory2"/>
<unit name="infantry2"/>
<unit name="submarine2"/>
</unitList>
<gamePlay>
<delegate name="initDelegate" javaClass="games.strategy.triplea.delegate.InitializationDelegate"/>
Expand Down Expand Up @@ -721,7 +722,6 @@
<option name="isFactory" value="true"/>
</attachment>


<attachment name="unitAttachment" attachTo="factory2" javaClass="UnitAttachment" type="unitType">
<option name="isFactory" value="true"/>
<option name="canProduceXUnits" value="3"/>
Expand All @@ -733,6 +733,14 @@
<option name="defense" value="2"/>
<option name="requiresUnits" value="factory2"/>
</attachment>
<attachment name="unitAttachment" attachTo="submarine2" javaClass="UnitAttachment" type="unitType">
<option name="isSub" value="true"/>
<option name="movement" value="2"/>
<option name="isSea" value="true"/>
<option name="attack" value="2"/>
<option name="defense" value="2"/>
<option name="requiresUnits" value="factory2"/>
</attachment>

<attachment name="unitAttachment"
attachTo="aaGun"
Expand Down

0 comments on commit 4c5c6bf

Please sign in to comment.