From 8adb5dee558569eee4783fe0c1836cff198a7bd3 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Wed, 22 May 2024 22:13:58 -0700 Subject: [PATCH 1/2] Base zone import on the copy constructor The copy constructor already guarantees that everything has independent copies made - and if it missed something that would be a bug. So it's appealing to use it for map imports where we want the same thing. Also preserve the creation time in the copy constructor when `keepIds` so that it is preserved when campaigns are copied and such. `Zone#import()` no longer needs to do as much. All it does not is clear the initiative list. --- .../java/net/rptools/maptool/model/Zone.java | 19 +++---------------- .../rptools/maptool/util/PersistenceUtil.java | 3 +++ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/main/java/net/rptools/maptool/model/Zone.java b/src/main/java/net/rptools/maptool/model/Zone.java index 643bfd7cbb..2809c656bb 100644 --- a/src/main/java/net/rptools/maptool/model/Zone.java +++ b/src/main/java/net/rptools/maptool/model/Zone.java @@ -478,10 +478,6 @@ public String toString() { drawablesByLayer.put(Layer.BACKGROUND, backgroundDrawables); } - /** - * Note: When adding new fields to this class, make sure to update all constructors, {@link - * #imported()}, {@link #readResolve()}, and potentially {@link #optimize()}. - */ public Zone() { // TODO: Was this needed? // setGrid(new SquareGrid()); @@ -611,16 +607,11 @@ public DrawablePaint getFogPaint() { } /** - * Note: When adding new fields to this class, make sure to update all constructors, {@link - * #imported()}, {@link #readResolve()}, and potentially {@link #optimize()}. + * Create a new zone with old zone's properties and with new token ids. * *

JFJ 2010-10-27 Don't forget that since there are new zones AND new tokens created here from * the old one being passed in, if you have any data that needs to transfer over, you will need to * manually copy it as is done below for various items. - */ - - /** - * Create a new zone with old zone's properties and with new token ids. * * @param zone The zone to copy. */ @@ -637,6 +628,7 @@ public Zone(Zone zone) { public Zone(Zone zone, boolean keepIds) { if (keepIds) { this.id = zone.getId(); + this.creationTime = zone.creationTime; } backgroundPaint = zone.backgroundPaint; @@ -755,14 +747,9 @@ public GUID getId() { /** * Should be invoked only when a Zone has been imported from an external source and needs to be - * cleaned up before being used. Currently this cleanup consists of allocating a new GUID, setting - * the creation time to `now', and resetting the initiative list (setting the related zone and - * clearing the model). + * cleaned up before being used. */ public void imported() { - id = new GUID(); - creationTime = System.currentTimeMillis(); - initiativeList.setZone(this); initiativeList.clearModel(); } diff --git a/src/main/java/net/rptools/maptool/util/PersistenceUtil.java b/src/main/java/net/rptools/maptool/util/PersistenceUtil.java index 21254eb2b4..d570cecd72 100644 --- a/src/main/java/net/rptools/maptool/util/PersistenceUtil.java +++ b/src/main/java/net/rptools/maptool/util/PersistenceUtil.java @@ -320,6 +320,9 @@ public static PersistedMap loadMap(File mapFile) throws IOException { z.setName(n); z.imported(); // Resets creation timestamp and init panel, among other things z.optimize(); // Collapses overlaid or redundant drawables + + // Make sure the imported zone is as fresh as possible (new IDs all the way down). + persistedMap.zone = new Zone(z, false); } else { // TODO: Not a map but it is something with a property.xml file in it. // Should we have a filetype property in there? From 414ceacb1c3bd372861093f0f13a61c4c52a3ce0 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Wed, 22 May 2024 22:20:44 -0700 Subject: [PATCH 2/2] Zone creation time is not being used as its documentation suggests ZoneRenderers are not actually being compared based on the creation time. If they were at some point, they aren't anymore. No more need for ZoneRenderer to implement `Comparable<>`. --- .../client/ui/zone/renderer/ZoneRenderer.java | 13 +------------ src/main/java/net/rptools/maptool/model/Zone.java | 6 ------ 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/renderer/ZoneRenderer.java b/src/main/java/net/rptools/maptool/client/ui/zone/renderer/ZoneRenderer.java index 6e35a14be8..e4040b318c 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/renderer/ZoneRenderer.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/renderer/ZoneRenderer.java @@ -77,11 +77,9 @@ import net.rptools.parser.ParserException; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.jetbrains.annotations.NotNull; /** */ -public class ZoneRenderer extends JComponent - implements DropTargetListener, Comparable { +public class ZoneRenderer extends JComponent implements DropTargetListener { private static final long serialVersionUID = 3832897780066104884L; private static final Logger log = LogManager.getLogger(ZoneRenderer.class); @@ -3566,15 +3564,6 @@ private void onGridChanged(GridChanged event) { repaintDebouncer.dispatch(); } - // - // COMPARABLE - public int compareTo(@NotNull ZoneRenderer o) { - if (o != this) { - return (int) (zone.getCreationTime() - o.zone.getCreationTime()); - } - return 0; - } - // Begin token common macro identification private List highlightCommonMacros = new ArrayList(); diff --git a/src/main/java/net/rptools/maptool/model/Zone.java b/src/main/java/net/rptools/maptool/model/Zone.java index 2809c656bb..74e409bac3 100644 --- a/src/main/java/net/rptools/maptool/model/Zone.java +++ b/src/main/java/net/rptools/maptool/model/Zone.java @@ -363,12 +363,6 @@ public String toString() { public static final DrawablePaint DEFAULT_FOG = new DrawableColorPaint(Color.black); - // The zones should be ordered. We could have the server assign each zone - // an incrementing number as new zones are created, but that would take a lot - // more elegance than we really need. Instead, let's just keep track of the - // time when it was created. This should give us sufficient granularity, because - // seriously -- what's the likelihood of two GMs separately creating a new zone at exactly - // the same millisecond since the epoch? private long creationTime = System.currentTimeMillis(); private GUID id = new GUID(); // Ideally would be 'final', but that complicates imported()