Refactor TrackableObjects to support copy forward changed properties without completely overwriting references

forge-game/src/main/java/forge/game/GameView.java

@@ -37,7 +37,7 @@ public class GameView extends TrackableObject {
         set(TrackableProperty.WinningTeam, -1);
 
         GameRules rules = game.getRules();
-        set(TrackableProperty.Commander, rules.hasCommander());
+        set(TrackableProperty.IsCommander, rules.hasCommander());
         set(TrackableProperty.GameType, rules.getGameType());
         set(TrackableProperty.PoisonCountersToLose, rules.getPoisonCountersToLose());
         set(TrackableProperty.NumGamesInMatch, rules.getGamesPerMatch());
@@ -56,7 +56,7 @@ public class GameView extends TrackableObject {
         return get(TrackableProperty.Title);
     }
     public boolean isCommander() {
-        return get(TrackableProperty.Commander);
+        return get(TrackableProperty.IsCommander);
     }
     public GameType getGameType() {
         return get(TrackableProperty.GameType);

forge-game/src/main/java/forge/trackable/TrackableCollection.java

@@ -18,16 +18,4 @@ public class TrackableCollection<T extends TrackableObject> extends FCollection<
     public TrackableCollection(Iterable<T> i) {
         super(i);
     }
-
-    @Override
-    public boolean add(T item) {
-        //TODO: Track change
-        return super.add(item);
-    }
-
-    @Override
-    public boolean remove(Object item) {
-        //TODO: Track change
-        return super.remove(item);
-    }
 }

forge-game/src/main/java/forge/trackable/TrackableObject.java

@@ -64,26 +64,11 @@ public abstract class TrackableObject implements IIdentifiable, Serializable {
     }
 
     /**
-     * Copy to this Trackable object the first object of the provided iterator
+     * Copy all change properties of another Trackable object to this object.
-     * whose id matches.
-     * 
-     * @see TrackableObject#copy(TrackableObject)
      */
-    public final void copy(final Iterable<? extends TrackableObject> others) {
+    public final void copyChangedProps(final TrackableObject from) {
-        for (final TrackableObject other : others) {
+        for (final TrackableProperty prop : from.changedProps) {
-            if (this.equals(other)) {
+            prop.copyChangedProps(from, this);
-                copy(other);
-                break;
-            }
-        }
-    }
-
-    /**
-     * Copy all properties of another Trackable object to this object.
-     */
-    public final void copy(final TrackableObject other) {
-        for (final TrackableProperty prop : other.props.keySet()) {
-            set(prop, other.get(prop));
         }
     }
 

forge-game/src/main/java/forge/trackable/TrackableProperty.java

@@ -166,6 +166,10 @@ public enum TrackableProperty {
         return respectFreeze;
     }
 
+    public void copyChangedProps(TrackableObject from, TrackableObject to) {
+        type.copyChangedProps(from, to, this);
+    }
+
     @SuppressWarnings("unchecked")
     public <T> T getDefaultValue() {
         return ((TrackableType<T>)type).getDefaultValue();

forge-game/src/main/java/forge/trackable/TrackableTypes.java

@@ -24,11 +24,72 @@ public class TrackableTypes {
         private TrackableType() {
         }
 
+        protected void copyChangedProps(TrackableObject from, TrackableObject to, TrackableProperty prop) {
+            to.set(prop, from.get(prop));
+        }
         protected abstract T getDefaultValue();
         protected abstract T deserialize(TrackableDeserializer td, T oldValue);
         protected abstract void serialize(TrackableSerializer ts, T value);
     }
 
+    public static abstract class TrackableObjectType<T extends TrackableObject> extends TrackableType<T> {
+        private final HashMap<Integer, T> objLookup = new HashMap<Integer, T>();
+
+        private TrackableObjectType() {
+        }
+
+        public void clearLookupDictionary() {
+            objLookup.clear();
+        }
+
+        @Override
+        protected void copyChangedProps(TrackableObject from, TrackableObject to, TrackableProperty prop) {
+            T newObj = from.get(prop);
+            if (newObj != null) {
+                T existingObj = objLookup.get(newObj.getId());
+                if (existingObj != null) { //if object exists already, update its changed properties
+                    existingObj.copyChangedProps(newObj);
+                    newObj = existingObj;
+                }
+                else { //if object is new, cache in object lookup
+                    objLookup.put(newObj.getId(), newObj);
+                }
+            }
+            to.set(prop, newObj);
+        }
+    }
+
+    private static abstract class TrackableCollectionType<T extends TrackableObject> extends TrackableType<TrackableCollection<T>> {
+        private final TrackableObjectType<T> itemType;
+
+        private TrackableCollectionType(TrackableObjectType<T> itemType0) {
+            itemType = itemType0;
+        }
+
+        @Override
+        protected void copyChangedProps(TrackableObject from, TrackableObject to, TrackableProperty prop) {
+            TrackableCollection<T> newCollection = from.get(prop);
+            if (newCollection != null) {
+                //swap in objects in old collection for objects in new collection
+                for (int i = 0; i < newCollection.size(); i++) {
+                    T newObj = newCollection.get(i);
+                    if (newObj != null) {
+                        T existingObj = itemType.objLookup.get(newObj.getId());
+                        if (existingObj != null) { //if object exists already, update its changed properties
+                            existingObj.copyChangedProps(newObj);
+                            newCollection.remove(i);
+                            newCollection.add(i, existingObj);
+                        }
+                        else { //if object is new, cache in object lookup
+                            itemType.objLookup.put(newObj.getId(), newObj);
+                        }
+                    }
+                }
+            }
+            to.set(prop, newCollection);
+        }
+    }
+
     public static final TrackableType<Boolean> BooleanType = new TrackableType<Boolean>() {
         @Override
         public Boolean getDefaultValue() {
@@ -111,7 +172,7 @@ public class TrackableTypes {
         return type;
     }
 
-    public static final TrackableType<CardView> CardViewType = new TrackableType<CardView>() {
+    public static final TrackableObjectType<CardView> CardViewType = new TrackableObjectType<CardView>() {
         @Override
         protected CardView getDefaultValue() {
             return null;
@@ -131,7 +192,7 @@ public class TrackableTypes {
             ts.write(value == null ? -1 : value.getId()); //just write ID for lookup via index when deserializing
         }
     };
-    public static final TrackableType<TrackableCollection<CardView>> CardViewCollectionType = new TrackableType<TrackableCollection<CardView>>() {
+    public static final TrackableCollectionType<CardView> CardViewCollectionType = new TrackableCollectionType<CardView>(CardViewType) {
         @Override
         protected TrackableCollection<CardView> getDefaultValue() {
             return null;
@@ -147,7 +208,7 @@ public class TrackableTypes {
             ts.write(value);
         }
     };
-    public static final TrackableType<CardStateView> CardStateViewType = new TrackableType<CardStateView>() {
+    public static final TrackableObjectType<CardStateView> CardStateViewType = new TrackableObjectType<CardStateView>() {
         @Override
         protected CardStateView getDefaultValue() {
             return null;
@@ -191,7 +252,7 @@ public class TrackableTypes {
             }
         }
     };
-    public static final TrackableType<PlayerView> PlayerViewType = new TrackableType<PlayerView>() {
+    public static final TrackableObjectType<PlayerView> PlayerViewType = new TrackableObjectType<PlayerView>() {
         @Override
         protected PlayerView getDefaultValue() {
             return null;
@@ -211,7 +272,7 @@ public class TrackableTypes {
             ts.write(value == null ? -1 : value.getId()); //just write ID for lookup via index when deserializing
         }
     };
-    public static final TrackableType<TrackableCollection<PlayerView>> PlayerViewCollectionType = new TrackableType<TrackableCollection<PlayerView>>() {
+    public static final TrackableCollectionType<PlayerView> PlayerViewCollectionType = new TrackableCollectionType<PlayerView>(PlayerViewType) {
         @Override
         protected TrackableCollection<PlayerView> getDefaultValue() {
             return null;
@@ -227,7 +288,7 @@ public class TrackableTypes {
             ts.write(value);
         }
     };
-    public static final TrackableType<GameEntityView> GameEntityViewType = new TrackableType<GameEntityView>() {
+    public static final TrackableObjectType<GameEntityView> GameEntityViewType = new TrackableObjectType<GameEntityView>() {
         @Override
         protected GameEntityView getDefaultValue() {
             return null;
@@ -262,7 +323,7 @@ public class TrackableTypes {
             }
         }
     };
-    public static final TrackableType<StackItemView> StackItemViewType = new TrackableType<StackItemView>() {
+    public static final TrackableObjectType<StackItemView> StackItemViewType = new TrackableObjectType<StackItemView>() {
         @Override
         protected StackItemView getDefaultValue() {
             return null;
@@ -284,7 +345,7 @@ public class TrackableTypes {
             }
         }
     };
-    public static final TrackableType<TrackableCollection<StackItemView>> StackItemViewListType = new TrackableType<TrackableCollection<StackItemView>>() {
+    public static final TrackableCollectionType<StackItemView> StackItemViewListType = new TrackableCollectionType<StackItemView>(StackItemViewType) {
         @Override
         protected TrackableCollection<StackItemView> getDefaultValue() {
             return new TrackableCollection<StackItemView>();

forge-gui-desktop/src/main/java/forge/screens/match/CMatchUI.java

@@ -174,14 +174,14 @@ public final class CMatchUI
     }
 
     @Override
-    public void setGameView(final GameView gameView) {
+    public void setGameView(GameView gameView0) {
-        super.setGameView(gameView);
+        super.setGameView(gameView0);
-        cDetailPicture.setGameView(gameView);
+        gameView0 = getGameView(); //ensure updated game view used for below logic
-        screen.setTabCaption(gameView.getTitle());
+        if (gameView0 == null) { return; }
+
+        cDetailPicture.setGameView(gameView0);
+        screen.setTabCaption(gameView0.getTitle());
         if (sortedPlayers != null) {
-            for (final PlayerView pv : sortedPlayers) {
-                pv.copy(gameView.getPlayers());
-            }
             FThreads.invokeInEdtNowOrLater(new Runnable() {
                 @Override public final void run() {
                     for (final VField f : getFieldViews()) {

forge-gui/src/main/java/forge/match/AbstractGuiGame.java

@@ -65,8 +65,15 @@ public abstract class AbstractGuiGame implements IGuiGame, IMayViewCards {
         return gameView;
     }
     @Override
-    public void setGameView(final GameView gameView) {
+    public void setGameView(final GameView gameView0) {
-        this.gameView = gameView;
+        if (gameView == null || gameView0 == null) {
+            gameView = gameView0;
+            return;
+        }
+
+        //if game view set to another instance without being first cleared,
+        //update existing game view object instead of overwriting it
+        gameView.copyChangedProps(gameView0);
     }
 
     public final IGameController getGameController() {

forge-gui/src/main/java/forge/match/HostedMatch.java

@@ -44,6 +44,7 @@ import forge.quest.QuestController;
 import forge.sound.MusicPlaylist;
 import forge.sound.SoundSystem;
 import forge.trackable.TrackableCollection;
+import forge.trackable.TrackableTypes;
 import forge.util.CollectionSuppliers;
 import forge.util.collect.FCollectionView;
 import forge.util.maps.HashMapOfLists;
@@ -127,6 +128,11 @@ public class HostedMatch {
     }
 
     public void startGame() {
+        //ensure lookup dictionaries are cleared before each game
+        TrackableTypes.CardViewType.clearLookupDictionary();
+        TrackableTypes.PlayerViewType.clearLookupDictionary();
+        TrackableTypes.StackItemViewType.clearLookupDictionary();
+
         nextGameDecisions.clear();
         SoundSystem.instance.setBackgroundMusic(MusicPlaylist.MATCH);
 
@@ -168,6 +174,7 @@ public class HostedMatch {
                 final PlayerControllerHuman humanController = (PlayerControllerHuman) p.getController();
                 final IGuiGame gui = guis.get(p.getRegisteredPlayer());
                 humanController.setGui(gui);
+                gui.setGameView(null); //clear out game view first so we don't copy into old game view
                 gui.setGameView(gameView);
                 gui.setOriginalGameController(p.getView(), humanController);