From 937fd763c5f09e969a70b346c373edd7ef88b9d9 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Tue, 24 Oct 2023 10:56:25 -0700 Subject: [PATCH 1/8] Make LightSource immutable so they don't need to be cloned. This involved reworking `CampaignPropertiesDialog` and `LightSourceCreator` as the only places the needed to mutate `LightSource`. New static factories make it clearer which kind of light is being created while enforcing nullability that doesn't agree with the "primary" constructor. The light's ID must be provided by the caller now instead of `LightSource` deciding it for itself (avoids the need for `setId()`). `Light` was already in practice immutable, but now both `Light` and `LightSource` are `final` classes with `final` fields. `LightSource.lightList` remains a mutable list for serialization purposes, but is guarded to prevent any mutations. --- .../CampaignPropertiesDialog.java | 48 +++++++---- .../java/net/rptools/maptool/model/Light.java | 2 +- .../rptools/maptool/model/LightSource.java | 85 +++++++++---------- .../net/rptools/maptool/model/SightType.java | 5 +- .../maptool/tool/LightSourceCreator.java | 46 +++++----- 5 files changed, 102 insertions(+), 84 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/campaignproperties/CampaignPropertiesDialog.java b/src/main/java/net/rptools/maptool/client/ui/campaignproperties/CampaignPropertiesDialog.java index f968d99e4a..f143c3ea49 100644 --- a/src/main/java/net/rptools/maptool/client/ui/campaignproperties/CampaignPropertiesDialog.java +++ b/src/main/java/net/rptools/maptool/client/ui/campaignproperties/CampaignPropertiesDialog.java @@ -24,6 +24,7 @@ import java.io.LineNumberReader; import java.io.StringReader; import java.text.ParseException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -488,7 +489,8 @@ private void commitSightMap(final String text) { } // Parse Details double magnifier = 1; - LightSource personalLight = null; + // If null, no personal light has been defined. + List personalLightLights = null; String[] args = value.split("\\s+"); ShapeType shape = ShapeType.CIRCLE; @@ -543,11 +545,10 @@ private void commitSightMap(final String text) { } } - if (personalLight == null) { - personalLight = new LightSource(); - personalLight.setType(LightSource.Type.NORMAL); + if (personalLightLights == null) { + personalLightLights = new ArrayList<>(); } - personalLight.add( + personalLightLights.add( new Light( shape, 0, @@ -559,7 +560,6 @@ private void commitSightMap(final String text) { perRangeLumens, false, false)); - personalLight.setScaleWithToken(scaleWithToken); } else { throw new ParseException( String.format("Unrecognized personal light syntax: %s", arg), 0); @@ -588,6 +588,11 @@ private void commitSightMap(final String text) { errlog.add(I18N.getText(errmsg, reader.getLineNumber(), toBeParsed)); } } + + LightSource personalLight = + personalLightLights == null + ? null + : LightSource.createPersonal(scaleWithToken, personalLightLights); SightType sight = new SightType(label, magnifier, personalLight, shape, arc, scaleWithToken); sight.setDistance(range); @@ -670,14 +675,22 @@ private Map> commitLightMap( continue; } + // region Light source properties. String name = line.substring(0, split).trim(); - LightSource lightSource = new LightSource(name); + GUID id = new GUID(); + LightSource.Type type = LightSource.Type.NORMAL; + boolean scaleWithToken = false; + List lights = new ArrayList<>(); + // endregion + // region Individual light properties ShapeType shape = ShapeType.CIRCLE; // TODO: Make a preference for default shape double arc = 0; double offset = 0; boolean gmOnly = false; boolean owner = false; String distance = null; + // endregion + for (String arg : line.substring(split + 1).split("\\s+")) { arg = arg.trim(); if (arg.length() == 0) { @@ -695,7 +708,7 @@ private Map> commitLightMap( } // Scale with token designation if (arg.equalsIgnoreCase("SCALE")) { - lightSource.setScaleWithToken(true); + scaleWithToken = true; continue; } // Shape designation ? @@ -708,8 +721,7 @@ private Map> commitLightMap( // Type designation ? try { - LightSource.Type type = LightSource.Type.valueOf(arg.toUpperCase()); - lightSource.setType(type); + type = LightSource.Type.valueOf(arg.toUpperCase()); continue; } catch (IllegalArgumentException iae) { // Expected when not defining a shape @@ -770,7 +782,7 @@ private Map> commitLightMap( } } - boolean isAura = lightSource.getType() == LightSource.Type.AURA; + boolean isAura = type == LightSource.Type.AURA; if (!isAura && (gmOnly || owner)) { errlog.add(I18N.getText("msg.error.mtprops.light.gmOrOwner", reader.getLineNumber())); gmOnly = false; @@ -788,24 +800,26 @@ private Map> commitLightMap( perRangeLumens, gmOnly, owner); - lightSource.add(t); + lights.add(t); } catch (ParseException pe) { errlog.add( I18N.getText("msg.error.mtprops.light.distance", reader.getLineNumber(), distance)); } } - // Keep ID the same if modifying existing light - // TODO FJE Why? Is there some benefit to doing so? Changes to light sources require the map - // to be re-rendered anyway, don't they? + // Keep ID the same if modifying existing light. This avoids tokens losing their lights when + // the light definition is modified. if (originalLightSourcesMap.containsKey(currentGroupName)) { for (LightSource ls : originalLightSourcesMap.get(currentGroupName).values()) { if (ls.getName().equalsIgnoreCase(name)) { - lightSource.setId(ls.getId()); + assert ls.getId() != null; + id = ls.getId(); break; } } } - lightSourceMap.put(lightSource.getId(), lightSource); + + final var source = LightSource.createRegular(name, id, type, scaleWithToken, lights); + lightSourceMap.put(source.getId(), source); } // Last group if (currentGroupName != null) { diff --git a/src/main/java/net/rptools/maptool/model/Light.java b/src/main/java/net/rptools/maptool/model/Light.java index 67981b9dce..95315a87a3 100644 --- a/src/main/java/net/rptools/maptool/model/Light.java +++ b/src/main/java/net/rptools/maptool/model/Light.java @@ -23,7 +23,7 @@ import net.rptools.maptool.server.proto.LightDto; import net.rptools.maptool.server.proto.ShapeTypeDto; -public class Light implements Serializable { +public final class Light implements Serializable { private final @Nonnull ShapeType shape; private final double facingOffset; private final double radius; diff --git a/src/main/java/net/rptools/maptool/model/LightSource.java b/src/main/java/net/rptools/maptool/model/LightSource.java index a633a70e99..88ba8eb722 100644 --- a/src/main/java/net/rptools/maptool/model/LightSource.java +++ b/src/main/java/net/rptools/maptool/model/LightSource.java @@ -14,6 +14,7 @@ */ package net.rptools.maptool.model; +import com.google.common.collect.ImmutableList; import com.google.protobuf.StringValue; import java.awt.geom.Area; import java.io.IOException; @@ -22,7 +23,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; @@ -33,16 +33,21 @@ import net.rptools.maptool.server.proto.LightSourceDto; import org.apache.commons.lang.math.NumberUtils; -public class LightSource implements Comparable, Serializable { +/** + * Represents a light source that can be attached to tokens. + * + *

This class is immutable. + */ +public final class LightSource implements Comparable, Serializable { public enum Type { NORMAL, AURA } - private @Nullable String name; - private @Nullable GUID id; - private @Nonnull Type type; - private boolean scaleWithToken; + private final @Nullable String name; + private final @Nullable GUID id; + private final @Nonnull Type type; + private final boolean scaleWithToken; private final @Nonnull List lightList; // Lumens are now in the individual Lights. This field is only here for backwards compatibility @@ -54,21 +59,32 @@ public enum Type { * *

Since a personal light source is directly attached to a specific sight type, they do not * need (or have) names and GUIDs. + * + * @param scaleWithToken if {@code true}, the size of the lights will scale with the token size. + * @param lights The set of lights that constitute the personal light source. */ - public LightSource() { - this(null, null, Type.NORMAL, false, Collections.emptyList()); + public static LightSource createPersonal(boolean scaleWithToken, Collection lights) { + return new LightSource(null, null, Type.NORMAL, scaleWithToken, ImmutableList.copyOf(lights)); } /** * Constructs a non-personal light source. * - *

These light sources are referenced both by name and GUID, and thus need both. A new GUID - * will be created automatically. + *

These light sources are referenced both by name and GUID, and thus need both. * * @param name The name of the light source. + * @param id The unique ID of the light source. + * @param type The type of light, whether a normal light or an aura. + * @param scaleWithToken if {@code true}, the size of the lights will scale with the token size. + * @param lights The set of lights that constitute the personal light source. */ - public LightSource(@Nonnull String name) { - this(name, new GUID(), Type.NORMAL, false, Collections.emptyList()); + public static LightSource createRegular( + @Nonnull String name, + @Nonnull GUID id, + @Nonnull Type type, + boolean scaleWithToken, + @Nonnull Collection lights) { + return new LightSource(name, id, type, scaleWithToken, ImmutableList.copyOf(lights)); } private LightSource( @@ -76,14 +92,19 @@ private LightSource( @Nullable GUID id, @Nonnull Type type, boolean scaleWithToken, - @Nonnull Collection lights) { + @Nonnull List lights) { this.name = name; this.id = id; this.type = type; this.scaleWithToken = scaleWithToken; + this.lightList = lights; + } - this.lightList = new LinkedList<>(); - this.lightList.addAll(lights); + @Serial + public Object writeReplace() { + // Make sure XStream keeps the serialization nice. We don't need the XML to contain + // implementation details of the ImmutableList in use. + return new LightSource(name, id, type, scaleWithToken, new ArrayList<>(lightList)); } @SuppressWarnings("ConstantConditions") @@ -93,7 +114,7 @@ private LightSource( Objects.requireNonNullElse(lightList, Collections.emptyList()); final List lights; if (lumens == Integer.MIN_VALUE) { - // This is an up-to-date Lightsource with lumens already stored in the Lights. + // This is an up-to-date LightSource with lumens already stored in the Lights. lights = originalLights; } else { // This is an old light source with a lumens value that needs to be pushed into the individual @@ -120,7 +141,7 @@ private LightSource( this.id, Objects.requireNonNullElse(this.type, Type.NORMAL), this.scaleWithToken, - lights); + ImmutableList.copyOf(lights)); } @Override @@ -144,10 +165,6 @@ public int hashCode() { return Objects.hashCode(id); } - public void setId(@Nonnull GUID id) { - this.id = id; - } - public @Nullable GUID getId() { return id; } @@ -156,37 +173,17 @@ public void setId(@Nonnull GUID id) { return name; } - public void setName(@Nonnull String name) { - this.name = name; - } - - public void add(@Nonnull Light source) { - lightList.add(source); - } - - public void remove(@Nonnull Light source) { - lightList.remove(source); - } - /** - * @return the lights belonging to this LightSource. + * @return A read-only list of lights belonging to this LightSource */ public @Nonnull List getLightList() { - return Collections.unmodifiableList(lightList); + return lightList; } public @Nonnull Type getType() { return type; } - public void setType(@Nonnull Type type) { - this.type = type; - } - - public void setScaleWithToken(boolean scaleWithToken) { - this.scaleWithToken = scaleWithToken; - } - public boolean isScaleWithToken() { return scaleWithToken; } @@ -255,7 +252,7 @@ public int compareTo(@Nonnull LightSource o) { dto.hasId() ? GUID.valueOf(dto.getId().getValue()) : null, Type.valueOf(dto.getType().name()), dto.getScaleWithToken(), - dto.getLightsList().stream().map(Light::fromDto).toList()); + dto.getLightsList().stream().map(Light::fromDto).collect(ImmutableList.toImmutableList())); } public @Nonnull LightSourceDto toDto() { diff --git a/src/main/java/net/rptools/maptool/model/SightType.java b/src/main/java/net/rptools/maptool/model/SightType.java index 42abf38eab..083d1db9e5 100644 --- a/src/main/java/net/rptools/maptool/model/SightType.java +++ b/src/main/java/net/rptools/maptool/model/SightType.java @@ -15,6 +15,7 @@ package net.rptools.maptool.model; import java.awt.geom.Area; +import javax.annotation.Nullable; import net.rptools.maptool.server.proto.ShapeTypeDto; import net.rptools.maptool.server.proto.SightTypeDto; @@ -64,12 +65,12 @@ public SightType() { // For serialization } - public SightType(String name, double multiplier, LightSource personalLightSource) { + public SightType(String name, double multiplier, @Nullable LightSource personalLightSource) { this(name, multiplier, personalLightSource, ShapeType.CIRCLE); } public SightType( - String name, double multiplier, LightSource personalLightSource, ShapeType shape) { + String name, double multiplier, @Nullable LightSource personalLightSource, ShapeType shape) { this.name = name; this.multiplier = multiplier; this.personalLightSource = personalLightSource; diff --git a/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java b/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java index a6d2fe74bf..f752fd6ab6 100644 --- a/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java +++ b/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Map; import net.rptools.lib.FileUtil; +import net.rptools.maptool.model.GUID; import net.rptools.maptool.model.Light; import net.rptools.maptool.model.LightSource; import net.rptools.maptool.model.ShapeType; @@ -57,28 +58,33 @@ public static void main(String[] args) { } private static LightSource createLightSource(String name, double radius, double arcAngle) { - LightSource source = new LightSource(name); - // source.add(new Light(0, 5, arcAngle, new DrawableColorPaint(new Color(255, 255, 0, 50)))); - source.add(new Light(ShapeType.CIRCLE, 0, radius, arcAngle, null, 100, false, false)); - return source; + return LightSource.createRegular( + name, + new GUID(), + LightSource.Type.NORMAL, + false, + List.of( + // new Light(0, 5, arcAngle, new DrawableColorPaint(new Color(255, 255, 0, 50))) + new Light(ShapeType.CIRCLE, 0, radius, arcAngle, null, 100, false, false))); } private static LightSource createD20LightSource(String name, double radius, double arcAngle) { - LightSource source = new LightSource(name); - - // source.add(new Light(0, 5, arcAngle, new DrawableColorPaint(new Color(255, 255, 0, 50)))); - source.add(new Light(ShapeType.CIRCLE, 0, radius, arcAngle, null, 100, false, false)); - source.add( - new Light( - ShapeType.CIRCLE, - 0, - radius * 2, - arcAngle, - new DrawableColorPaint(new Color(0, 0, 0, 100)), - 100, - false, - false)); - - return source; + return LightSource.createRegular( + name, + new GUID(), + LightSource.Type.NORMAL, + false, + List.of( + // new Light(0, 5, arcAngle, new DrawableColorPaint(new Color(255, 255, 0, 50))) + new Light(ShapeType.CIRCLE, 0, radius, arcAngle, null, 100, false, false), + new Light( + ShapeType.CIRCLE, + 0, + radius * 2, + arcAngle, + new DrawableColorPaint(new Color(0, 0, 0, 100)), + 100, + false, + false))); } } From 69c342dd327055c1e72bc476c34df6d8eb5c4c47 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Tue, 24 Oct 2023 12:02:10 -0700 Subject: [PATCH 2/8] Simplify existing light source system The protocol for adding and removing lights was streamlined: since the light source GUID is the only piece needed for this functionality, that is now all that gets passed around via protobuf. Previously the entire light source definition was passed around for these operations. `AttachedLightSource` is now more encapsulated so that resolving `LightSource` from `GUID` is easier. Callers no longer need to bother with grabbing the `GUID` from the `AttachedLightSource`, nor do they have to know where to use it. Instead they can just ask the `AttachedLightSource` to do the lookup, providing only the `Campaign` for context. This will enable future changes to how light sources can looked up. `AttachedLightSource` has also been made immutable by finalizing the class and `lightSourceId` field. --- .../client/ServerCommandClientImpl.java | 10 +---- .../client/ui/AbstractTokenPopupMenu.java | 4 +- .../ui/zone/LightSourceIconOverlay.java | 3 +- .../maptool/client/ui/zone/ZoneView.java | 11 +++-- .../maptool/model/AttachedLightSource.java | 43 +++++++++++++------ .../net/rptools/maptool/model/Campaign.java | 17 -------- .../java/net/rptools/maptool/model/Token.java | 40 ++++++++--------- .../rptools/maptool/server/ServerCommand.java | 2 - src/main/proto/data_transfer_objects.proto | 2 +- 9 files changed, 61 insertions(+), 71 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java b/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java index 0f83ff82f1..33722cd2e4 100644 --- a/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java +++ b/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java @@ -674,18 +674,10 @@ public void updateTokenProperty(Token token, Token.Update update, String value) @Override public void updateTokenProperty(Token token, Token.Update update, LightSource value) { - updateTokenProperty( - token, update, TokenPropertyValueDto.newBuilder().setLightSource(value.toDto()).build()); - } - - @Override - public void updateTokenProperty( - Token token, Token.Update update, LightSource value1, String value2) { updateTokenProperty( token, update, - TokenPropertyValueDto.newBuilder().setLightSource(value1.toDto()).build(), - TokenPropertyValueDto.newBuilder().setStringValue(value2).build()); + TokenPropertyValueDto.newBuilder().setLightSourceId(value.getId().toString()).build()); } @Override diff --git a/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java b/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java index 8cab678f65..03ed55c127 100644 --- a/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java +++ b/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java @@ -466,9 +466,9 @@ public void actionPerformed(ActionEvent e) { continue; } if (token.hasLightSource(lightSource)) { - token.removeLightSource(lightSource); + token.removeLightSource(lightSource.getId()); } else { - token.addLightSource(lightSource); + token.addLightSource(lightSource.getId()); } MapTool.serverCommand().putToken(renderer.getZone().getId(), token); diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java b/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java index 6561151fcd..d81a05f3f0 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java @@ -34,8 +34,7 @@ public void paintOverlay(ZoneRenderer renderer, Graphics2D g) { if (token.hasLightSources()) { boolean foundNormalLight = false; for (AttachedLightSource attachedLightSource : token.getLightSources()) { - LightSource lightSource = - MapTool.getCampaign().getLightSource(attachedLightSource.getLightSourceId()); + LightSource lightSource = attachedLightSource.resolve(MapTool.getCampaign()); if (lightSource != null && lightSource.getType() == LightSource.Type.NORMAL) { foundNormalLight = true; break; diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java b/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java index b3c728bd56..2551aba084 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java @@ -294,8 +294,7 @@ private List calculateLitAreas(Token lightSourceToken, double final var result = new ArrayList(); for (final var attachedLightSource : lightSourceToken.getLightSources()) { - LightSource lightSource = - MapTool.getCampaign().getLightSource(attachedLightSource.getLightSourceId()); + LightSource lightSource = attachedLightSource.resolve(MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -653,7 +652,7 @@ public List getDrawableAuras() { Point p = FogUtil.calculateVisionCenter(token, zone); for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -721,7 +720,7 @@ private void findLightSources() { if (token.hasLightSources() && token.isVisible()) { if (!token.isVisibleOnlyToOwner() || AppUtil.playerOwns(token)) { for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -911,7 +910,7 @@ private void onTokensRemoved(TokensRemoved event) { for (Token token : event.tokens()) { if (token.hasAnyTopology()) tokenChangedTopology = true; for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -966,7 +965,7 @@ private boolean processTokenAddChangeEvent(List tokens) { token.hasLightSources() && (token.isVisible() || MapTool.getPlayer().isEffectiveGM()); if (token.hasAnyTopology()) hasTopology = true; for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = c.getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(c); if (lightSource != null) { Set lightSet = lightSourceMap.get(lightSource.getType()); if (hasLightSource) { diff --git a/src/main/java/net/rptools/maptool/model/AttachedLightSource.java b/src/main/java/net/rptools/maptool/model/AttachedLightSource.java index 3502c4f681..aa32853eda 100644 --- a/src/main/java/net/rptools/maptool/model/AttachedLightSource.java +++ b/src/main/java/net/rptools/maptool/model/AttachedLightSource.java @@ -14,26 +14,45 @@ */ package net.rptools.maptool.model; +import java.util.Map; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import net.rptools.maptool.server.proto.AttachedLightSourceDto; -public class AttachedLightSource { +public final class AttachedLightSource { - private GUID lightSourceId; + private final @Nonnull GUID lightSourceId; - public AttachedLightSource() { - // for serialization - } - - private AttachedLightSource(GUID lightSourceId) { + public AttachedLightSource(@Nonnull GUID lightSourceId) { this.lightSourceId = lightSourceId; } - public AttachedLightSource(LightSource source) { - lightSourceId = source.getId(); + /** + * Obtain the attached {@code LightSource} from the campaign. + * + * @param campaign The campaign in which to look up light source IDs. + * @return The {@code LightSource} referenced by this {@code AttachedLightSource}, or {@code null} + * if no such light source exists. + */ + public @Nullable LightSource resolve(Campaign campaign) { + for (Map map : campaign.getLightSourcesMap().values()) { + if (map.containsKey(lightSourceId)) { + return map.get(lightSourceId); + } + } + + return null; } - public GUID getLightSourceId() { - return lightSourceId; + /** + * Check if this {@code AttachedLightSource} references a {@code LightSource} with a matching ID. + * + * @param lightSourceId The ID of the light source to match against. + * @return {@code true} If {@code lightSourceId} is the same as the ID of the attached light + * source. + */ + public boolean matches(@Nonnull GUID lightSourceId) { + return lightSourceId.equals(this.lightSourceId); } public static AttachedLightSource fromDto(AttachedLightSourceDto dto) { @@ -42,7 +61,7 @@ public static AttachedLightSource fromDto(AttachedLightSourceDto dto) { public AttachedLightSourceDto toDto() { var dto = AttachedLightSourceDto.newBuilder(); - dto.setLightSourceId(getLightSourceId().toString()); + dto.setLightSourceId(lightSourceId.toString()); return dto.build(); } } diff --git a/src/main/java/net/rptools/maptool/model/Campaign.java b/src/main/java/net/rptools/maptool/model/Campaign.java index a48ac05501..805f477471 100644 --- a/src/main/java/net/rptools/maptool/model/Campaign.java +++ b/src/main/java/net/rptools/maptool/model/Campaign.java @@ -328,23 +328,6 @@ public List getLookupTables() { return list; } - /** - * Convenience method that iterates through {@link #getLightSourcesMap()} and returns the value - * for the key lightSourceId. - * - * @param lightSourceId the id to look for - * @return the {@link LightSource} or null if not found - */ - public LightSource getLightSource(GUID lightSourceId) { - - for (Map map : getLightSourcesMap().values()) { - if (map.containsKey(lightSourceId)) { - return map.get(lightSourceId); - } - } - return null; - } - /** * Stub that calls campaignProperties.getLightSourcesMap(). * diff --git a/src/main/java/net/rptools/maptool/model/Token.java b/src/main/java/net/rptools/maptool/model/Token.java index 778474a986..ab18224ea3 100644 --- a/src/main/java/net/rptools/maptool/model/Token.java +++ b/src/main/java/net/rptools/maptool/model/Token.java @@ -918,14 +918,14 @@ public String getImageTableName() { return imageTableName; } - public void addLightSource(LightSource source) { - lightSourceList.add(new AttachedLightSource(source)); + public void addLightSource(GUID lightSourceId) { + lightSourceList.add(new AttachedLightSource(lightSourceId)); } public void removeLightSourceType(LightSource.Type lightType) { for (ListIterator i = lightSourceList.listIterator(); i.hasNext(); ) { AttachedLightSource als = i.next(); - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource != null && lightSource.getType() == lightType) { i.remove(); } @@ -935,7 +935,7 @@ public void removeLightSourceType(LightSource.Type lightType) { public void removeGMAuras() { for (ListIterator i = lightSourceList.listIterator(); i.hasNext(); ) { AttachedLightSource als = i.next(); - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -950,7 +950,7 @@ public void removeGMAuras() { public void removeOwnerOnlyAuras() { for (ListIterator i = lightSourceList.listIterator(); i.hasNext(); ) { AttachedLightSource als = i.next(); - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -964,7 +964,7 @@ public void removeOwnerOnlyAuras() { public boolean hasOwnerOnlyAuras() { for (AttachedLightSource als : lightSourceList) { - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -979,7 +979,7 @@ public boolean hasOwnerOnlyAuras() { public boolean hasGMAuras() { for (AttachedLightSource als : lightSourceList) { - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -994,7 +994,7 @@ public boolean hasGMAuras() { public boolean hasLightSourceType(LightSource.Type lightType) { for (AttachedLightSource als : lightSourceList) { - LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId()); + LightSource lightSource = als.resolve(MapTool.getCampaign()); if (lightSource != null && lightSource.getType() == lightType) { return true; } @@ -1002,12 +1002,8 @@ public boolean hasLightSourceType(LightSource.Type lightType) { return false; } - public void removeLightSource(LightSource source) { - lightSourceList.removeIf( - als -> - als != null - && als.getLightSourceId() != null - && als.getLightSourceId().equals(source.getId())); + public void removeLightSource(GUID lightSourceId) { + lightSourceList.removeIf(als -> als.matches(lightSourceId)); } /** Clear the lightSourceList */ @@ -1016,13 +1012,13 @@ public void clearLightSources() { } public boolean hasLightSource(LightSource source) { - if (lightSourceList.size() == 0) { + if (source.getId() == null) { + // Shouldn't happen as this method should only be used with non-personal lights. return false; } + for (AttachedLightSource als : lightSourceList) { - if (als != null - && als.getLightSourceId() != null - && als.getLightSourceId().equals(source.getId())) { + if (als.matches(source.getId())) { return true; } } @@ -2507,6 +2503,10 @@ protected Object readResolve() { if (lightSourceList == null) { lightSourceList = new ArrayList<>(); } + // There used to be checks elsewhere that elements were not null. In case those were legitimate, + // let's filter them out here instead. + lightSourceList.removeIf(Objects::isNull); + if (macroPropertiesMap == null) { macroPropertiesMap = new HashMap<>(); } @@ -2823,11 +2823,11 @@ public void updateProperty(Zone zone, Update update, List if (hasLightSources()) { lightChanged = true; } - removeLightSource(LightSource.fromDto(parameters.get(0).getLightSource())); + removeLightSource(GUID.valueOf(parameters.get(0).getLightSourceId())); break; case addLightSource: lightChanged = true; - addLightSource(LightSource.fromDto(parameters.get(0).getLightSource())); + addLightSource(GUID.valueOf(parameters.get(0).getLightSourceId())); break; case setHasSight: if (hasLightSources()) { diff --git a/src/main/java/net/rptools/maptool/server/ServerCommand.java b/src/main/java/net/rptools/maptool/server/ServerCommand.java index ae411df169..7edec64833 100644 --- a/src/main/java/net/rptools/maptool/server/ServerCommand.java +++ b/src/main/java/net/rptools/maptool/server/ServerCommand.java @@ -204,8 +204,6 @@ void updateTokenProperty( void updateTokenProperty(Token token, Token.Update update, LightSource value); - void updateTokenProperty(Token token, Token.Update update, LightSource value1, String value2); - void updateTokenProperty(Token token, Token.Update update, int value1, int value2); void updateTokenProperty(Token token, Token.Update update, boolean value); diff --git a/src/main/proto/data_transfer_objects.proto b/src/main/proto/data_transfer_objects.proto index ab95750696..705b761df6 100644 --- a/src/main/proto/data_transfer_objects.proto +++ b/src/main/proto/data_transfer_objects.proto @@ -674,7 +674,7 @@ message TokenPropertyValueDto { string string_value = 3; double double_value = 4; MacroButtonPropertiesListDto macros = 5; - LightSourceDto light_source = 6; + string light_source_id = 6; AreaDto area = 7; StringListDto string_values = 8; GridDto grid = 9; From 5a130bf3dbee07d86fd309bf30e34e09e268c28d Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Tue, 24 Oct 2023 12:14:39 -0700 Subject: [PATCH 3/8] Implement light sources on tokens It is now possible for tokens to carry light source definitions, much like campaigns can. These new light sources are called "unique lights" as they are only available to the token that defines them. If any unique light source are defined on a token, they can be accessed like any other lights via the right-click menu, under the special category "Unique". In order to support lookup of such light sources, `AttachedLightSource#resolve()` now also accepts a `Token` for context, in addition to the `Campaign`. The token is checked first for a matching light source, then the campaign is checked as a fallback. For networking, the `TokenPropertyValueDto.lightSource` field was added back in because creating a unique light source now requires the complete definition to be passed. This commit does not add any way for a user to add any light sources to token. That will come later. --- .../client/ServerCommandClientImpl.java | 14 +++-- .../client/ui/AbstractTokenPopupMenu.java | 41 ++++++++++++--- .../ui/zone/LightSourceIconOverlay.java | 2 +- .../maptool/client/ui/zone/ZoneView.java | 11 ++-- .../maptool/model/AttachedLightSource.java | 10 +++- .../java/net/rptools/maptool/model/Token.java | 51 ++++++++++++++++--- src/main/proto/data_transfer_objects.proto | 34 +++++++------ 7 files changed, 124 insertions(+), 39 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java b/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java index 33722cd2e4..282a553ef4 100644 --- a/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java +++ b/src/main/java/net/rptools/maptool/client/ServerCommandClientImpl.java @@ -674,10 +674,16 @@ public void updateTokenProperty(Token token, Token.Update update, String value) @Override public void updateTokenProperty(Token token, Token.Update update, LightSource value) { - updateTokenProperty( - token, - update, - TokenPropertyValueDto.newBuilder().setLightSourceId(value.getId().toString()).build()); + if (update == Token.Update.createUniqueLightSource) { + // This case requires sending the full light source definition. + updateTokenProperty( + token, update, TokenPropertyValueDto.newBuilder().setLightSource(value.toDto()).build()); + } else { + updateTokenProperty( + token, + update, + TokenPropertyValueDto.newBuilder().setLightSourceId(value.getId().toString()).build()); + } } @Override diff --git a/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java b/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java index 03ed55c127..818258701d 100644 --- a/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java +++ b/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java @@ -22,7 +22,7 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -143,17 +143,46 @@ protected JMenu createLightSourceMenu() { } menu.addSeparator(); } + + // Add unique light sources for the token. + { + JMenu subMenu = new JMenu("Unique"); + + List lightSources = new ArrayList<>(tokenUnderMouse.getUniqueLightSources()); + Collections.sort(lightSources); + + LIGHTSOURCES: + for (LightSource lightSource : lightSources) { + for (Light light : lightSource.getLightList()) { + // TODO Shouldn't we only skip if *all* child lights are GM only. And what about owner + // only? + if (light.isGM() && !MapTool.getPlayer().isGM()) { + continue LIGHTSOURCES; + } + } + JCheckBoxMenuItem menuItem = + new JCheckBoxMenuItem(new ToggleLightSourceAction(lightSource)); + menuItem.setSelected(tokenUnderMouse.hasLightSource(lightSource)); + subMenu.add(menuItem); + } + if (subMenu.getItemCount() != 0) { + menu.add(subMenu); + menu.addSeparator(); + } + } + for (Entry> entry : MapTool.getCampaign().getLightSourcesMap().entrySet()) { JMenu subMenu = new JMenu(entry.getKey()); - List lightSources = new ArrayList(entry.getValue().values()); - LightSource[] lightSourceList = new LightSource[entry.getValue().size()]; - lightSources.toArray(lightSourceList); - Arrays.sort(lightSourceList); + List lightSources = new ArrayList<>(entry.getValue().values()); + Collections.sort(lightSources); + LIGHTSOURCES: - for (LightSource lightSource : lightSourceList) { + for (LightSource lightSource : lightSources) { for (Light light : lightSource.getLightList()) { + // TODO Shouldn't we only skip if *all* child lights are GM only. And what about owner + // only? if (light.isGM() && !MapTool.getPlayer().isGM()) { continue LIGHTSOURCES; } diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java b/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java index d81a05f3f0..c1aed770f3 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/LightSourceIconOverlay.java @@ -34,7 +34,7 @@ public void paintOverlay(ZoneRenderer renderer, Graphics2D g) { if (token.hasLightSources()) { boolean foundNormalLight = false; for (AttachedLightSource attachedLightSource : token.getLightSources()) { - LightSource lightSource = attachedLightSource.resolve(MapTool.getCampaign()); + LightSource lightSource = attachedLightSource.resolve(token, MapTool.getCampaign()); if (lightSource != null && lightSource.getType() == LightSource.Type.NORMAL) { foundNormalLight = true; break; diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java b/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java index 2551aba084..e1ebef3454 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java @@ -294,7 +294,8 @@ private List calculateLitAreas(Token lightSourceToken, double final var result = new ArrayList(); for (final var attachedLightSource : lightSourceToken.getLightSources()) { - LightSource lightSource = attachedLightSource.resolve(MapTool.getCampaign()); + LightSource lightSource = + attachedLightSource.resolve(lightSourceToken, MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -652,7 +653,7 @@ public List getDrawableAuras() { Point p = FogUtil.calculateVisionCenter(token, zone); for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(token, MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -720,7 +721,7 @@ private void findLightSources() { if (token.hasLightSources() && token.isVisible()) { if (!token.isVisibleOnlyToOwner() || AppUtil.playerOwns(token)) { for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(token, MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -910,7 +911,7 @@ private void onTokensRemoved(TokensRemoved event) { for (Token token : event.tokens()) { if (token.hasAnyTopology()) tokenChangedTopology = true; for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(token, MapTool.getCampaign()); if (lightSource == null) { continue; } @@ -965,7 +966,7 @@ private boolean processTokenAddChangeEvent(List tokens) { token.hasLightSources() && (token.isVisible() || MapTool.getPlayer().isEffectiveGM()); if (token.hasAnyTopology()) hasTopology = true; for (AttachedLightSource als : token.getLightSources()) { - LightSource lightSource = als.resolve(c); + LightSource lightSource = als.resolve(token, c); if (lightSource != null) { Set lightSet = lightSourceMap.get(lightSource.getType()); if (hasLightSource) { diff --git a/src/main/java/net/rptools/maptool/model/AttachedLightSource.java b/src/main/java/net/rptools/maptool/model/AttachedLightSource.java index aa32853eda..03568e342b 100644 --- a/src/main/java/net/rptools/maptool/model/AttachedLightSource.java +++ b/src/main/java/net/rptools/maptool/model/AttachedLightSource.java @@ -28,13 +28,19 @@ public AttachedLightSource(@Nonnull GUID lightSourceId) { } /** - * Obtain the attached {@code LightSource} from the campaign. + * Obtain the attached {@code LightSource} from the token or campaign. * + * @param token The token in which to look up light source IDs. * @param campaign The campaign in which to look up light source IDs. * @return The {@code LightSource} referenced by this {@code AttachedLightSource}, or {@code null} * if no such light source exists. */ - public @Nullable LightSource resolve(Campaign campaign) { + public @Nullable LightSource resolve(Token token, Campaign campaign) { + final var uniqueLightSource = token.getUniqueLightSource(lightSourceId); + if (uniqueLightSource != null) { + return uniqueLightSource; + } + for (Map map : campaign.getLightSourcesMap().values()) { if (map.containsKey(lightSourceId)) { return map.get(lightSourceId); diff --git a/src/main/java/net/rptools/maptool/model/Token.java b/src/main/java/net/rptools/maptool/model/Token.java index ab18224ea3..d0ba4bd371 100644 --- a/src/main/java/net/rptools/maptool/model/Token.java +++ b/src/main/java/net/rptools/maptool/model/Token.java @@ -211,6 +211,8 @@ public enum Update { setPortraitImage, setCharsheetImage, setLayout, + createUniqueLightSource, + deleteUniqueLightSource, clearLightSources, removeLightSource, addLightSource, @@ -335,6 +337,7 @@ public String toString() { private MD5Key charsheetImage; private MD5Key portraitImage; + private Map uniqueLightSources = new LinkedHashMap<>(); private List lightSourceList = new ArrayList<>(); private String sightType; private boolean hasSight; @@ -473,7 +476,10 @@ public Token(Token token) { ownerType = token.ownerType; ownerList.addAll(token.ownerList); + + uniqueLightSources.putAll(token.uniqueLightSources); lightSourceList.addAll(token.lightSourceList); + state.putAll(token.state); getPropertyMap().clear(); getPropertyMap().putAll(token.propertyMapCI); @@ -918,6 +924,22 @@ public String getImageTableName() { return imageTableName; } + public @Nonnull Collection getUniqueLightSources() { + return uniqueLightSources.values(); + } + + public @Nullable LightSource getUniqueLightSource(GUID lightSourceId) { + return uniqueLightSources.getOrDefault(lightSourceId, null); + } + + public void addUniqueLightSource(LightSource source) { + uniqueLightSources.put(source.getId(), source); + } + + public void removeUniqueLightSource(GUID lightSourceId) { + uniqueLightSources.remove(lightSourceId); + } + public void addLightSource(GUID lightSourceId) { lightSourceList.add(new AttachedLightSource(lightSourceId)); } @@ -925,7 +947,7 @@ public void addLightSource(GUID lightSourceId) { public void removeLightSourceType(LightSource.Type lightType) { for (ListIterator i = lightSourceList.listIterator(); i.hasNext(); ) { AttachedLightSource als = i.next(); - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(this, MapTool.getCampaign()); if (lightSource != null && lightSource.getType() == lightType) { i.remove(); } @@ -935,7 +957,7 @@ public void removeLightSourceType(LightSource.Type lightType) { public void removeGMAuras() { for (ListIterator i = lightSourceList.listIterator(); i.hasNext(); ) { AttachedLightSource als = i.next(); - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(this, MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -950,7 +972,7 @@ public void removeGMAuras() { public void removeOwnerOnlyAuras() { for (ListIterator i = lightSourceList.listIterator(); i.hasNext(); ) { AttachedLightSource als = i.next(); - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(this, MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -964,7 +986,7 @@ public void removeOwnerOnlyAuras() { public boolean hasOwnerOnlyAuras() { for (AttachedLightSource als : lightSourceList) { - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(this, MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -979,7 +1001,7 @@ public boolean hasOwnerOnlyAuras() { public boolean hasGMAuras() { for (AttachedLightSource als : lightSourceList) { - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(this, MapTool.getCampaign()); if (lightSource != null) { List lights = lightSource.getLightList(); for (Light light : lights) { @@ -994,7 +1016,7 @@ public boolean hasGMAuras() { public boolean hasLightSourceType(LightSource.Type lightType) { for (AttachedLightSource als : lightSourceList) { - LightSource lightSource = als.resolve(MapTool.getCampaign()); + LightSource lightSource = als.resolve(this, MapTool.getCampaign()); if (lightSource != null && lightSource.getType() == lightType) { return true; } @@ -2500,6 +2522,9 @@ protected Object readResolve() { if (ownerList == null) { ownerList = new HashSet<>(); } + if (uniqueLightSources == null) { + uniqueLightSources = new LinkedHashMap<>(); + } if (lightSourceList == null) { lightSourceList = new ArrayList<>(); } @@ -2813,6 +2838,14 @@ public void updateProperty(Zone zone, Update update, List setSizeScale(parameters.get(0).getDoubleValue()); setAnchor(parameters.get(1).getIntValue(), parameters.get(2).getIntValue()); break; + case createUniqueLightSource: + lightChanged = true; + addUniqueLightSource(LightSource.fromDto(parameters.get(0).getLightSource())); + break; + case deleteUniqueLightSource: + lightChanged = true; + removeUniqueLightSource(GUID.valueOf(parameters.get(0).getLightSourceId())); + break; case clearLightSources: if (hasLightSources()) { lightChanged = true; @@ -2946,6 +2979,10 @@ public static Token fromDto(TokenDto dto) { dto.hasCharsheetImage() ? new MD5Key(dto.getCharsheetImage().getValue()) : null; token.portraitImage = dto.hasPortraitImage() ? new MD5Key(dto.getPortraitImage().getValue()) : null; + + dto.getUniqueLightSourcesList().stream() + .map(LightSource::fromDto) + .forEach(source -> token.uniqueLightSources.put(source.getId(), source)); token.lightSourceList.addAll( dto.getLightSourcesList().stream() .map(AttachedLightSource::fromDto) @@ -3073,6 +3110,8 @@ public TokenDto toDto() { if (portraitImage != null) { dto.setPortraitImage(StringValue.of(portraitImage.toString())); } + dto.addAllUniqueLightSources( + uniqueLightSources.values().stream().map(LightSource::toDto).collect(Collectors.toList())); dto.addAllLightSources( lightSourceList.stream().map(AttachedLightSource::toDto).collect(Collectors.toList())); if (sightType != null) { diff --git a/src/main/proto/data_transfer_objects.proto b/src/main/proto/data_transfer_objects.proto index 705b761df6..bf17d44571 100644 --- a/src/main/proto/data_transfer_objects.proto +++ b/src/main/proto/data_transfer_objects.proto @@ -342,6 +342,7 @@ message TokenDto { bool is_flipped_iso = 47; google.protobuf.StringValue charsheet_image = 48; google.protobuf.StringValue portrait_image = 49; + repeated LightSourceDto unique_light_sources = 72; repeated AttachedLightSourceDto light_sources = 50; google.protobuf.StringValue sight_type = 51; bool has_sight = 52; @@ -644,16 +645,18 @@ enum TokenUpdateDto { setPortraitImage = 43; setCharsheetImage = 44; setLayout = 45; - clearLightSources = 46; - removeLightSource = 47; - addLightSource = 48; - setHasSight = 49; - setSightType = 50; - flipX = 51; - flipY = 52; - flipIso = 53; - setSpeechName = 54; - removeFacing = 55; + createUniqueLightSource = 46; + deleteUniqueLightSource = 47; + clearLightSources = 48; + removeLightSource = 49; + addLightSource = 50; + setHasSight = 51; + setSightType = 52; + flipX = 53; + flipY = 54; + flipIso = 55; + setSpeechName = 56; + removeFacing = 57; } message AssetTransferHeaderDto { @@ -675,11 +678,12 @@ message TokenPropertyValueDto { double double_value = 4; MacroButtonPropertiesListDto macros = 5; string light_source_id = 6; - AreaDto area = 7; - StringListDto string_values = 8; - GridDto grid = 9; - TokenFootPrintDto token_foot_print = 10; - string topology_type = 11; + LightSourceDto light_source = 7; + AreaDto area = 8; + StringListDto string_values = 9; + GridDto grid = 10; + TokenFootPrintDto token_foot_print = 11; + string topology_type = 12; } } From b056c5b2c69b8a696d75d8e6221a69945ad10a42 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Tue, 24 Oct 2023 16:22:27 -0700 Subject: [PATCH 4/8] Integrate unique light sources with existing light functions These functions have been updated: - `getLights([category, [delim, [token, [map]]]])` - `setLight(category, name, state, [token, [map]])` - `hasLightSource(category, name, [token, [map]])` They now support a special category `"$unique"` to identify unique light sources. Meanwhile the `"*"` category wildcard will also find unique light sources in addition to campaign light sources. --- .../client/functions/TokenLightFunctions.java | 98 ++++++++++++------- 1 file changed, 63 insertions(+), 35 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java b/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java index 5a1f8dda96..82a99dc299 100644 --- a/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java +++ b/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java @@ -32,6 +32,8 @@ public class TokenLightFunctions extends AbstractFunction { private static final TokenLightFunctions instance = new TokenLightFunctions(); + private static final String TOKEN_CATEGORY = "$unique"; + private TokenLightFunctions() { super(0, 5, "hasLightSource", "clearLights", "setLight", "getLights"); } @@ -83,8 +85,9 @@ public Object childEvaluate( * Gets the names of the light sources that are on. * * @param token The token to get the light sources for. - * @param category The category to get the light sources for, if null then the light sources for - * all categories will be returned. + * @param category The category to get the light sources for. If "*" then the light sources for + * all categories will be returned. If "$unique" then the light sources defined on the token + * will be returned. * @param delim the delimiter for the list. * @return a string list containing the lights that are on. * @throws ParserException if the light type can't be found. @@ -95,7 +98,13 @@ private static String getLights(Token token, String category, String delim) Map> lightSourcesMap = MapTool.getCampaign().getLightSourcesMap(); - if (category == null || category.equals("*")) { + if (category.equals("*")) { + // Look up on both token and campaign. + for (LightSource ls : token.getUniqueLightSources()) { + if (token.hasLightSource(ls)) { + lightList.add(ls.getName()); + } + } for (Map lsMap : lightSourcesMap.values()) { for (LightSource ls : lsMap.values()) { if (token.hasLightSource(ls)) { @@ -103,18 +112,23 @@ private static String getLights(Token token, String category, String delim) } } } - } else { - if (lightSourcesMap.containsKey(category)) { - for (LightSource ls : lightSourcesMap.get(category).values()) { - if (token.hasLightSource(ls)) { - lightList.add(ls.getName()); - } + } else if (TOKEN_CATEGORY.equals(category)) { + for (LightSource ls : token.getUniqueLightSources()) { + if (token.hasLightSource(ls)) { + lightList.add(ls.getName()); } - } else { - throw new ParserException( - I18N.getText("macro.function.tokenLight.unknownLightType", "getLights", category)); } + } else if (lightSourcesMap.containsKey(category)) { + for (LightSource ls : lightSourcesMap.get(category).values()) { + if (token.hasLightSource(ls)) { + lightList.add(ls.getName()); + } + } + } else { + throw new ParserException( + I18N.getText("macro.function.tokenLight.unknownLightType", "getLights", category)); } + if ("json".equals(delim)) { JsonArray jarr = new JsonArray(); lightList.forEach(l -> jarr.add(new JsonPrimitive(l))); @@ -128,7 +142,8 @@ private static String getLights(Token token, String category, String delim) * Sets the light value for a token. * * @param token the token to set the light for. - * @param category the category of the light source. + * @param category the category of the light source. Use "$unique" for light sources defined on + * the token. * @param name the name of the light source. * @param val the value to set for the light source, 0 for off non 0 for on. * @return 0 if the light was not found, otherwise 1; @@ -140,21 +155,25 @@ private static BigDecimal setLight(Token token, String category, String name, Bi Map> lightSourcesMap = MapTool.getCampaign().getLightSourcesMap(); - if (lightSourcesMap.containsKey(category)) { - for (LightSource ls : lightSourcesMap.get(category).values()) { - if (ls.getName().equals(name)) { - found = true; - if (val.equals(BigDecimal.ZERO)) { - MapTool.serverCommand().updateTokenProperty(token, Token.Update.removeLightSource, ls); - } else { - MapTool.serverCommand().updateTokenProperty(token, Token.Update.addLightSource, ls); - } - } - } + Iterable sources; + if (TOKEN_CATEGORY.equals(category)) { + sources = token.getUniqueLightSources(); + } else if (lightSourcesMap.containsKey(category)) { + sources = lightSourcesMap.get(category).values(); } else { throw new ParserException( I18N.getText("macro.function.tokenLight.unknownLightType", "setLights", category)); } + + final var updateAction = + BigDecimal.ZERO.equals(val) ? Token.Update.removeLightSource : Token.Update.addLightSource; + for (LightSource ls : sources) { + if (name.equals(ls.getName())) { + found = true; + MapTool.serverCommand().updateTokenProperty(token, updateAction, ls); + } + } + return found ? BigDecimal.ONE : BigDecimal.ZERO; } @@ -162,6 +181,7 @@ private static BigDecimal setLight(Token token, String category, String name, Bi * Checks to see if the token has a light source. The token is checked to see if it has a light * source with the name in the second parameter from the category in the first parameter. A "*" * for category indicates all categories are checked; a "*" for name indicates all names are + * checked. The "$unique" category indicates that only light sources defined on the token are * checked. * * @param token the token to check. @@ -180,6 +200,12 @@ public static boolean hasLightSource(Token token, String category, String name) MapTool.getCampaign().getLightSourcesMap(); if ("*".equals(category)) { + // Look up on both token and campaign. + for (LightSource ls : token.getUniqueLightSources()) { + if (ls.getName().equals(name) && token.hasLightSource(ls)) { + return true; + } + } for (Map lsMap : lightSourcesMap.values()) { for (LightSource ls : lsMap.values()) { if (ls.getName().equals(name) && token.hasLightSource(ls)) { @@ -187,19 +213,21 @@ public static boolean hasLightSource(Token token, String category, String name) } } } - } else { - if (lightSourcesMap.containsKey(category)) { - for (LightSource ls : lightSourcesMap.get(category).values()) { - if (ls.getName().equals(name) || "*".equals(name)) { - if (token.hasLightSource(ls)) { - return true; - } - } + } else if (TOKEN_CATEGORY.equals(category)) { + for (LightSource ls : token.getUniqueLightSources()) { + if ((ls.getName().equals(name) || "*".equals(name)) && token.hasLightSource(ls)) { + return true; } - } else { - throw new ParserException( - I18N.getText("macro.function.tokenLight.unknownLightType", "hasLightSource", category)); } + } else if (lightSourcesMap.containsKey(category)) { + for (LightSource ls : lightSourcesMap.get(category).values()) { + if ((ls.getName().equals(name) || "*".equals(name)) && token.hasLightSource(ls)) { + return true; + } + } + } else { + throw new ParserException( + I18N.getText("macro.function.tokenLight.unknownLightType", "hasLightSource", category)); } return false; From 85b5527e4519be2276083984ba80a3276e462ca1 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 27 Oct 2023 13:36:37 -0700 Subject: [PATCH 5/8] Add macro functions for manipulating unique light sources The following new functions have been added: - `createUniqueLightSource(lightSourceJson, [token, [map]])` to create a new unique light source. - `updateUniqueLightSource(lightSourceJson, [token, [map]])` to modify the unique light source with the matching name. - `deleteUniqueLightSource(name, [token, [map]])` to delete a unique light source by name. - `getUniqueLightSource(name, [token, [map]])` to get a unique light source by name, as a JSON object. - `getUniqueLightSources([token, [map]])` to get all of a token's light sources as a JSON array of JSON objects. - `getUniqueLightSourceNames([token, [map]])` to get the names of all of a token's light sources, as a JSON array. The JSON format for a unique light source look like: ```json { "name": "Torch - 20", "type": "NORMAL", "scale": false, "lights": [ {"shape": "CIRCLE", "range": 20, "lumens": 100}, {"shape": "CIRCLE", "range": 40, "color": "#000000", "lumens": 50} ] } ``` Note that any boolean values are JSON booleans, not 0/1. Light sources have the following fields: - `"name"`: `string`; required. - `"type"`: (`"NORMAL"`|`"AURA"`); defaults to `"NORMAL"`. - `"scale"`: `boolean`; defaults to `false`. - `"lights"` `array`; defauls to `[]`. Each light has these fields: - `"range"`: `number`, required. - `"shape"`: (`"SQUARE"`|`"CIRCLE"`|`"CONE"`|`"HEX"`|`"GRID"`), defaults to `"CIRCLE"`. - `"offset"`: `number`, defaults to `0`. Only permitted if the shape is `"CONE"`. - `"arc"`: `number`, defaults to `0`. Only permitted if the shape is `"CONE"`. - `"gmOnly"`: `boolean`, defaults to `false`. Only permitted if the light source type is `"AURA"`. - `"ownerOnly"`: `boolean`, defaults to `false`. Only permitted if the light source is `"AURA"`. - `"color"`: `string` of the form `"#rrggbb`, defaults to `null`. - `"lumens"`: `number` (integer), defaults to 100. --- .../client/functions/TokenLightFunctions.java | 257 +++++++++++++++++- 1 file changed, 256 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java b/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java index 82a99dc299..fcf295768d 100644 --- a/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java +++ b/src/main/java/net/rptools/maptool/client/functions/TokenLightFunctions.java @@ -15,14 +15,19 @@ package net.rptools.maptool.client.functions; import com.google.gson.JsonArray; +import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; +import java.awt.Color; import java.math.BigDecimal; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.language.I18N; import net.rptools.maptool.model.*; +import net.rptools.maptool.model.drawing.DrawableColorPaint; import net.rptools.maptool.util.FunctionUtil; import net.rptools.parser.Parser; import net.rptools.parser.ParserException; @@ -35,7 +40,19 @@ public class TokenLightFunctions extends AbstractFunction { private static final String TOKEN_CATEGORY = "$unique"; private TokenLightFunctions() { - super(0, 5, "hasLightSource", "clearLights", "setLight", "getLights"); + super( + 0, + 5, + "hasLightSource", + "clearLights", + "setLight", + "getLights", + "createUniqueLightSource", + "updateUniqueLightSource", + "deleteUniqueLightSource", + "getUniqueLightSource", + "getUniqueLightSources", + "getUniqueLightSourceNames"); } public static TokenLightFunctions getInstance() { @@ -78,6 +95,49 @@ public Object childEvaluate( Token token = FunctionUtil.getTokenFromParam(resolver, functionName, parameters, 2, 3); return getLights(token, type, delim); } + if (functionName.equalsIgnoreCase("createUniqueLightSource")) { + FunctionUtil.blockUntrustedMacro(functionName); + FunctionUtil.checkNumberParam(functionName, parameters, 1, 3); + + JsonObject lightSource = FunctionUtil.paramAsJsonObject(functionName, parameters, 0); + Token token = FunctionUtil.getTokenFromParam(resolver, functionName, parameters, 1, 2); + return createUniqueLightSource(lightSource, token, false).getName(); + } + if (functionName.equalsIgnoreCase("updateUniqueLightSource")) { + FunctionUtil.blockUntrustedMacro(functionName); + FunctionUtil.checkNumberParam(functionName, parameters, 1, 3); + + JsonObject lightSource = FunctionUtil.paramAsJsonObject(functionName, parameters, 0); + Token token = FunctionUtil.getTokenFromParam(resolver, functionName, parameters, 1, 2); + return createUniqueLightSource(lightSource, token, true).getName(); + } + if (functionName.equalsIgnoreCase("deleteUniqueLightSource")) { + FunctionUtil.checkNumberParam(functionName, parameters, 1, 3); + + String name = parameters.get(0).toString(); + Token token = FunctionUtil.getTokenFromParam(resolver, functionName, parameters, 1, 2); + deleteUniqueLightSource(name, token); + return ""; + } + if (functionName.equalsIgnoreCase("getUniqueLightSource")) { + FunctionUtil.checkNumberParam(functionName, parameters, 1, 3); + + String name = parameters.get(0).toString(); + Token token = FunctionUtil.getTokenFromParam(resolver, functionName, parameters, 1, 2); + return Objects.requireNonNullElse(getUniqueLightSource(name, token), ""); + } + if (functionName.equalsIgnoreCase("getUniqueLightSources")) { + FunctionUtil.checkNumberParam(functionName, parameters, 0, 2); + + Token token = FunctionUtil.getTokenFromParam(resolver, functionName, parameters, 0, 1); + return getUniqueLightSources(token); + } + if (functionName.equalsIgnoreCase("getUniqueLightSourceNames")) { + FunctionUtil.checkNumberParam(functionName, parameters, 0, 2); + + Token token = FunctionUtil.getTokenFromParam(resolver, functionName, parameters, 0, 1); + return getUniqueLightSourceNames(token); + } return null; } @@ -232,4 +292,199 @@ public static boolean hasLightSource(Token token, String category, String name) return false; } + + private static LightSource createUniqueLightSource( + JsonObject lightSourceDef, Token token, boolean isUpdate) throws ParserException { + if (!lightSourceDef.has("name")) { + throw new ParserException(I18N.getText("The light source must have a name.")); + } + final String name = lightSourceDef.get("name").getAsString(); + + // Modifications require the light source to exist. Creation requires it to not exists. + final Optional existingSource = + token.getUniqueLightSources().stream() + .filter(source -> name.equals(source.getName())) + .findFirst(); + if (isUpdate && existingSource.isEmpty()) { + throw new ParserException( + I18N.getText( + "Light source %s is not defined for token %s", name, token.getId().toString())); + } + if (!isUpdate && existingSource.isPresent()) { + throw new ParserException( + I18N.getText( + "Light source %s is already defined for token %s", name, token.getId().toString())); + } + + final LightSource.Type type = + lightSourceDef.has("type") + ? LightSource.Type.valueOf(lightSourceDef.get("type").getAsString().toUpperCase()) + : LightSource.Type.NORMAL; + final boolean scaleWithToken = + lightSourceDef.has("scale") ? lightSourceDef.get("scale").getAsBoolean() : false; + final JsonArray lightDefs = + lightSourceDef.has("lights") ? lightSourceDef.getAsJsonArray("lights") : new JsonArray(); + + final var lights = new ArrayList(); + for (final var light : lightDefs) { + lights.add(parseLightJson(light.getAsJsonObject(), type)); + } + + final var lightSource = + LightSource.createRegular( + name, + existingSource.isPresent() ? existingSource.get().getId() : new GUID(), + type, + scaleWithToken, + lights); + token.addUniqueLightSource(lightSource); + MapTool.serverCommand() + .updateTokenProperty(token, Token.Update.createUniqueLightSource, lightSource); + return lightSource; + } + + private static void deleteUniqueLightSource(String name, Token token) { + final var sourcesToRemove = new ArrayList(); + for (final LightSource source : token.getUniqueLightSources()) { + if (name.equals(source.getName())) { + sourcesToRemove.add(source); + } + } + + for (final LightSource source : sourcesToRemove) { + token.removeUniqueLightSource(source.getId()); + MapTool.serverCommand() + .updateTokenProperty(token, Token.Update.deleteUniqueLightSource, source); + } + } + + private static JsonObject getUniqueLightSource(String name, Token token) { + for (final LightSource source : token.getUniqueLightSources()) { + if (name.equals(source.getName())) { + return lightSourceToJson(source); + } + } + + return null; + } + + private static JsonArray getUniqueLightSources(Token token) { + final var result = new JsonArray(); + + for (final LightSource source : token.getUniqueLightSources()) { + result.add(lightSourceToJson(source)); + } + + return result; + } + + private static JsonArray getUniqueLightSourceNames(Token token) { + final var result = new JsonArray(); + + for (final LightSource source : token.getUniqueLightSources()) { + result.add(source.getName()); + } + + return result; + } + + private static Light parseLightJson(JsonObject lightDef, LightSource.Type lightSourceType) + throws ParserException { + if (!lightDef.has("range")) { + throw new ParserException(I18N.getText("A range must be provided for each light")); + } + final var range = lightDef.get("range").getAsDouble(); + + final var shape = + lightDef.has("shape") + ? ShapeType.valueOf(lightDef.get("shape").getAsString()) + : ShapeType.CIRCLE; + // Cones permit the fields arc and offset, but no other shape accepts them. + if (shape != ShapeType.CONE) { + if (lightDef.has("offset")) { + throw new ParserException( + I18N.getText("Facing offset provided but the shape is not a cone")); + } + if (lightDef.has("arc")) { + throw new ParserException(I18N.getText("Arc provided but the shape is not a cone")); + } + } + final var offset = lightDef.has("offset") ? lightDef.get("offset").getAsDouble() : 0; + final var arc = lightDef.has("arc") ? lightDef.get("arc").getAsDouble() : 0; + + // Auras permit the gmOnly and ownerOnly flags, but no other type accepts them. + if (lightSourceType != LightSource.Type.AURA) { + if (lightDef.has("gmOnly")) { + throw new ParserException(I18N.getText("gmOnly flag provided but the type is not an aura")); + } + if (lightDef.has("ownerOnly")) { + throw new ParserException( + I18N.getText("ownerOnly flag provided but the type is not an aura")); + } + } + final var gmOnly = lightDef.has("gmOnly") ? !lightDef.get("gmOnly").getAsBoolean() : false; + final var ownerOnly = + lightDef.has("ownerOnly") ? !lightDef.get("ownerOnly").getAsBoolean() : false; + + final DrawableColorPaint colorPaint; + if (lightDef.has("color")) { + var colorString = lightDef.get("color").getAsString(); + if (!colorString.startsWith("#")) { + // Make sure it is parsed as a hex color string, not something else. + colorString = "#" + colorString; + } + + colorPaint = new DrawableColorPaint(Color.decode(colorString)); + } else { + colorPaint = null; + } + + final var lumens = lightDef.has("lumens") ? lightDef.get("lumens").getAsInt() : 100; + if (lumens == 0) { + throw new ParserException(I18N.getText("Lumens must be non-zero.")); + } + + return new Light(shape, offset, range, arc, colorPaint, lumens, gmOnly, ownerOnly); + } + + private static JsonObject lightSourceToJson(LightSource source) { + final var lightSourceDef = new JsonObject(); + lightSourceDef.addProperty("name", source.getName()); + lightSourceDef.addProperty("type", source.getType().toString()); + lightSourceDef.addProperty("scale", source.isScaleWithToken()); + + final var lightDefs = new JsonArray(); + for (final Light light : source.getLightList()) { + lightDefs.add(lightToJson(source, light)); + } + lightSourceDef.add("lights", lightDefs); + return lightSourceDef; + } + + private static JsonObject lightToJson(LightSource source, Light light) { + final var lightDef = new JsonObject(); + lightDef.addProperty("shape", light.getShape().toString()); + + if (light.getShape() == ShapeType.CONE) { + lightDef.addProperty("offset", light.getFacingOffset()); + lightDef.addProperty("arc", light.getArcAngle()); + } + + if (source.getType() == LightSource.Type.AURA) { + lightDef.addProperty("gmOnly", light.isGM()); + lightDef.addProperty("ownerOnly", light.isOwnerOnly()); + } + + lightDef.addProperty("range", light.getRadius()); + if (light.getPaint() instanceof DrawableColorPaint paint) { + lightDef.addProperty("color", toHex(paint.getColor())); + } + lightDef.addProperty("lumens", light.getLumens()); + + return lightDef; + } + + private static String toHex(int rgb) { + return String.format("#%06X", rgb & 0x00FFFFFF); + } } From b827261be6b2c0b8105ce12e32d4a7b41d378c43 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Thu, 2 Nov 2023 14:36:43 -0700 Subject: [PATCH 6/8] Fix to not hide partially GM-only light sources in the menu It's possible for only some lights in an aura light source to be marked as GM-only (or OWNER-only). These were not being shown in the right-click menu despite players being able to see them once equipped. Now these auras will show in the menu, and will only be hidden if the entire auras is GM-only. This also include a refactor to avoid duplicated logic between campaign light sources and unique light sources, while also avoiding some of the complicated looping. --- .../client/ui/AbstractTokenPopupMenu.java | 61 ++++++++----------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java b/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java index 818258701d..05376b49da 100644 --- a/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java +++ b/src/main/java/net/rptools/maptool/client/ui/AbstractTokenPopupMenu.java @@ -22,6 +22,7 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -146,25 +147,7 @@ protected JMenu createLightSourceMenu() { // Add unique light sources for the token. { - JMenu subMenu = new JMenu("Unique"); - - List lightSources = new ArrayList<>(tokenUnderMouse.getUniqueLightSources()); - Collections.sort(lightSources); - - LIGHTSOURCES: - for (LightSource lightSource : lightSources) { - for (Light light : lightSource.getLightList()) { - // TODO Shouldn't we only skip if *all* child lights are GM only. And what about owner - // only? - if (light.isGM() && !MapTool.getPlayer().isGM()) { - continue LIGHTSOURCES; - } - } - JCheckBoxMenuItem menuItem = - new JCheckBoxMenuItem(new ToggleLightSourceAction(lightSource)); - menuItem.setSelected(tokenUnderMouse.hasLightSource(lightSource)); - subMenu.add(menuItem); - } + JMenu subMenu = createLightCategoryMenu("Unique", tokenUnderMouse.getUniqueLightSources()); if (subMenu.getItemCount() != 0) { menu.add(subMenu); menu.addSeparator(); @@ -173,30 +156,34 @@ protected JMenu createLightSourceMenu() { for (Entry> entry : MapTool.getCampaign().getLightSourcesMap().entrySet()) { - JMenu subMenu = new JMenu(entry.getKey()); - - List lightSources = new ArrayList<>(entry.getValue().values()); - Collections.sort(lightSources); - - LIGHTSOURCES: - for (LightSource lightSource : lightSources) { - for (Light light : lightSource.getLightList()) { - // TODO Shouldn't we only skip if *all* child lights are GM only. And what about owner - // only? - if (light.isGM() && !MapTool.getPlayer().isGM()) { - continue LIGHTSOURCES; - } - } + JMenu subMenu = createLightCategoryMenu(entry.getKey(), entry.getValue().values()); + if (subMenu.getItemCount() != 0) { + menu.add(subMenu); + } + } + return menu; + } + + protected JMenu createLightCategoryMenu(String categoryName, Collection sources) { + JMenu subMenu = new JMenu(categoryName); + + List lightSources = new ArrayList<>(sources); + Collections.sort(lightSources); + + for (LightSource lightSource : lightSources) { + // Don't include light sources that don't have lights visible to the player. Note that the + // player must be an owner to use the popup, so don't bother checking `::isOwner()`. + boolean include = + MapTool.getPlayer().isGM() || !lightSource.getLightList().stream().allMatch(Light::isGM); + if (include) { JCheckBoxMenuItem menuItem = new JCheckBoxMenuItem(new ToggleLightSourceAction(lightSource)); menuItem.setSelected(tokenUnderMouse.hasLightSource(lightSource)); subMenu.add(menuItem); } - if (subMenu.getItemCount() != 0) { - menu.add(subMenu); - } } - return menu; + + return subMenu; } protected Token getTokenUnderMouse() { From 67196365a1b7715b31845b67b7aa80bb3e84d226 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Thu, 2 Nov 2023 14:52:06 -0700 Subject: [PATCH 7/8] Remove commented out light sources in LightSourceCreator --- .../java/net/rptools/maptool/tool/LightSourceCreator.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java b/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java index f752fd6ab6..0379f1eb29 100644 --- a/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java +++ b/src/main/java/net/rptools/maptool/tool/LightSourceCreator.java @@ -63,9 +63,7 @@ private static LightSource createLightSource(String name, double radius, double new GUID(), LightSource.Type.NORMAL, false, - List.of( - // new Light(0, 5, arcAngle, new DrawableColorPaint(new Color(255, 255, 0, 50))) - new Light(ShapeType.CIRCLE, 0, radius, arcAngle, null, 100, false, false))); + List.of(new Light(ShapeType.CIRCLE, 0, radius, arcAngle, null, 100, false, false))); } private static LightSource createD20LightSource(String name, double radius, double arcAngle) { @@ -75,7 +73,6 @@ private static LightSource createD20LightSource(String name, double radius, doub LightSource.Type.NORMAL, false, List.of( - // new Light(0, 5, arcAngle, new DrawableColorPaint(new Color(255, 255, 0, 50))) new Light(ShapeType.CIRCLE, 0, radius, arcAngle, null, 100, false, false), new Light( ShapeType.CIRCLE, From 29e4660fdcf6e37659cb8d1cab15e8db616f8154 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Thu, 2 Nov 2023 15:17:55 -0700 Subject: [PATCH 8/8] Document LightSource.lightList regarding the serialization subtleties --- .../net/rptools/maptool/model/LightSource.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/main/java/net/rptools/maptool/model/LightSource.java b/src/main/java/net/rptools/maptool/model/LightSource.java index 88ba8eb722..0da670d38e 100644 --- a/src/main/java/net/rptools/maptool/model/LightSource.java +++ b/src/main/java/net/rptools/maptool/model/LightSource.java @@ -48,6 +48,20 @@ public enum Type { private final @Nullable GUID id; private final @Nonnull Type type; private final boolean scaleWithToken; + + /** + * This light segments that make up the light source. + * + *

In practice this will be an {@code ImmutableList} during runtime. However, previously + * serialized {@code LightSource} instances may have specified that it must be a {@code + * LinkedList} or other specific {@code List} implementation. So we need to keep this as a {@code + * List} in order to deserialize those. + * + *

There is also one case where it won't be an {@code ImmutableList}, and that is during + * serialization. At such a time, a temporary {@code LightSource} is created with an {@code + * ArrayList} instead. (see {@link #writeReplace()}) so that the XML does not depend on the use of + * {@code ImmutableList} or any other particular {@code List} implementation. + */ private final @Nonnull List lightList; // Lumens are now in the individual Lights. This field is only here for backwards compatibility