feat: only refuel 100 units of fuel at a time

This prevents "spillover" when refueling
to full.  Also added a bunch of testing
to the refuel logic and removed some
no longer needed exception catching.

packages/cli/lib/net/actions.dart

@@ -290,38 +290,28 @@ Future<RefuelShip200ResponseData?> refuelIfNeededAndLog(
     return null;
   }
   // shipInfo(ship, 'Refueling (${ship.fuel.current} / ${ship.fuel.capacity})');
-  try {
-    final data = await refuelShip(api, agentCache, shipCache, ship);
-    final marketTransaction = data.transaction;
-    final agent = agentCache.agent;
-    logMarketTransaction(
-      ship,
-      marketPrices,
-      agent,
-      marketTransaction,
-      transactionEmoji: '⛽',
-    );
-    final transaction = Transaction.fromMarketTransaction(
-      marketTransaction,
-      agent.credits,
-      AccountingType.fuel,
-    );
-    await db.insertTransaction(transaction);
-    // Reset flight mode on refueling.
-    if (ship.nav.flightMode != ShipNavFlightMode.CRUISE) {
-      shipInfo(ship, 'Resetting flight mode to cruise');
-      await setShipFlightMode(api, shipCache, ship, ShipNavFlightMode.CRUISE);
-    }
-    return data;
-  } on ApiException catch (e) {
-    // This should no longer be needed now that we check if the market sells
-    // fuel before trying to purchase.
-    if (!isMarketDoesNotSellFuelException(e)) {
-      rethrow;
-    }
-    shipInfo(ship, 'Market does not sell fuel, not refueling.');
+  final data = await refuelShip(api, agentCache, shipCache, ship);
+  final marketTransaction = data.transaction;
+  final agent = agentCache.agent;
+  logMarketTransaction(
+    ship,
+    marketPrices,
+    agent,
+    marketTransaction,
+    transactionEmoji: '⛽',
+  );
+  final transaction = Transaction.fromMarketTransaction(
+    marketTransaction,
+    agent.credits,
+    AccountingType.fuel,
+  );
+  await db.insertTransaction(transaction);
+  // Reset flight mode on refueling.
+  if (ship.nav.flightMode != ShipNavFlightMode.CRUISE) {
+    shipInfo(ship, 'Resetting flight mode to cruise');
+    await setShipFlightMode(api, shipCache, ship, ShipNavFlightMode.CRUISE);
   }
-  return null;
+  return data;
 }
 
 /// Dock the ship if needed and log the transaction

packages/cli/lib/net/direct.dart

@@ -172,9 +172,30 @@ Future<RefuelShip200ResponseData> refuelShip(
   Api api,
   AgentCache agentCache,
   ShipCache shipCache,
-  Ship ship,
-) async {
-  final responseWrapper = await api.fleet.refuelShip(ship.symbol);
+  Ship ship, {
+  bool topUp = false,
+}) async {
+  RefuelShipRequest? refuelShipRequest;
+  if (!topUp) {
+    // If we're not topping up, round down to the closest unit.
+    // units needed is specified in ship fuel units, but we are actually
+    // charged in market units.  One market unit = 100 ship fuel units.
+    // So we round to the closest 100.
+    // Note Ship.shouldRefuel will only return true if the ship needs
+    // more than 100 units of fuel which works out nicely with this logic
+    // to prevent ships from constantly trying to refuel but failing to.
+    final unitsNeeded = (ship.fuelUnitsNeeded ~/ 100) * 100;
+    if (unitsNeeded > 0) {
+      refuelShipRequest = RefuelShipRequest(units: unitsNeeded);
+    } else {
+      // We were asked to refuel with topUp = false, but we don't need fuel.
+      throw StateError(
+        'refuelShip called with topUp = false and < 100 fuel needed',
+      );
+    }
+  }
+  final responseWrapper = await api.fleet
+      .refuelShip(ship.symbol, refuelShipRequest: refuelShipRequest);
   final data = responseWrapper!.data;
   agentCache.agent = data.agent;
   ship.fuel = data.fuel;

packages/cli/test/net/actions_test.dart

@@ -23,8 +23,6 @@ class _MockMarket extends Mock implements Market {}
 
 class _MockMarketPrices extends Mock implements MarketPrices {}
 
-class _MockMarketTransaction extends Mock implements MarketTransaction {}
-
 class _MockShip extends Mock implements Ship {}
 
 class _MockShipNav extends Mock implements ShipNav {}
@@ -33,8 +31,6 @@ class _MockShipyardTransaction extends Mock implements ShipyardTransaction {}
 
 class _MockShipCache extends Mock implements ShipCache {}
 
-class _MockShipFuel extends Mock implements ShipFuel {}
-
 void main() {
   test('purchaseShip', () async {
     final Api api = _MockApi();
@@ -163,9 +159,8 @@ void main() {
     when(() => shipNav.waypointSymbol).thenReturn('A');
     when(() => shipNav.status).thenReturn(ShipNavStatus.IN_ORBIT);
     when(() => shipNav.flightMode).thenReturn(ShipNavFlightMode.CRUISE);
-    final shipFuel = _MockShipFuel();
+    final shipFuel = ShipFuel(current: 0, capacity: 0);
     when(() => ship.fuel).thenReturn(shipFuel);
-    when(() => shipFuel.capacity).thenReturn(0);
     final shipCache = _MockShipCache();
 
     when(
@@ -278,7 +273,6 @@ void main() {
   test('refuelIfNeededAndLog', () async {
     final waypointSymbol = WaypointSymbol.fromString('S-A-W');
     const tradeSymbol = TradeSymbol.FUEL;
-    final shipFuel = _MockShipFuel();
     final ship = _MockShip();
     final shipNav = _MockShipNav();
     when(() => ship.nav).thenReturn(shipNav);
@@ -287,7 +281,6 @@ void main() {
     when(() => shipNav.flightMode).thenReturn(ShipNavFlightMode.CRUISE);
     const shipSymbol = ShipSymbol('S', 1);
     when(() => ship.symbol).thenReturn(shipSymbol.symbol);
-    when(() => ship.fuel).thenReturn(shipFuel);
     final api = _MockApi();
     final fleetApi = _MockFleetApi();
     when(() => api.fleet).thenReturn(fleetApi);
@@ -326,15 +319,13 @@ void main() {
           data: RefuelShip200ResponseData(
             agent: agent,
             transaction: marketTransaction,
-            fuel: shipFuel,
+            fuel: ShipFuel(capacity: 1000, current: 1000),
           ),
         ),
       ),
     );
 
-    when(() => shipFuel.capacity).thenReturn(900);
-    when(() => shipFuel.current).thenReturn(634);
-
+    when(() => ship.fuel).thenReturn(ShipFuel(capacity: 1000, current: 634));
     await runWithLogger(
       logger,
       () => refuelIfNeededAndLog(
@@ -359,6 +350,7 @@ void main() {
         sellPrice: 11,
       ),
     ]);
+    when(() => ship.fuel).thenReturn(ShipFuel(capacity: 1000, current: 634));
 
     await runWithLogger(
       logger,
@@ -375,7 +367,55 @@ void main() {
     verify(
       () => fleetApi.refuelShip(
         shipSymbol.symbol,
-        // TODO(eseidel): Should refill a specific number of units!
+        refuelShipRequest: RefuelShipRequest(units: 300),
+      ),
+    ).called(1);
+
+    clearInteractions(fleetApi);
+    when(() => ship.fuel).thenReturn(ShipFuel(capacity: 1000, current: 901));
+    // Trying to refuel with less than 100 needed, should not refuel.
+    await runWithLogger(
+      logger,
+      () => refuelIfNeededAndLog(
+        api,
+        db,
+        marketPrices,
+        agentCache,
+        shipCache,
+        market,
+        ship,
+      ),
+    );
+    verifyNever(
+      () => fleetApi.refuelShip(
+        shipSymbol.symbol,
+        refuelShipRequest: any(named: 'refuelShipRequest'),
+      ),
+    );
+
+    // Directly calling refuelShip with topUp=false and less than 100 needed
+    // should throw an exception.
+    when(() => ship.fuel).thenReturn(ShipFuel(capacity: 1000, current: 901));
+    expect(
+      () async => refuelShip(api, agentCache, shipCache, ship),
+      throwsA(
+        predicate(
+          (e) =>
+              e is StateError &&
+              e.message ==
+                  'refuelShip called with topUp = false and < 100 fuel needed',
+        ),
+      ),
+    );
+
+    // Directly calling refuelShip with topUp=true and less than 100 needed
+    // should refuel.
+    clearInteractions(fleetApi);
+    when(() => ship.fuel).thenReturn(ShipFuel(capacity: 1000, current: 901));
+    await refuelShip(api, agentCache, shipCache, ship, topUp: true);
+    verify(
+      () => fleetApi.refuelShip(
+        shipSymbol.symbol,
       ),
     ).called(1);
   });

packages/types/lib/api.dart

@@ -388,6 +388,11 @@ extension ShipUtils on Ship {
   // For repeated short trips, avoiding buying fuel when we're close to full.
   bool get shouldRefuel => fuel.current < (fuel.capacity - 100);
 
+  /// Returns the number of units of fuel needed to top up the ship.
+  /// This is in ship fuel units, not market fuel units.
+  /// 1 unit of market fuel = 100 units of ship fuel.
+  int get fuelUnitsNeeded => fuel.capacity - fuel.current;
+
   /// Returns the amount of space available on the ship.
   int get availableSpace => cargo.availableSpace;