KeywordCollection: refactor to make it work without string Map

KeywordInstance now has Original Keyword inside if needed elsewhere
Cycling better split into normal KeywordWithCost and TypeCycling into KeywordWithCostAndType

.gitattributes

@@ -603,7 +603,6 @@ forge-game/src/main/java/forge/game/event/GameEventTurnPhase.java -text
 forge-game/src/main/java/forge/game/event/GameEventZone.java -text
 forge-game/src/main/java/forge/game/event/IGameEventVisitor.java -text
 forge-game/src/main/java/forge/game/event/package-info.java -text
-forge-game/src/main/java/forge/game/keyword/Cycling.java -text
 forge-game/src/main/java/forge/game/keyword/Keyword.java -text
 forge-game/src/main/java/forge/game/keyword/KeywordCollection.java -text
 forge-game/src/main/java/forge/game/keyword/KeywordInstance.java -text

forge-game/src/main/java/forge/game/keyword/Cycling.java

@@ -1,21 +0,0 @@
-package forge.game.keyword;
-
-public class Cycling extends KeywordWithCost {
-    private String type;
-
-    public Cycling() {
-    }
-    public Cycling(String type0, String details) {
-        type = type0;
-        initialize(Keyword.CYCLING, details);
-    }
-
-    @Override
-    protected String formatReminderText(String reminderText) {
-        if (type == null) {
-            return super.formatReminderText(reminderText);
-        }
-        //handle special case of type cycling
-        return super.formatReminderText("%s, Discard this card: Search your library for a " + type + " card, reveal it, and put it into your hand. Then shuffle your library.");
-    }
-}

forge-game/src/main/java/forge/game/keyword/Keyword.java

@@ -32,7 +32,7 @@ public enum Keyword {
     CONVOKE(SimpleKeyword.class, true, "Your creatures can help cast this spell. Each creature you tap while playing this spell reduces its cost by {1} or by one mana of that creature's color."),
     CREW(KeywordWithAmount.class, true, "Tap any number of creatures you control with total power %1$d or more: This Vehicle becomes an artifact creature until end of turn."),
     CUMULATIVE_UPKEEP(KeywordWithCost.class, false, "At the beginning of your upkeep, put an age counter on this permanent, then sacrifice it unless you pay its upkeep cost for each age counter on it."),
-    CYCLING(Cycling.class, false, "%s, Discard this card: Draw a card."), //Typecycling reminder text handled by Cycling class
+    CYCLING(KeywordWithCost.class, false, "%s, Discard this card: Draw a card."), //Typecycling reminder text handled by Cycling class
     DASH(KeywordWithCost.class, true, "You may cast this spell for its dash cost. If you do, it gains haste, and it's returned from the battlefield to its owner's hand at the beginning of the next end step."),
     DEATHTOUCH(SimpleKeyword.class, true, "Any amount of damage this deals to a creature is enough to destroy it."),
     DEFENDER(SimpleKeyword.class, true, "This creature can't attack."),
@@ -134,6 +134,7 @@ public enum Keyword {
     TRANSFIGURE(KeywordWithCost.class, false, "%s, Sacrifice this creature: Search your library for a creature card with the same converted mana cost as this creature and put that card onto the battlefield. Then shuffle your library. Transfigure only as a sorcery."),
     TRANSMUTE(KeywordWithCost.class, false, "%s, Discard this card: Search your library for a card with the same converted mana cost as this card, reveal it, and put it into your hand. Then shuffle your library. Transmute only as a sorcery."),
     TRIBUTE(KeywordWithAmount.class, false, "As this creature enters the battlefield, an opponent of your choice may put {%d:+1/+1 counter} on it."),
+    TYPECYCLING(KeywordWithCostAndType.class, false, "%s, Discard this card: Search your library for a %s card, reveal it, and put it into your hand. Then shuffle your library."),
     UNDAUNTED(SimpleKeyword.class, false, "This spell costs {1} less to cast for each opponent."),
     UNDYING(SimpleKeyword.class, true, "When this creature dies, if it had no +1/+1 counters on it, return it to the battlefield under its owner's control with a +1/+1 counter on it."),
     UNEARTH(KeywordWithCost.class, false, "%s: Return this card from your graveyard to the battlefield. It gains haste. Exile it at the beginning of the next end step or if it would leave the battlefield. Unearth only as a sorcery."),
@@ -174,18 +175,7 @@ public enum Keyword {
             //check for special keywords that have a prefix before the keyword enum name
             int idx = k.indexOf(' ');
             String firstWord = idx == -1 ? enumName : enumName.substring(0, idx);
-            if (firstWord.endsWith("CYCLING")) {
+            if (firstWord.endsWith("WALK")) {
-                //handle special case of Typecycling
-                idx = firstWord.length() + 1;
-                if (idx < k.length()) {
-                    details = k.substring(idx);
-                }
-                else {
-                    details = "";
-                }
-                return new Cycling(firstWord.substring(0, firstWord.length() - 7), details);
-            }
-            else if (firstWord.endsWith("WALK")) {
                 keyword = Keyword.LANDWALK;
                 details = firstWord.substring(0, firstWord.length() - 4);
             }
@@ -209,7 +199,7 @@ public enum Keyword {
         catch (Exception e) {
             inst = new UndefinedKeyword();
         }
-        inst.initialize(keyword, details);
+        inst.initialize(k, keyword, details);
         return inst;
     }
 

forge-game/src/main/java/forge/game/keyword/KeywordCollection.java

@@ -4,11 +4,8 @@ import java.io.Serializable;
 
 import java.util.Collection;
 import java.util.Iterator;
-import java.util.Map;
-import java.util.Map.Entry;
 
 import com.google.common.collect.Multimap;
-import com.google.common.collect.Maps;
 import com.google.common.collect.MultimapBuilder;
 
 public class KeywordCollection implements Iterable<String>, Serializable {
@@ -23,20 +20,17 @@ public class KeywordCollection implements Iterable<String>, Serializable {
     }
 
     public boolean isEmpty() {
-        return stringMap.isEmpty(); //TODO: Replace with map when stringMap goes away
+        return map.isEmpty();
     }
 
     public int size() {
-        return stringMap.size(); //TODO: Replace with map when stringMap goes away
+        return map.values().size();
     }
 
     public int getAmount(Keyword keyword) {
         int amount = 0;
-        Collection<KeywordInstance<?>> instances = map.get(keyword);
+        for (KeywordInstance<?> inst : map.get(keyword)) {
-        if (instances != null) {
+            amount += inst.getAmount();
-            for (KeywordInstance<?> inst : instances) {
-                amount += inst.getAmount();
-            }
         }
         return amount;
     }
@@ -47,11 +41,6 @@ public class KeywordCollection implements Iterable<String>, Serializable {
         Collection<KeywordInstance<?>> list = map.get(keyword);
         if (list.isEmpty() || !keyword.isMultipleRedundant) {
             list.add(inst);
-            int amount = 0;
-            for (KeywordInstance<?> i : list) {
-                amount += i.getAmount();
-            }
-            stringMap.put(k, amount);
             return true;
         }
         return false;
@@ -64,18 +53,17 @@ public class KeywordCollection implements Iterable<String>, Serializable {
     }
 
     public boolean remove(String keyword) {
-        int amount = getAmount(keyword);
+        Iterator<KeywordInstance<?>> it = map.values().iterator();
-        boolean result = true;
+        
-        switch (amount) {
+        boolean result = false;
-        case 0:
+        while (it.hasNext()) {
-            result = false;
+            KeywordInstance<?> k = it.next();
-            break;
+            if (keyword.equals(k.getOriginal())) {
-        case 1:
+                it.remove();
-            stringMap.remove(keyword);
+                result = true;
-            break;
+            }
-        default:
-            stringMap.put(keyword, amount - 1);
         }
+        
         return result;
     }
 
@@ -87,43 +75,42 @@ public class KeywordCollection implements Iterable<String>, Serializable {
 
     public void clear() {
         map.clear();
-        stringMap.clear();
     }
 
-    //Below is temporary code to mimic the current List<String>
-    //TODO: Remove when keywords no longer implemented that way
-    private Map<String, Integer> stringMap = Maps.newHashMap();
     public boolean contains(String keyword) {
-        return stringMap.containsKey(keyword);
+        for (KeywordInstance<?> inst : map.values()) {
+            if (keyword.equals(inst.getOriginal())) {
+                return true;
+            }
+        }
+        return false;
     }
 
-    public int getAmount(String keyword) {
+    public int getAmount(String k) {
-        Integer amount = stringMap.get(keyword);
+        int amount = 0;
-        return amount == null ? 0 : amount.intValue();
+        for (KeywordInstance<?> inst : map.values()) {
+            if (k.equals(inst.getOriginal())) {
+                amount++;
+            }
+        }
+        return amount;
     }
 
     @Override
     public Iterator<String> iterator() {
         return new Iterator<String>() {
-            private final Iterator<Entry<String, Integer>> iterator = stringMap.entrySet().iterator();
+            private final Iterator<KeywordInstance<?>> iterator = map.values().iterator();
-            private String entryKey;
+            
-            private int entryRemainder = 0;
 
             @Override
             public boolean hasNext() {
-                return entryRemainder > 0 || iterator.hasNext();
+                return iterator.hasNext();
             }
 
             @Override
             public String next() {
-                if (entryRemainder > 0) {
+                KeywordInstance<?> entry = iterator.next();
-                    entryRemainder--;
+                return entry.getOriginal();
-                    return entryKey;
-                }
-                Entry<String, Integer> entry = iterator.next();
-                entryKey = entry.getKey();
-                entryRemainder = entry.getValue() - 1;
-                return entryKey;
             }
 
             @Override
@@ -155,8 +142,7 @@ public class KeywordCollection implements Iterable<String>, Serializable {
         }
 
         public int getAmount(String keyword) {
-            Integer amount = stringMap.get(keyword);
+            return KeywordCollection.this.getAmount(keyword);
-            return amount == null ? 0 : amount.intValue();
         }
 
         public boolean contains(Keyword keyword) {

forge-game/src/main/java/forge/game/keyword/KeywordInstance.java

@@ -7,7 +7,11 @@ import forge.util.Lang;
 
 public abstract class KeywordInstance<T extends KeywordInstance<?>> {
     private Keyword keyword;
+    private String original;
 
+    public String getOriginal() {
+        return original;
+    }
     public Keyword getKeyword() {
         return keyword;
     }
@@ -25,7 +29,8 @@ public abstract class KeywordInstance<T extends KeywordInstance<?>> {
     public int getAmount() {
         return 1;
     }
-    protected void initialize(Keyword keyword0, String details) {
+    protected void initialize(String original0, Keyword keyword0, String details) {
+        original = original0;
         keyword = keyword0;
         parse(details);
     }