Air moving with carriers ignore fuel costs (#3249)

* Add new unit field chargedFlatFuelCost

* Add new match for chargedFlatFuelCost

* Make sure to reset new property

* Make method public

* Update route fuel cost logic and make sure to set chargedFlatFuelCost

* Add unit test for carrier fuel

* Add comment

game-core/src/main/java/games/strategy/engine/data/Route.java

@@ -11,10 +11,14 @@
 import java.util.Set;
 import java.util.function.Predicate;
 
+import games.strategy.engine.data.changefactory.ChangeFactory;
 import games.strategy.triplea.TripleAUnit;
 import games.strategy.triplea.attachments.UnitAttachment;
+import games.strategy.triplea.delegate.AirMovementValidator;
+import games.strategy.triplea.delegate.GameStepPropertiesHelper;
 import games.strategy.triplea.delegate.Matches;
 import games.strategy.util.CollectionUtils;
+import games.strategy.util.Tuple;
 
 /**
  * A route between two territories.
@@ -396,31 +400,78 @@ public int getMovementLeft(final Unit unit) {
     return movementLeft;
   }
 
-  private ResourceCollection getMovementFuelCostCharge(final Unit unit, final GameData data) {
-    final ResourceCollection col = new ResourceCollection(data);
-    if (Matches.unitIsBeingTransported().test(unit)) {
-      return col;
-    }
-    final UnitAttachment ua = UnitAttachment.get(unit.getType());
-    col.add(ua.getFuelCost());
-    col.multiply(getMovementCost(unit));
-    if (Matches.unitHasNotMoved().test(unit)) {
-      col.add(ua.getFuelFlatCost());
+  public static Change getFuelChanges(final Collection<Unit> units, final Route route, final PlayerID player,
+      final GameData data) {
+    final CompositeChange changes = new CompositeChange();
+    final Tuple<ResourceCollection, Set<Unit>> tuple =
+        Route.getFuelCostsAndUnitsChargedFlatFuelCost(units, route, player, data);
+    changes.add(ChangeFactory.removeResourceCollection(player, tuple.getFirst()));
+    for (final Unit unit : tuple.getSecond()) {
+      changes.add(ChangeFactory.unitPropertyChange(unit, Boolean.TRUE, TripleAUnit.CHARGED_FLAT_FUEL_COST));
     }
-    return col;
+    return changes;
+  }
+
+  public static ResourceCollection getMovementFuelCostCharge(final Collection<Unit> units, final Route route,
+      final PlayerID player, final GameData data) {
+    return Route.getFuelCostsAndUnitsChargedFlatFuelCost(units, route, player, data).getFirst();
   }
 
-  public static ResourceCollection getMovementFuelCostCharge(final Collection<Unit> unitsAll, final Route route,
-      final PlayerID currentPlayer, final GameData data) {
-    final Set<Unit> units = new HashSet<>(unitsAll);
+  /**
+   * Find fuel costs and which units are to be charged flat fuel costs. Ignores dependent units
+   * and if non-combat then ignores air units moving with carrier.
+   */
+  private static Tuple<ResourceCollection, Set<Unit>> getFuelCostsAndUnitsChargedFlatFuelCost(
+      final Collection<Unit> units, final Route route, final PlayerID player, final GameData data) {
+    final Set<Unit> unitsToChargeFuelCosts = new HashSet<>(units);
+
+    // If non-combat then remove air units moving with a carrier
+    if (GameStepPropertiesHelper.isNonCombatMove(data, true)) {
+
+      // Add allied air first so that the carriers take them into account before owned air
+      final List<Unit> canLandOnCarrierUnits = CollectionUtils.getMatches(units,
+          Matches.unitIsOwnedBy(player).negate()
+              .and(Matches.isUnitAllied(player, data))
+              .and(Matches.unitCanLandOnCarrier()));
+      canLandOnCarrierUnits.addAll(CollectionUtils.getMatches(units,
+          Matches.unitIsOwnedBy(player).and(Matches.unitCanLandOnCarrier())));
+      unitsToChargeFuelCosts.removeAll(AirMovementValidator.whatAirCanLandOnTheseCarriers(
+          CollectionUtils.getMatches(units, Matches.unitIsCarrier()),
+          canLandOnCarrierUnits, route.getStart()));
+    }
+
+    // Remove dependent units
+    unitsToChargeFuelCosts.removeAll(CollectionUtils.getMatches(units,
+        Matches.unitIsBeingTransportedByOrIsDependentOfSomeUnitInThisList(units, player, data, true)));
 
-    units.removeAll(CollectionUtils.getMatches(unitsAll,
-        Matches.unitIsBeingTransportedByOrIsDependentOfSomeUnitInThisList(unitsAll, currentPlayer, data, true)));
+    // Find fuel cost and whether to charge flat fuel cost
     final ResourceCollection movementCharge = new ResourceCollection(data);
-    for (final Unit unit : units) {
-      movementCharge.add(route.getMovementFuelCostCharge(unit, data));
+    final Set<Unit> unitsChargedFlatFuelCost = new HashSet<>();
+    for (final Unit unit : unitsToChargeFuelCosts) {
+      final Tuple<ResourceCollection, Boolean> tuple = route.getFuelCostsAndIfChargedFlatFuelCost(unit, data);
+      movementCharge.add(tuple.getFirst());
+      if (tuple.getSecond()) {
+        unitsChargedFlatFuelCost.add(unit);
+      }
+    }
+    return Tuple.of(movementCharge, unitsChargedFlatFuelCost);
+  }
+
+  private Tuple<ResourceCollection, Boolean> getFuelCostsAndIfChargedFlatFuelCost(final Unit unit,
+      final GameData data) {
+    final ResourceCollection resources = new ResourceCollection(data);
+    boolean chargedFlatFuelCost = false;
+    if (Matches.unitIsBeingTransported().test(unit)) {
+      return Tuple.of(resources, chargedFlatFuelCost);
+    }
+    final UnitAttachment ua = UnitAttachment.get(unit.getType());
+    resources.add(ua.getFuelCost());
+    resources.multiply(getMovementCost(unit));
+    if (Matches.unitHasNotBeenChargedFlatFuelCost().test(unit)) {
+      resources.add(ua.getFuelFlatCost());
+      chargedFlatFuelCost = true;
     }
-    return movementCharge;
+    return Tuple.of(resources, chargedFlatFuelCost);
   }
 
   public static Route create(final List<Route> routes) {

game-core/src/main/java/games/strategy/triplea/TripleAUnit.java

@@ -56,6 +56,7 @@ public class TripleAUnit extends Unit {
   public static final String WAS_IN_AIR_BATTLE = "wasInAirBattle";
   public static final String LAUNCHED = "launched";
   public static final String AIRBORNE = "airborne";
+  public static final String CHARGED_FLAT_FUEL_COST = "chargedFlatFuelCost";
   // the transport that is currently transporting us
   private TripleAUnit m_transportedBy = null;
   // the units we have unloaded this turn
@@ -90,6 +91,8 @@ public class TripleAUnit extends Unit {
   private int m_launched = 0;
   // was this unit airborne and launched this turn
   private boolean m_airborne = false;
+  // was charged flat fuel cost already this turn
+  private boolean m_chargedFlatFuelCost = false;
 
   public static TripleAUnit get(final Unit u) {
     return (TripleAUnit) u;
@@ -298,6 +301,15 @@ private void setAirborne(final boolean value) {
     m_airborne = value;
   }
 
+  public boolean getChargedFlatFuelCost() {
+    return m_chargedFlatFuelCost;
+  }
+
+  @GameProperty(xmlProperty = false, gameProperty = true, adds = false)
+  private void setChargedFlatFuelCost(final boolean value) {
+    m_chargedFlatFuelCost = value;
+  }
+
   @GameProperty(xmlProperty = false, gameProperty = true, adds = false)
   private void setWasInAirBattle(final boolean value) {
     m_wasInAirBattle = value;
@@ -601,6 +613,10 @@ public Map<String, MutableProperty<?>> getPropertyMap() {
             MutableProperty.ofSimple(
                 this::setAirborne,
                 this::getAirborne))
+        .put("chargedFlatFuelCost",
+            MutableProperty.ofSimple(
+                this::setChargedFlatFuelCost,
+                this::getChargedFlatFuelCost))
         .build();
   }
 }

game-core/src/main/java/games/strategy/triplea/delegate/AirMovementValidator.java

@@ -497,7 +497,12 @@ private static int maxMovementLeftForTheseAirUnitsBeingValidated(final Collectio
     return max;
   }
 
-  private static Collection<Unit> whatAirCanLandOnTheseCarriers(final Collection<Unit> carriers,
+  /**
+   * Does not, and is not supposed to, account for any units already on this carrier (like allied/cargo fighters).
+   * Instead this method only adds up the total capacity of each unit, and accounts for damaged carriers with special
+   * properties and restrictions. So should pass allied/cargo fighters in and have them first in the list.
+   */
+  public static Collection<Unit> whatAirCanLandOnTheseCarriers(final Collection<Unit> carriers,
       final Collection<Unit> airUnits, final Territory territoryUnitsAreIn) {
     final Collection<Unit> airThatCanLandOnThem = new ArrayList<>();
     for (final Unit carrier : carriers) {
@@ -654,8 +659,7 @@ private static Collection<Unit> getAirThatMustLandOnCarriers(final GameData data
   /**
    * Does not, and is not supposed to, account for any units already on this carrier (like allied/cargo fighters).
    * Instead this method only adds up the total capacity of each unit, and accounts for damaged carriers with special
-   * properties and
-   * restrictions.
+   * properties and restrictions.
    */
   public static int carrierCapacity(final Collection<Unit> units, final Territory territoryUnitsAreCurrentlyIn) {
     int sum = 0;
@@ -668,8 +672,7 @@ public static int carrierCapacity(final Collection<Unit> units, final Territory
   /**
    * Does not, and is not supposed to, account for any units already on this carrier (like allied/cargo fighters).
    * Instead this method only adds up the total capacity of each unit, and accounts for damaged carriers with special
-   * properties and
-   * restrictions.
+   * properties and restrictions.
    */
   public static int carrierCapacity(final Unit unit, final Territory territoryUnitsAreCurrentlyIn) {
     if (Matches.unitIsCarrier().test(unit)) {

game-core/src/main/java/games/strategy/triplea/delegate/Matches.java

@@ -202,6 +202,10 @@ public static Predicate<Unit> unitHasNotMoved() {
     return unitHasMoved().negate();
   }
 
+  public static Predicate<Unit> unitHasNotBeenChargedFlatFuelCost() {
+    return unit -> !TripleAUnit.get(unit).getChargedFlatFuelCost();
+  }
+
   static Predicate<Unit> unitCanAttack(final PlayerID id) {
     return unit -> {
       final UnitAttachment ua = UnitAttachment.get(unit.getType());

game-core/src/main/java/games/strategy/triplea/delegate/MoveDelegate.java

@@ -307,6 +307,9 @@ static Change getResetUnitStateChange(final GameData data) {
       if (taUnit.getWasAmphibious()) {
         change.add(ChangeFactory.unitPropertyChange(u, Boolean.FALSE, TripleAUnit.UNLOADED_AMPHIBIOUS));
       }
+      if (taUnit.getChargedFlatFuelCost()) {
+        change.add(ChangeFactory.unitPropertyChange(u, Boolean.FALSE, TripleAUnit.CHARGED_FLAT_FUEL_COST));
+      }
     }
     return change;
   }

game-core/src/main/java/games/strategy/triplea/delegate/MovePerformer.java

@@ -33,6 +33,9 @@
 import games.strategy.util.CollectionUtils;
 import games.strategy.util.PredicateBuilder;
 
+/**
+ * Used to move units and make changes to game state.
+ */
 public class MovePerformer implements Serializable {
   private static final long serialVersionUID = 3752242292777658310L;
   private transient AbstractMoveDelegate moveDelegate;
@@ -163,7 +166,7 @@ public void execute(final ExecutionStack stack, final IDelegateBridge bridge) {
         if (Properties.getUseFuelCost(data)) {
           // markFuelCostResourceChange must be done
           // before we load/unload units
-          change.add(markFuelCostResourceChange(units, route, id, data));
+          change.add(Route.getFuelChanges(units, route, id, data));
         }
         markTransportsMovement(arrived, transporting, route);
         if (route.anyMatch(mustFightThrough) && arrived.size() != 0) {
@@ -296,12 +299,6 @@ private static Predicate<Territory> getMustFightThroughMatch(final PlayerID id,
         .or(Matches.territoryIsOwnedByPlayerWhosRelationshipTypeCanTakeOverOwnedTerritoryAndPassableAndNotWater(id));
   }
 
-  private static Change markFuelCostResourceChange(final Collection<Unit> units, final Route route, final PlayerID id,
-      final GameData data) {
-    return ChangeFactory.removeResourceCollection(id,
-        Route.getMovementFuelCostCharge(units, route, id, data));
-  }
-
   private Change markMovementChange(final Collection<Unit> units, final Route route, final PlayerID id) {
     final GameData data = bridge.getData();
     final CompositeChange change = new CompositeChange();

game-core/src/test/java/games/strategy/triplea/delegate/VictoryTest.java

@@ -27,6 +27,7 @@
 public class VictoryTest {
   private GameData gameData;
   private PlayerID italians;
+  private PlayerID germans;
   private ITestDelegateBridge testBridge;
 
   private IntegerMap<Resource> italianResources;
@@ -35,16 +36,21 @@ public class VictoryTest {
   private Territory kenya;
   private UnitType motorized;
   private UnitType armour;
+  private UnitType fighter;
+  private UnitType carrier;
   private Territory frenchEastAfrica;
   private Territory frenchWestAfrica;
   private Territory angloEgypt;
   private Territory libya;
+  private Territory sz29;
+  private Territory sz30;
   private MoveDelegate moveDelegate;
 
   @BeforeEach
   public void setUp() throws Exception {
     gameData = TestMapGameData.VICTORY_TEST.getGameData();
     italians = GameDataTestUtil.italians(gameData);
+    germans = GameDataTestUtil.germans(gameData);
     testBridge = GameDataTestUtil.getDelegateBridge(italians, gameData);
     // we need to initialize the original owner
     final InitializationDelegate initDel =
@@ -61,11 +67,14 @@ public void setUp() throws Exception {
     kenya = gameData.getMap().getTerritory("Kenya");
     motorized = gameData.getUnitTypeList().getUnitType(Constants.UNIT_TYPE_MOTORIZED);
     armour = GameDataTestUtil.armour(gameData);
+    fighter = GameDataTestUtil.fighter(gameData);
+    carrier = GameDataTestUtil.carrier(gameData);
     frenchEastAfrica = gameData.getMap().getTerritory("French Equatorial Africa");
     frenchWestAfrica = gameData.getMap().getTerritory("French West Africa");
     angloEgypt = gameData.getMap().getTerritory("Anglo Egypt");
     libya = gameData.getMap().getTerritory("Libya");
-
+    sz29 = gameData.getMap().getTerritory("29 Sea Zone");
+    sz30 = gameData.getMap().getTerritory("30 Sea Zone");
   }
 
   @Test
@@ -157,18 +166,18 @@ public void testMotorizedNoBlitzBlitzedTerritory() {
   }
 
   @Test
-  public void testFuelUseMotorized() {
+  public void testFuelCostAndFuelFlatCost() {
     gameData.performChange(ChangeFactory.changeOwner(kenya, italians));
     gameData.performChange(ChangeFactory.changeOwner(britishCongo, italians));
     gameData.performChange(ChangeFactory.changeOwner(frenchEastAfrica, italians));
-    gameData.performChange(ChangeFactory.addUnits(kenya, motorized.create(1, italians)));
     testBridge.setStepName("CombatMove");
     moveDelegate.setDelegateBridgeAndPlayer(testBridge);
     moveDelegate.start();
     final int fuelAmount = italians.getResources().getQuantity("Fuel");
     final int puAmount = italians.getResources().getQuantity("PUs");
     final int oreAmount = italians.getResources().getQuantity("Ore");
 
+    gameData.performChange(ChangeFactory.addUnits(kenya, motorized.create(1, italians)));
     moveDelegate.move(kenya.getUnits().getUnits(), gameData.getMap().getRoute(kenya, britishCongo));
     assertEquals(fuelAmount - 2, italians.getResources().getQuantity("Fuel"));
     assertEquals(puAmount - 1, italians.getResources().getQuantity("PUs"));
@@ -198,6 +207,59 @@ public void testFuelUseMotorized() {
     moveDelegate.end();
   }
 
+  @Test
+  public void testFuelForCarriers() {
+    testBridge.setStepName("CombatMove");
+    moveDelegate.setDelegateBridgeAndPlayer(testBridge);
+    moveDelegate.start();
+    final int fuelAmount = italians.getResources().getQuantity("Fuel");
+
+    // Combat move where air is always charged fuel
+    gameData.performChange(ChangeFactory.addUnits(sz29, carrier.create(1, italians)));
+    gameData.performChange(ChangeFactory.addUnits(sz29, fighter.create(2, italians)));
+    moveDelegate.move(sz29.getUnits().getUnits(), gameData.getMap().getRoute(sz29, sz30));
+    assertEquals(fuelAmount - 7, italians.getResources().getQuantity("Fuel"));
+
+    // Rest of the cases use non-combat move
+    moveDelegate.end();
+    testBridge.setStepName("NonCombatMove");
+    moveDelegate.setDelegateBridgeAndPlayer(testBridge);
+    moveDelegate.start();
+
+    // Non-combat move where air isn't charged fuel
+    gameData.performChange(ChangeFactory.addUnits(sz29, carrier.create(1, italians)));
+    gameData.performChange(ChangeFactory.addUnits(sz29, fighter.create(2, italians)));
+    moveDelegate.move(sz29.getUnits().getUnits(), gameData.getMap().getRoute(sz29, sz30));
+    assertEquals(fuelAmount - 8, italians.getResources().getQuantity("Fuel"));
+    gameData.performChange(ChangeFactory.removeUnits(sz30, sz30.getUnits()));
+
+    // Move onto carrier, move with carrier, move off carrier
+    gameData.performChange(ChangeFactory.addUnits(sz29, carrier.create(1, italians)));
+    gameData.performChange(ChangeFactory.addUnits(sz29, fighter.create(1, italians)));
+    gameData.performChange(ChangeFactory.addUnits(sz30, fighter.create(1, italians)));
+    moveDelegate.move(sz30.getUnits().getUnits(), gameData.getMap().getRoute(sz30, sz29));
+    assertEquals(fuelAmount - 11, italians.getResources().getQuantity("Fuel"));
+    moveDelegate.move(sz29.getUnits().getUnits(), gameData.getMap().getRoute(sz29, sz30));
+    assertEquals(fuelAmount - 12, italians.getResources().getQuantity("Fuel"));
+    moveDelegate.move(sz30.getUnits().getMatches(Matches.unitIsAir()), gameData.getMap().getRoute(sz30, sz29));
+    assertEquals(fuelAmount - 16, italians.getResources().getQuantity("Fuel"));
+    gameData.performChange(ChangeFactory.removeUnits(sz29, sz29.getUnits()));
+    gameData.performChange(ChangeFactory.removeUnits(sz30, sz30.getUnits()));
+
+    // Too many fighters for carrier
+    gameData.performChange(ChangeFactory.addUnits(sz29, carrier.create(1, italians)));
+    gameData.performChange(ChangeFactory.addUnits(sz29, fighter.create(3, italians)));
+    moveDelegate.move(sz29.getUnits().getUnits(), gameData.getMap().getRoute(sz29, sz30));
+    assertEquals(fuelAmount - 20, italians.getResources().getQuantity("Fuel"));
+
+    // Allied and owned fighters
+    gameData.performChange(ChangeFactory.addUnits(sz29, carrier.create(2, italians)));
+    gameData.performChange(ChangeFactory.addUnits(sz29, fighter.create(2, italians)));
+    gameData.performChange(ChangeFactory.addUnits(sz29, fighter.create(3, germans)));
+    moveDelegate.move(sz29.getUnits().getUnits(), gameData.getMap().getRoute(sz29, sz30));
+    assertEquals(fuelAmount - 25, italians.getResources().getQuantity("Fuel"));
+  }
+
   @Test
   public void testMultipleResourcesToPurchase() {
     testBridge.setStepName("italianPurchase");

game-core/src/test/resources/victory_test.xml

@@ -1914,6 +1914,8 @@
                          <option name="canEscort" value="true"/>
                          <option name="airDefense" value="2"/>
                          <option name="airAttack" value="2"/>
+                         <option name="fuelCost" value="Fuel" count="1"/>
+                         <option name="fuelFlatCost" value="Fuel" count="2"/>
                 </attachment>
 
                 <attachment name="unitAttachment" attachTo="jp_fighter" javaClass="games.strategy.triplea.attachments.UnitAttachment" type="unitType">
@@ -2037,6 +2039,7 @@
                          <option name="attack" value="1"/>
                          <option name="defense" value="2"/>
                          <option name="blockade" value="1"/>
+                         <option name="fuelCost" value="Fuel" count="1"/>                        
                 </attachment>
 
                 <!-- Super_Carrier -->
@@ -4444,7 +4447,7 @@
                         <resourceGiven player="Italians" resource="PUs" quantity="16"/>
                         <resourceGiven player="Chinese" resource="PUs" quantity="0"/>
 
-                        <resourceGiven player="Italians" resource="Fuel" quantity="20"/>
+                        <resourceGiven player="Italians" resource="Fuel" quantity="50"/>
                         <resourceGiven player="Italians" resource="Ore" quantity="20"/>
 
                 </resourceInitialize>