Merge pull request #343 from erik-vos/master

1837: Improvements and bug fixes, part III

src/main/java/net/sf/rails/common/parser/Tag.java

@@ -276,18 +276,24 @@ private void parseSubTags(Element element) throws ConfigurationException {
                         value = attribute.getNodeValue();
                         attributes.put(name, value);
                     }
-                } else if ( "IfOption".equalsIgnoreCase(childTagName)) {
-                    Node nameAttr = nnp.getNamedItem("name");
-                    if (nameAttr == null)
-                        throw new ConfigurationException(
-                                "IfOption has no optionName attribute");
-                    name = nameAttr.getNodeValue();
-
-                    Node parmAttr = nnp.getNamedItem("parm");
-                    if (parmAttr != null) {
-                        value = parmAttr.getNodeValue();
-                        Iterable<String> parameters = Splitter.on(XMLTags.VALUES_DELIM).split(value);
-                        name = GameOption.constructParameterisedName(name, ImmutableList.copyOf(parameters));
+                } else if ("IfOption".equalsIgnoreCase(childTagName)
+                        || "IfVariant".equalsIgnoreCase(childTagName)) {
+
+                    if ("IfOption".equalsIgnoreCase(childTagName)) {
+                        Node nameAttr = nnp.getNamedItem("name");
+                        if (nameAttr == null)
+                            throw new ConfigurationException(
+                                    "IfOption has no optionName attribute");
+                        name = nameAttr.getNodeValue();
+
+                        Node parmAttr = nnp.getNamedItem("parm");
+                        if (parmAttr != null) {
+                            value = parmAttr.getNodeValue();
+                            Iterable<String> parameters = Splitter.on(XMLTags.VALUES_DELIM).split(value);
+                            name = GameOption.constructParameterisedName(name, ImmutableList.copyOf(parameters));
+                        }
+                    } else {  // IfVariant
+                        name = "Variant";
                     }
 
                     Node valueAttr = nnp.getNamedItem("value");

src/main/java/net/sf/rails/game/StartItem.java

@@ -37,6 +37,10 @@ public class StartItem extends RailsAbstractItem {
     protected int column = 0;
     protected int index;
 
+    // To allow the displayed name to include both primary and secondary.
+    // Default is the primary name (id) only.
+    protected String displayName = null;
+
     // Bids
     protected final GenericState<Player> lastBidder = new GenericState<>(this, "lastBidder");
     protected final Map<Player, CountingMoneyModel> bids = Maps.newHashMap();
@@ -271,6 +275,14 @@ public Certificate getSecondary() {
         return secondary;
     }
 
+    public String getDisplayName() {
+        return (displayName != null ? displayName : getId());
+    }
+
+    public void setDisplayName(String displayName) {
+        this.displayName = displayName;
+    }
+
     /**
      * Get the start item base price.
      *

src/main/java/net/sf/rails/game/StartPacket.java

@@ -30,6 +30,14 @@ public class StartPacket extends RailsAbstractItem {
     protected int minimumIncrement = 5;
     /** The modulus of all bids (i.e. of which value the bid must be a multiple) */
     protected int modulus = 5;
+    /** Is multiple-column display enabled?
+     *  If so, row and col attributes become mandatory for all start items
+     *  (if the <MultipleColumn> tag precedes all start items).*/
+    protected boolean multipleColumns = false;
+    /** The number of columns. Will be derived from the column attributes. */
+    protected int numberOfColumns = 1;
+    /** The number of rows. Will be derived from the row attributes, if multipleColumns is true. */
+    protected int numberOfRows;
 
     /** Default name */
     public static final String DEFAULT_ID = "Initial";
@@ -56,6 +64,12 @@ public static StartPacket create(RailsItem parent, String id, String roundClassN
      * @throws ConfigurationException if anything goes wrong.
      */
     public void configureFromXML(Tag tag) throws ConfigurationException {
+
+        // Multiple column display?
+        Tag columnsTag = tag.getChild("MultipleColumns");
+        multipleColumns = columnsTag != null;
+
+        // Bidding parameters, if applicable
         Tag biddingTag = tag.getChild("Bidding");
         if (biddingTag != null) {
             minimumInitialIncrement =
@@ -83,14 +97,27 @@ public void configureFromXML(Tag tag) throws ConfigurationException {
 
             int basePrice = itemTag.getAttributeAsInteger("basePrice", 0);
             boolean reduceable = itemTag.getAttributeAsBoolean("reduceable", false);
-            StartItem item = StartItem.create(this, itemName, itemType, basePrice, reduceable, index++, president);
+            StartItem item = StartItem.create(this, itemName, itemType,
+                    basePrice, reduceable, index++, president);
             items.add(item);
 
             // Optional attributes
             int row = itemTag.getAttributeAsInteger("row", 0);
-            if (row > 0) item.setRow(row);
+            item.setRow(row);
             int column = itemTag.getAttributeAsInteger("column", 0);
-            if (column > 0) item.setColumn(column);
+            if (multipleColumns) {
+                if (!(row > 0 && column > 0)) {
+                    throw new ConfigurationException(
+                            "With multiple columns, both row and column attributes are required");
+                }
+                item.setColumn(column);
+                numberOfRows = Math.max (numberOfRows, row);
+                numberOfColumns = Math.max (numberOfColumns, column);
+            }
+
+            // Displayed name
+            String displayName = itemTag.getAttributeAsString("displayName", null);
+            if (displayName != null) item.setDisplayName(displayName);
 
             // Check if there is another certificate
             List<Tag> subItemTags = itemTag.getChildren("SubItem");
@@ -241,4 +268,15 @@ public int getModulus() {
         return modulus;
     }
 
+    public boolean isMultipleColumns() {
+        return multipleColumns;
+    }
+
+    public int getNumberOfColumns() {
+        return numberOfColumns;
+    }
+
+    public int getNumberOfRows() {
+        return numberOfRows;
+    }
 }

src/main/java/net/sf/rails/game/TileUpgrade.java

@@ -383,12 +383,13 @@ private Rotation processRotations(HexSide side) {
             log.debug("base={} target={}", base, target);
         }
         // check if there are stations to map
-        Map<Station, Station> stationMapping = assignStations(base, target);
+       Map<Station, Station> stationMapping = assignStations(base, target);
 
         // Try something else: match sides with old and new Stations in a simple way
         // Each pair of stations that matches with the same side is connected.
         // This finally appears to work for the 1837 green Vienna upgrade.
         // Though it may only work where both base and target tiles have a fixed orientation.
+        // (may be obsolete, now that automatic relay works better (see issue #341).
         if (stationMapping == null) {
             stationMapping = new HashMap<>(6);
             Station b, t;
@@ -506,8 +507,11 @@ private Map<Station, Station> assignStations(TrackConfig base, TrackConfig targe
                 }
             }
             // check if all base and target stations are assigned
-            if (stationMap.keySet().size() != baseNb ||
-                    Sets.newHashSet(stationMap.values()).size() != targetNb) {
+            if (stationMap.keySet().size() != baseNb
+                /* Unclear why the all-stations-mapped check was applied to the new tile.
+                It inhibited upgrading 1837 Vienna to green (4 -> 6 stations).
+                || Sets.newHashSet(stationMap.values()).size() != targetNb*/
+            ) {
                 stationMap = null;
                 log.debug("Mapping: Not all stations assigned, set stationMap to null");
             }

src/main/java/net/sf/rails/ui/swing/ORUIManager.java

@@ -788,8 +788,9 @@ private void relayBaseTokens (LayTile action) {
         Tile newTile = action.getLaidTile();
         Tile oldTile = hex.getCurrentTile();
 
-        // Why does that need to be configured?
-        // Shouldn't tokens always be relaid??
+        // Check if manual token relay is required.
+        // This was an emergency measure in cases where automatic relay
+        // did not work (e.g. 1837 tile 427). Now probably obsolete.
         if (!action.isRelayBaseTokens()
                 && !oldTile.relayBaseTokensOnUpgrade()) return; // is deprecated
 
@@ -798,7 +799,6 @@ private void relayBaseTokens (LayTile action) {
         /* Check which tokens must be relaid, and in which sequence.
          * Ideally, the game engine should instruct the UI what to do
          * if there is more than one stop and more than one token.
-         * TODO LayTile does not yet allow that.
          *
          * For now, the only case that needs special handling is the 1835 BA home hex L6,
          * where it it possible to have two tokens laid before even one tile.