From 9eec135cf4550760c22f51565e8f879a132fe03e Mon Sep 17 00:00:00 2001
From: Alex <ajkmisc@outlook.com>
Date: Mon, 3 Jan 2022 22:28:21 -0600
Subject: [PATCH] Fix overpopulation bug when taking abandoned colony.

Hoping I can persuade adoption of this approach as it eliminates the
entire class of bugs. It is good practice to encapsulate enforcement of
post-conditions, rather than externalize it to all callers (which also
creates code redundancies and induces linkage/exposure to internals of
other objects, such as Planet).
---
 src/rotp/model/colony/Colony.java | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/rotp/model/colony/Colony.java b/src/rotp/model/colony/Colony.java
index 08a4dd338..580718448 100644
--- a/src/rotp/model/colony/Colony.java
+++ b/src/rotp/model/colony/Colony.java
@@ -260,8 +260,16 @@ public String printString() {
 
     public int displayPopulation()          { return population < 1 ? (int) Math.ceil(population) : (int) population; }
     public float population()               { return population; }
-    public void setPopulation(float pop)    { population = pop; }
-    public void adjustPopulation(float pop) { population += pop; }
+    public int setPopulation(float pop)    {
+        float currentMaxPop = planet().currentSize();
+        float lost = pop - currentMaxPop;
+        if (lost > 0) {
+            population = currentMaxPop;
+            return (int) lost;
+        }
+        population = max(0.0f, pop);
+        return 0;
+    }
     public int rebels()                      { return rebels; }
     public void rebels(int i)                { rebels = i; }
     public int deltaPopulation()             { return (int) population - (int) previousPopulation - (int) inTransport(); }
@@ -957,8 +965,12 @@ public void scheduleTransportsToSystem(StarSystem dest, int pop) {
             empire.setVisibleShips();
     }
     public void acceptTransport(Transport t) {
-        setPopulation(min(planet.currentSize(), (population() + t.size())));
-        log("Accepting ", str(t.size()), " transports at: ", starSystem().name(), ". New pop:", fmt(population(), 2));
+        int lostPop = setPopulation(population() + t.size());
+        if (lostPop > 0 && t.empire().isPlayerControlled()) {
+            // TODO: Alert to user...
+        }
+        log("Accepting ", str(t.size() - lostPop), " transports at: ", starSystem().name(), ". New pop:", fmt(population(), 2),
+                "(lost ", str(lostPop), ")");
         t.size(0);
     }
     public float maxTransportsToReceive() {
@@ -1150,7 +1162,10 @@ private void capturedByTransport(Transport tr) {
             }
         }
 
-        setPopulation(min(planet.currentSize(),tr.size()));
+        int lostPop = setPopulation(tr.size());
+        if (lostPop > 0 && tr.empire().isPlayerControlled()) {
+            // TODO: Excess transports lost -- ought to be an alert?
+        }
         tr.size(0);
         shipyard().capturedBy(tr.empire());
         industry().capturedBy(tr.empire());