From b860d72c2d5ee33ea7ef359cada9389b35e56b31 Mon Sep 17 00:00:00 2001
From: Dan Albert <dan@gingerhq.net>
Date: Sat, 18 Mar 2023 13:42:51 -0700
Subject: [PATCH] Prevent transfer splitting from leaving empty units.

If the transport were able to move exactly the quantity of units of the
given type remaining in the transfer without moving the whole order, the
transfer order would be left with instructions to transfer zero of that
unit. That's an invariant violation, and was resulting in _later_
transfers attempting to create a convoy with zero units, which pydcs
rightly rejects.

For example: if 2 Abrams and 2 Paladins are ordered to transfer from a
disconnected FOB to a distant location that is connected by road, and
only two cargo slots are available, that transfer could be split into:

```
{
    Abrams: 2
}
```

and

```
{
    Abrams: 0,
    Paladin: 2
}
```

Depending on the route, those airlifted units might end up still needing
road transit (we prefer short airlifts rather than direct routes because
it gives the player more opportunities to intercept enemy convoys). The
current turn would airlift the Abrams-only group and the Paladin group
would wait. On the next turn the Abrams would travel by road and the
Paladins would be airlifted. On the _third_ turn, the Paladins (and zero
Abrams) would generate an invalid convoy.

Fixes https://github.com/dcs-liberation/dcs_liberation/issues/2761.
---
 changelog.md      |  2 ++
 game/transfers.py | 19 +++++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/changelog.md b/changelog.md
index a2196baa..547181fb 100644
--- a/changelog.md
+++ b/changelog.md
@@ -15,6 +15,8 @@ Saves from 6.x are not compatible with 7.0.
 
 ## Fixes
 
+* **[Campaign]** Fixed a longstanding bug where oversized airlifts could corrupt a save with empty convoys. 
+
 # 6.1.1
 
 ## Fixes
diff --git a/game/transfers.py b/game/transfers.py
index c1cf8ecb..d7a68b2d 100644
--- a/game/transfers.py
+++ b/game/transfers.py
@@ -122,12 +122,19 @@ class TransferOrder:
         self.units.clear()
 
     def kill_unit(self, unit_type: GroundUnitType) -> None:
-        if unit_type not in self.units or not self.units[unit_type]:
-            raise KeyError(f"{self} has no {unit_type} remaining")
-        if self.units[unit_type] == 1:
+        self.take_units(unit_type, 1)
+
+    def take_units(self, unit_type: GroundUnitType, count: int) -> None:
+        """Removes units from the transfer without returning them to inventory.
+
+        The caller takes ownership of the units.
+        """
+        if unit_type not in self.units or self.units[unit_type] < count:
+            raise KeyError(f"{self} has fewer than {count} {unit_type} remaining")
+
+        self.units[unit_type] -= count
+        if not self.units[unit_type]:
             del self.units[unit_type]
-        else:
-            self.units[unit_type] -= 1
 
     @property
     def size(self) -> int:
@@ -612,7 +619,7 @@ class PendingTransfers:
         for unit_type, remaining in transfer.units.items():
             take = min(remaining, size)
             size -= take
-            transfer.units[unit_type] -= take
+            transfer.take_units(unit_type, take)
             units[unit_type] = take
             if not size:
                 break