Merge pull request #3920 from kwvanderlinde/bugfix/3913-npe-LookupTab…

…le-toDto

Handle nullabilty when converting LookupTable to DTO

src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java

@@ -309,7 +309,7 @@ public Object childEvaluate(
       entryDetails.addProperty("min", entry.getMin());
       entryDetails.addProperty("max", entry.getMax());
       entryDetails.addProperty("value", entry.getValue());
-      entryDetails.addProperty("picked", entry.getPicked() == null ? false : entry.getPicked());
+      entryDetails.addProperty("picked", entry.getPicked());
 
       MD5Key imageId = entry.getImageId();
       if (imageId != null) {

src/main/java/net/rptools/maptool/model/LookupTable.java

@@ -17,6 +17,8 @@
 import com.google.protobuf.StringValue;
 import java.util.*;
 import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import net.rptools.common.expression.ExpressionParser;
 import net.rptools.common.expression.Result;
 import net.rptools.lib.MD5Key;
@@ -29,15 +31,15 @@ public class LookupTable {
 
   private static ExpressionParser expressionParser = new ExpressionParser();
 
-  private List<LookupEntry> entryList = new ArrayList<>();
-  private String name;
-  private String defaultRoll;
-  private MD5Key tableImage;
-  private Boolean visible;
-  private Boolean allowLookup;
+  private @Nonnull List<LookupEntry> entryList = new ArrayList<>();
+  private @Nullable String name;
+  private @Nullable String defaultRoll;
+  private @Nullable MD5Key tableImage;
+  private @Nonnull Boolean visible = true;
+  private @Nonnull Boolean allowLookup = true;
   // Flags a table as Pick Once, i.e. each entry can only be chosen once before the
   // table must be reset().
-  private Boolean pickOnce = false;
+  private @Nonnull Boolean pickOnce = false;
 
   public static final String NO_PICKS_LEFT = "NO_PICKS_LEFT";
 
@@ -47,11 +49,10 @@ public LookupTable(LookupTable table) {
     name = table.name;
     defaultRoll = table.defaultRoll;
     tableImage = table.tableImage;
-    pickOnce = Objects.requireNonNullElse(table.pickOnce, false);
-
-    if (table.entryList != null) {
-      entryList.addAll(table.entryList);
-    }
+    pickOnce = table.pickOnce;
+    visible = table.visible;
+    allowLookup = table.allowLookup;
+    entryList.addAll(table.entryList);
   }
 
   public void setRoll(String roll) {
@@ -309,7 +310,7 @@ public void setTableImage(MD5Key tableImage) {
    *
    * @return Boolean - true if table is Pick Once
    */
-  public Boolean getPickOnce() {
+  public boolean getPickOnce() {
     // Older tables won't have it set.
     if (pickOnce == null) {
       pickOnce = false;
@@ -324,7 +325,7 @@ public Boolean getPickOnce() {
    *
    * @param pickOnce - Boolean
    */
-  public void setPickOnce(Boolean pickOnce) {
+  public void setPickOnce(boolean pickOnce) {
     this.pickOnce = pickOnce;
     this.reset();
   }
@@ -368,14 +369,12 @@ public static class LookupEntry {
     private int min;
     private int max;
     // For Pick Once tables each entry is flagged as picked (true) or not (false).
-    private Boolean picked = false;
-
-    private String value;
+    private @Nonnull Boolean picked = false;
+    private @Nullable String value;
+    private @Nullable MD5Key imageId;
 
-    private MD5Key imageId;
-
-    /** @Deprecated here to prevent xstream from breaking b24-b25 */
-    private String result;
+    /** @deprecated here to prevent xstream from breaking b24-b25 */
+    @Deprecated private @Nullable String result;
 
     public LookupEntry(int min, int max, String value, MD5Key imageId) {
       this.min = min;
@@ -384,10 +383,16 @@ public LookupEntry(int min, int max, String value, MD5Key imageId) {
       this.imageId = imageId;
     }
 
+    @SuppressWarnings("ConstantValue")
     private Object readResolve() {
       if (picked == null) {
         picked = false;
       }
+      // Temporary fix to convert b24 to b25
+      if (result != null) {
+        value = result;
+        result = null;
+      }
 
       return this;
     }
@@ -396,11 +401,11 @@ public MD5Key getImageId() {
       return imageId;
     }
 
-    public void setPicked(Boolean b) {
+    public void setPicked(boolean b) {
       picked = b;
     }
 
-    public Boolean getPicked() {
+    public boolean getPicked() {
       return picked;
     }
 
@@ -413,11 +418,6 @@ public int getMin() {
     }
 
     public String getValue() {
-      // Temporary fix to convert b24 to b25
-      if (result != null) {
-        value = result;
-        result = null;
-      }
       return value;
     }
 
@@ -467,10 +467,7 @@ public Set<MD5Key> getAllAssetIds() {
    * @return Boolean -- True indicates that the table will be visible to players. False indicates
    *     that the table will be hidden from players.
    */
-  public Boolean getVisible() {
-    if (visible == null) {
-      visible = true;
-    }
+  public boolean getVisible() {
     return visible;
   }
 
@@ -480,7 +477,7 @@ public Boolean getVisible() {
    * @param value(Boolean) -- True specifies that the table will be visible to players. False
    *     indicates that the table will be hidden from players.
    */
-  public void setVisible(Boolean value) {
+  public void setVisible(boolean value) {
     visible = value;
   }
 
@@ -491,10 +488,7 @@ public void setVisible(Boolean value) {
    *     indicates that players will be prevented from calling values from this table. GM's can
    *     ALWAYS perform lookups against a table.
    */
-  public Boolean getAllowLookup() {
-    if (allowLookup == null) {
-      allowLookup = true;
-    }
+  public boolean getAllowLookup() {
     return allowLookup;
   }
 
@@ -505,10 +499,11 @@ public Boolean getAllowLookup() {
    *     indicates that players will be prevented from calling values from this table. GM's can
    *     ALWAYS perform lookups against a table.
    */
-  public void setAllowLookup(Boolean value) {
+  public void setAllowLookup(boolean value) {
     allowLookup = value;
   }
 
+  @SuppressWarnings("ConstantValue")
   private Object readResolve() {
     if (visible == null) {
       visible = true;
@@ -527,10 +522,10 @@ private Object readResolve() {
 
   public static LookupTable fromDto(LookupTableDto dto) {
     var table = new LookupTable();
-    table.name = dto.getName();
+    table.name = dto.hasName() ? dto.getName().getValue() : null;
     table.entryList =
         dto.getEntriesList().stream().map(e -> LookupEntry.fromDto(e)).collect(Collectors.toList());
-    table.defaultRoll = dto.getDefaultRoll();
+    table.defaultRoll = dto.hasDefaultRoll() ? dto.getDefaultRoll().getValue() : null;
     table.tableImage = dto.hasTableImage() ? new MD5Key(dto.getTableImage().getValue()) : null;
     table.setVisible(dto.getVisible());
     table.setAllowLookup(dto.getAllowLookup());
@@ -541,9 +536,11 @@ public static LookupTable fromDto(LookupTableDto dto) {
   public LookupTableDto toDto() {
     var dto = LookupTableDto.newBuilder();
     dto.addAllEntries(entryList.stream().map(e -> e.toDto()).collect(Collectors.toList()));
-    dto.setName(name);
+    if (name != null) {
+      dto.setName(StringValue.of(name));
+    }
     if (defaultRoll != null) {
-      dto.setDefaultRoll(defaultRoll);
+      dto.setDefaultRoll(StringValue.of(defaultRoll));
     }
     if (tableImage != null) {
       dto.setTableImage(StringValue.of(tableImage.toString()));

src/main/proto/data_transfer_objects.proto

@@ -49,8 +49,8 @@ message CampaignDto {
 
 message LookupTableDto {
   repeated LookupEntryDto entries = 1;
-  string name = 2;
-  string default_roll = 3;
+  google.protobuf.StringValue name = 2;
+  google.protobuf.StringValue default_roll = 3;
   google.protobuf.StringValue table_image = 4;
   bool visible = 5;
   bool allow_lookup = 6;