Remedial region discovery work.

src/net/sf/freecol/common/model/Region.java

@@ -410,16 +410,22 @@ public class Region extends FreeColGameObject implements Nameable {
     /**
      * Check if this region is has been discovered.
      *
-     * Use the discoverer field to provide mutual exclusion.
-     * Called in csMove when the unit moves into a discoverable region.
+     * Reserve the ability to discover this region by setting the discoverer
+     * field.
+     * Called in csCheckDiscoverRegion when the unit moves into a
+     * discoverable region.
      *
      * @param unit The {@code Unit} that might have discovered the region.
      * @return True if the region has been discovered.
      */
     public synchronized boolean checkDiscover(Unit unit) {
-        if (this.discoverer == null) {
-            this.discoverer = unit.getId();
-            return true;
+        if (this.discoverable) {
+            if (this.discoverer == null) {
+                this.discoverer = unit.getId();
+                return true;
+            } else if (this.discoverer.equals(unit.getId())) {
+                return true; // Work around a desynch problem
+            }
         }
         return false;
     }
@@ -433,8 +439,12 @@ public class Region extends FreeColGameObject implements Nameable {
      * @return A list of discovered {@code Region}s.
      */
     public List<Region> discover(Player player, Unit unit, Turn turn) {
+        if (unit == null || this.discoverer == null ||
+            !this.discoverer.equals(unit.getId())) {
+            // checkDiscover must have been run
+            return Collections.<Region>emptyList();
+        }
         this.discoveredBy = player;
-        assert this.discoverer.equals(unit.getId()); // Require prediscover
         this.discoveredIn = turn;
         this.discoverable = false;
         List<Region> discov = transform(getChildren(), Region::getDiscoverable);
@@ -667,7 +677,11 @@ public class Region extends FreeColGameObject implements Nameable {
             .append(' ').append((key != null) ? key : (name != null) ? name
                 : "<unnamed>")
             .append(' ').append(type);
-        if (getDiscoverable()) sb.append('!');
+        if (getDiscoverable()) {
+            sb.append('!');
+        } else {
+            sb.append("discoverer=").append(this.discoverer);
+        }
         sb.append(']');
         return sb.toString();
     }

src/net/sf/freecol/server/control/InGameController.java

@@ -2617,7 +2617,7 @@ public final class InGameController extends Controller {
         // Try to learn
         ChangeSet cs = new ChangeSet();
         unit.setMovesLeft(0);
-        unit.csVisit(serverPlayer, is, 0, cs);
+        // csVisit has already been called in askLearnSkill
         Location loc = unit.getLocation();
         switch (is.getAlarm(serverPlayer).getLevel()) {
         case HATEFUL: // Killed, might be visible to other players.
@@ -3458,7 +3458,7 @@ public final class InGameController extends Controller {
         ChangeSet cs = new ChangeSet();
         Tile tile = is.getTile();
 
-        unit.csVisit(serverPlayer, is, -1, cs);
+        // Do not call csVisit yet, the natives might slaughter the unit first
         tile.updateIndianSettlement(serverPlayer);
         cs.add(See.only(serverPlayer), tile);
         cs.add(See.only(serverPlayer), new NationSummaryMessage(owner,

src/net/sf/freecol/server/model/ServerUnit.java

@@ -886,17 +886,17 @@ public class ServerUnit extends Unit implements TurnTaker {
     /**
      * Check for a new region.
      *
-     * @param tile The {@code Tile} to chack at.
+     * @param tile The {@code Tile} to check at.
      * @param cs A {@code ChangeSet} to update.
      */
     public void csCheckDiscoverRegion(Tile tile, ChangeSet cs) {
         final Region region = tile.getDiscoverableRegion();
-        if (region == null || !region.checkDiscover(this)) return;
         final Player owner = this.getOwner();
-        if (owner.isEuropean()) {
+        if (region != null && owner.isEuropean()
+            && region.checkDiscover(this)) {
             cs.add(See.only(owner),
-                   new NewRegionNameMessage(region, tile, this,
-                       owner.getNameForRegion(region)));
+                new NewRegionNameMessage(region, tile, this,
+                    owner.getNameForRegion(region)));
         }
     }
         

test/src/net/sf/freecol/server/generator/MapGeneratorTest.java

@@ -224,7 +224,11 @@ public class MapGeneratorTest extends FreeColTestCase {
         Player player = new Player(game, FreeColObject.ID_ATTRIBUTE_TAG);
         ServerUnit unit = new ServerUnit(game, null, player,
             spec().getUnitType("model.unit.caravel"));
-        pacific.discover(player, unit, new Turn(1));
+        assertTrue(pacific.checkDiscover(unit));
+        List<Region> discovered = pacific.discover(player, unit, new Turn(1));
+        // The Pacific sub-regions are not discoverable
+        assertEquals(1, discovered.size());
+        assertEquals(pacific, discovered.get(0));
 
         assertFalse(pacific.getDiscoverable());
         assertNull(pacific.getDiscoverableRegion());