From ee9a5358d0e2fc7f9360bd0825e92c3860cb0437 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Fri, 16 Feb 2024 22:07:30 -0500 Subject: [PATCH 01/11] Update nightly-publish.yml Disabled nightly build by changing an element that would normally schedule a job. --- .github/workflows/nightly-publish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nightly-publish.yml b/.github/workflows/nightly-publish.yml index 19e53f118b..eb60a76f95 100644 --- a/.github/workflows/nightly-publish.yml +++ b/.github/workflows/nightly-publish.yml @@ -1,7 +1,7 @@ -name: Nightly Release +name: Nightly Release (disabled) on: - schedule: + watch: # Change to 'schedule' to re-enable this build - cron: '13 2 * * *' # Odd time so that it doesn't run afoul of busy periods everyone picks jobs: From e1796d2b822903c0ed222033a4d0cc170a4a89e2 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Tue, 30 Apr 2024 01:45:21 -0400 Subject: [PATCH 02/11] Fixup of setTableEntry performance FullBleed complained on Discord about the performance of this MTscript function. It turns out that the algorithm is O(n), so putting 3200 entries in a table gets seriously slow. Fixing this will require the supporting `LookupTable` class to have its own `setTableEntry` method. This commit is some simpler formatting and refactoring fixes; the next commit will be more substantial in regards to the actual fix. --- .../client/functions/LookupTableFunction.java | 713 +++++++++--------- .../client/functions/getInfoFunction.java | 31 +- .../rptools/maptool/util/FunctionUtil.java | 17 +- 3 files changed, 391 insertions(+), 370 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java index d75e218a27..dfec967ffb 100644 --- a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java @@ -16,10 +16,6 @@ import com.google.gson.JsonArray; import com.google.gson.JsonObject; -import java.math.BigDecimal; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.functions.json.JSONMacroFunctions; @@ -32,6 +28,13 @@ import net.rptools.parser.VariableResolver; import net.rptools.parser.function.AbstractFunction; import org.apache.commons.lang.StringUtils; +import org.jetbrains.annotations.NotNull; + +import java.io.Serializable; +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; public class LookupTableFunction extends AbstractFunction { @@ -84,362 +87,92 @@ public Object childEvaluate( throws ParserException { if ("getTableNames".equalsIgnoreCase(function)) { - - FunctionUtil.checkNumberParam("getTableNames", params, 0, 1); - String delim = ","; - if (params.size() > 0) { - delim = params.get(0).toString(); - } - if ("json".equalsIgnoreCase(delim)) { - JsonArray jsonArray = new JsonArray(); - getTableList(MapTool.getPlayer().isGM()).forEach(jsonArray::add); - return jsonArray; - } - return StringUtils.join(getTableList(MapTool.getPlayer().isGM()), delim); - + return getTableNames(params); } else if ("getTableVisible".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("getTableVisible", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - return lookupTable.getVisible() ? BigDecimal.ONE : BigDecimal.ZERO; - + return getTableVisible(function, params); } else if ("setTableVisible".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("setTableVisible", params, 2, 2); - String name = params.get(0).toString(); - String visible = params.get(1).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - lookupTable.setVisible(FunctionUtil.getBooleanValue(visible)); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return lookupTable.getVisible() ? BigDecimal.ONE : BigDecimal.ZERO; - + return setTableVisible(function, params); } else if ("getTableAccess".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("getTableAccess", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - return lookupTable.getAllowLookup() ? BigDecimal.ONE : BigDecimal.ZERO; - + return getTableAccess(function, params); } else if ("setTableAccess".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("setTableAccess", params, 2, 2); - String name = params.get(0).toString(); - String access = params.get(1).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - lookupTable.setAllowLookup(FunctionUtil.getBooleanValue(access)); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return lookupTable.getAllowLookup() ? BigDecimal.ONE : BigDecimal.ZERO; - + return setTableAccess(function, params); } else if ("getTableRoll".equalsIgnoreCase(function)) { - - FunctionUtil.checkNumberParam("getTableRoll", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - return lookupTable.getRoll(); - + return getTableRoll(function, params); } else if ("setTableRoll".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("setTableRoll", params, 2, 2); - String name = params.get(0).toString(); - String roll = params.get(1).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - lookupTable.setRoll(roll); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return lookupTable.getRoll(); - + return setTableRoll(function, params); } else if ("clearTable".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("clearTable", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - lookupTable.clearEntries(); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; - + return clearTable(function, params); } else if ("addTableEntry".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("addTableEntry", params, 4, 5); - String name = params.get(0).toString(); - String min = params.get(1).toString(); - String max = params.get(2).toString(); - String value = params.get(3).toString(); - MD5Key asset = null; - if (params.size() > 4) { - asset = FunctionUtil.getAssetKeyFromString(params.get(4).toString()); - } - LookupTable lookupTable = getMaptoolTable(name, function); - lookupTable.addEntry(Integer.parseInt(min), Integer.parseInt(max), value, asset); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; - + return addTableEntry(function, params); } else if ("deleteTableEntry".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("deleteTableEntry", params, 2, 2); - String name = params.get(0).toString(); - String roll = params.get(1).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - LookupEntry entry = lookupTable.getLookup(roll); - if (entry != null) { - List oldlist = new ArrayList<>(lookupTable.getEntryList()); - lookupTable.clearEntries(); - oldlist.stream() - .filter((e) -> (e != entry)) - .forEachOrdered( - (e) -> lookupTable.addEntry(e.getMin(), e.getMax(), e.getValue(), e.getImageId())); - } - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; - + return deleteTableEntry(function, params); } else if ("createTable".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("createTable", params, 3, 4); - String name = params.get(0).toString(); - String visible = params.get(1).toString(); - String lookups = params.get(2).toString(); - MD5Key asset = null; - if (params.size() > 3) { - asset = FunctionUtil.getAssetKeyFromString(params.get(3).toString()); - } - LookupTable lookupTable = new LookupTable(); - lookupTable.setName(name); - lookupTable.setVisible(FunctionUtil.getBooleanValue(visible)); - lookupTable.setAllowLookup(FunctionUtil.getBooleanValue(lookups)); - if (asset != null) lookupTable.setTableImage(asset); - MapTool.getCampaign().getLookupTableMap().put(name, lookupTable); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; - + return createTable(function, params); } else if ("deleteTable".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("deleteTable", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - MapTool.getCampaign().getLookupTableMap().remove(name); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; - + return deleteTable(function, params); } else if ("getTableImage".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("getTableImage", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - MD5Key img = lookupTable.getTableImage(); - // Returning null causes an NPE when output is dumped to chat. - return Objects.requireNonNullElse(img, ""); - + return getTableImage(function, params); } else if ("setTableImage".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("setTableImage", params, 2, 2); - String name = params.get(0).toString(); - MD5Key asset = FunctionUtil.getAssetKeyFromString(params.get(1).toString()); - LookupTable lookupTable = getMaptoolTable(name, function); - lookupTable.setTableImage(asset); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; - + return setTableImage(function, params); } else if ("copyTable".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("copyTable", params, 2, 2); - String oldName = params.get(0).toString(); - String newName = params.get(1).toString(); - LookupTable oldTable = getMaptoolTable(oldName, function); - LookupTable newTable = new LookupTable(oldTable); - newTable.setName(newName); - MapTool.getCampaign().getLookupTableMap().put(newName, newTable); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; - + return copyTable(function, params); } else if ("setTableEntry".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("setTableEntry", params, 3, 4); - String name = params.get(0).toString(); - String roll = params.get(1).toString(); - String result = params.get(2).toString(); - MD5Key imageId = null; - if (params.size() == 4) { - imageId = FunctionUtil.getAssetKeyFromString(params.get(3).toString()); - } - LookupTable lookupTable = getMaptoolTable(name, function); - LookupEntry entry = lookupTable.getLookup(roll); - if (entry == null) return 0; // no entry was found - int rollInt = Integer.parseInt(roll); - if (rollInt < entry.getMin() || rollInt > entry.getMax()) - return 0; // entry was found but doesn't match - List oldlist = new ArrayList<>(lookupTable.getEntryList()); - lookupTable.clearEntries(); - for (LookupEntry e : oldlist) - if (e != entry) { - lookupTable.addEntry(e.getMin(), e.getMax(), e.getValue(), e.getImageId()); - } else { - if (imageId == null) imageId = e.getImageId(); - - lookupTable.addEntry(e.getMin(), e.getMax(), result, imageId); - } - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return 1; + return setTableEntry(function, params); } else if ("getTableEntry".equalsIgnoreCase(function)) { - - FunctionUtil.checkNumberParam(function, params, 2, 2); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - String roll = params.get(1).toString(); - LookupEntry entry = lookupTable.getLookupDirect(roll); - if (entry == null) return ""; // no entry was found - - int rollInt = Integer.parseInt(roll); - if (rollInt < entry.getMin() || rollInt > entry.getMax()) - return ""; // entry was found but doesn't match - - JsonObject entryDetails = new JsonObject(); - entryDetails.addProperty("min", entry.getMin()); - entryDetails.addProperty("max", entry.getMax()); - entryDetails.addProperty("value", entry.getValue()); - entryDetails.addProperty("picked", entry.getPicked()); - - MD5Key imageId = entry.getImageId(); - if (imageId != null) { - entryDetails.addProperty("assetid", "asset://" + imageId.toString()); - } else { - entryDetails.addProperty("assetid", ""); - } - return entryDetails; + return getTableEntry(function, params); } else if ("resetTablePicks".equalsIgnoreCase(function)) { - /* - * resetTablePicks(tblName) - reset all entries on a table - * resetTablePicks(tblName, entriesToReset) - reset specific entries from a String List with "," delim - * resetTablePicks(tblName, entriesToReset, delim) - use custom delimiter - * resetTablePicks(tblName, entriesToReset, "json") - entriesToReset is a JsonArray - */ - checkTrusted(function); - FunctionUtil.checkNumberParam(function, params, 1, 3); - String tblName = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(tblName, function); - if (params.size() > 1) { - String delim = (params.size() > 2) ? params.get(2).toString() : ","; - List entriesToReset; - if (delim.equalsIgnoreCase("json")) { - JsonArray jsonArray = FunctionUtil.paramAsJsonArray(function, params, 1); - entriesToReset = - JSONMacroFunctions.getInstance() - .getJsonArrayFunctions() - .jsonArrayToListOfStrings(jsonArray); - } else { - entriesToReset = StrListFunctions.toList(params.get(1).toString(), delim); - } - lookupTable.reset(entriesToReset); - } else { - lookupTable.reset(); - } - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return ""; + return resetTablePicks(function, params); } else if ("setTablePickOnce".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("setTablePickOnce", params, 2, 2); - String name = params.get(0).toString(); - String pickonce = params.get(1).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - lookupTable.setPickOnce(FunctionUtil.getBooleanValue(pickonce)); - MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return lookupTable.getPickOnce() ? BigDecimal.ONE : BigDecimal.ZERO; - + return setTablePickOnce(function, params); } else if ("getTablePickOnce".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("getTablePickOnce", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - return lookupTable.getPickOnce() ? BigDecimal.ONE : BigDecimal.ZERO; - + return getTablePickOnce(function, params); } else if ("getTablePicksLeft".equalsIgnoreCase(function)) { - - checkTrusted(function); - FunctionUtil.checkNumberParam("getTablePicksLeft", params, 1, 1); - String name = params.get(0).toString(); - LookupTable lookupTable = getMaptoolTable(name, function); - return lookupTable.getPicksLeft(); - + return getTablePicksLeft(function, params); } else { // if tbl, table, tblImage or tableImage FunctionUtil.checkNumberParam(function, params, 1, 3); String name = params.get(0).toString(); String roll = null; if (params.size() > 1) { - roll = params.get(1).toString().length() == 0 ? null : params.get(1).toString(); - } - - LookupTable lookupTable = MapTool.getCampaign().getLookupTableMap().get(name); - if (!MapTool.getPlayer().isGM() && !lookupTable.getAllowLookup()) { - if (lookupTable.getVisible()) { - throw new ParserException( - function + "(): " + I18N.getText("msg.error.tableUnknown") + name); - } else { - throw new ParserException( - function + "(): " + I18N.getText("msg.error.tableAccessProhibited") + ": " + name); - } - } - if (lookupTable == null) { - throw new ParserException( - I18N.getText("macro.function.LookupTableFunctions.unknownTable", function, name)); + roll = params.get(1).toString().isEmpty() ? null : params.get(1).toString(); } + LookupTable lookupTable = checkTableAccess(function, name); LookupEntry result = lookupTable.getLookup(roll); if (result == null) { return null; } + assert result.getValue() != null; if (result.getValue().equals(LookupTable.NO_PICKS_LEFT)) { return result.getValue(); } - if (function.equals("table") || function.equals("tbl")) { + if (function.equalsIgnoreCase("table") || function.equalsIgnoreCase("tbl")) { String val = result.getValue(); try { - BigDecimal bival = new BigDecimal(val); - return bival; + return new BigDecimal(val); } catch (NumberFormatException nfe) { return val; } - } else if ("tableImage".equalsIgnoreCase(function) - || "tblImage" - .equalsIgnoreCase(function)) { // We want the image URI through tblImage or tableImage - + } else if (function.equalsIgnoreCase("tableImage") || function.equalsIgnoreCase("tblImage")) { // We want the image URI through tblImage or tableImage if (result.getImageId() == null) { return ""; // empty string if no image is found (#538) } + StringBuilder assetId = new StringBuilder("asset://"); + assetId.append(result.getImageId().toString()); - BigDecimal size = null; if (params.size() > 2) { - if (params.get(2) instanceof BigDecimal) { - size = (BigDecimal) params.get(2); - } else { + try { + Integer imageSize = FunctionUtil.paramAsInteger(function, params, 2, false); + int i = Math.max(imageSize, 1); // Constrain to a minimum of 1 + assetId.append("-"); + assetId.append(i); + } catch (ParserException pe) { throw new ParserException( I18N.getText("macro.function.LookupTableFunctions.invalidSize", function)); } } - - StringBuilder assetId = new StringBuilder("asset://"); - assetId.append(result.getImageId().toString()); - if (size != null) { - int i = Math.max(size.intValue(), 1); // Constrain to a minimum of 1 - assetId.append("-"); - assetId.append(i); - } return assetId.toString(); } else { throw new ParserException(I18N.getText("macro.function.general.unknownFunction", function)); @@ -447,6 +180,334 @@ public Object childEvaluate( } } + /** + * Returns the lookupTable object identified by name if + * it exists in the campaign. Otherwise, throws ParserException. + * @param function name of MTscript function (for error message) + * @param name name of table + * @return table identified by name + * @throws ParserException Thrown for any access errors to the named table (unknown, access prohibited, or table doesn't exist) + */ + private LookupTable checkTableAccess(String function, String name) throws ParserException { + LookupTable lookupTable = MapTool.getCampaign().getLookupTableMap().get(name); + if (!MapTool.getPlayer().isGM() && !lookupTable.getAllowLookup()) { + if (lookupTable.getVisible()) { + throw new ParserException( + function + "(): " + I18N.getText("msg.error.tableUnknown") + name); + } else { + throw new ParserException( + function + "(): " + I18N.getText("msg.error.tableAccessProhibited") + ": " + name); + } + } + if (lookupTable == null) { + throw new ParserException( + I18N.getText("macro.function.LookupTableFunctions.unknownTable", function, name)); + } + return lookupTable; + } + + private int getTablePicksLeft(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("getTablePicksLeft", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + return lookupTable.getPicksLeft(); + } + + private @NotNull BigDecimal getTablePickOnce(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("getTablePickOnce", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + return lookupTable.getPickOnce() ? BigDecimal.ONE : BigDecimal.ZERO; + } + + private @NotNull BigDecimal setTablePickOnce(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("setTablePickOnce", params, 2, 2); + String name = params.get(0).toString(); + String pickonce = params.get(1).toString(); + LookupTable lookupTable = checkTableAccess(name, function); + lookupTable.setPickOnce(FunctionUtil.getBooleanValue(pickonce)); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return lookupTable.getPickOnce() ? BigDecimal.ONE : BigDecimal.ZERO; + } + + /** + * Resets the entries on a table in regards to whether such entries have been + * selected already. This is used in the PickOnce family of tables. + * + * resetTablePicks(tblName) - reset all entries on a table + * resetTablePicks(tblName, entriesToReset) - reset specific entries from a String List with "," delim + * resetTablePicks(tblName, entriesToReset, delim) - use custom delimiter + * resetTablePicks(tblName, entriesToReset, "json") - entriesToReset is a JsonArray + * + * @param function name of the MTscript function (used for error messages) + * @param params list of function parameters + * @throws ParserException Thrown when an invalid or missing parameter is supplied + */ + private @NotNull String resetTablePicks(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam(function, params, 1, 3); + String tblName = params.get(0).toString(); + LookupTable lookupTable = checkTableAccess(tblName, function); + if (params.size() > 1) { + String delim = (params.size() > 2) ? params.get(2).toString() : ","; + List entriesToReset; + if (delim.equalsIgnoreCase("json")) { + JsonArray jsonArray = FunctionUtil.paramAsJsonArray(function, params, 1); + entriesToReset = + JSONMacroFunctions.getInstance() + .getJsonArrayFunctions() + .jsonArrayToListOfStrings(jsonArray); + } else { + entriesToReset = StrListFunctions.toList(params.get(1).toString(), delim); + } + lookupTable.reset(entriesToReset); + } else { + lookupTable.reset(); + } + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private @NotNull Object getTableEntry(String function, List params) throws ParserException { + FunctionUtil.checkNumberParam(function, params, 2, 2); + String name = params.get(0).toString(); + LookupTable lookupTable = checkTableAccess(name, function); + String roll = params.get(1).toString(); + LookupEntry entry = lookupTable.getLookupDirect(roll); + if (entry == null) return ""; + + int rollInt = Integer.parseInt(roll); + if (rollInt < entry.getMin() || rollInt > entry.getMax()) + return ""; + + JsonObject entryDetails = new JsonObject(); + entryDetails.addProperty("min", entry.getMin()); + entryDetails.addProperty("max", entry.getMax()); + entryDetails.addProperty("value", entry.getValue()); + entryDetails.addProperty("picked", entry.getPicked()); + + MD5Key imageId = entry.getImageId(); + if (imageId != null) { + entryDetails.addProperty("assetid", "asset://" + imageId.toString()); + } else { + entryDetails.addProperty("assetid", ""); + } + return entryDetails; + } + + private int setTableEntry(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("setTableEntry", params, 3, 4); + String name = params.get(0).toString(); + String roll = params.get(1).toString(); + String result = params.get(2).toString(); + MD5Key imageId = null; + if (params.size() == 4) { + imageId = FunctionUtil.getAssetKeyFromString(params.get(3).toString()); + } + LookupTable lookupTable = checkTableAccess(name, function); + LookupEntry entry = lookupTable.getLookup(roll); + if (entry == null) return 0; + int rollInt = Integer.parseInt(roll); + if (rollInt < entry.getMin() || rollInt > entry.getMax()) + return 0; + List oldlist = new ArrayList<>(lookupTable.getEntryList()); + lookupTable.clearEntries(); + for (LookupEntry e : oldlist) + if (e != entry) { + lookupTable.addEntry(e.getMin(), e.getMax(), e.getValue(), e.getImageId()); + } else { + if (imageId == null) imageId = e.getImageId(); + + lookupTable.addEntry(e.getMin(), e.getMax(), result, imageId); + } + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return 1; + } + + private @NotNull String copyTable(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("copyTable", params, 2, 2); + String oldName = params.get(0).toString(); + String newName = params.get(1).toString(); + LookupTable oldTable = checkTableAccess(oldName, function); + LookupTable newTable = new LookupTable(oldTable); + newTable.setName(newName); + MapTool.getCampaign().getLookupTableMap().put(newName, newTable); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private @NotNull String setTableImage(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("setTableImage", params, 2, 2); + String name = params.get(0).toString(); + MD5Key asset = FunctionUtil.getAssetKeyFromString(params.get(1).toString()); + LookupTable lookupTable = checkTableAccess(name, function); + lookupTable.setTableImage(asset); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private @NotNull Serializable getTableImage(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("getTableImage", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + MD5Key img = lookupTable.getTableImage(); + // Returning null causes an NPE when output is dumped to chat. + return Objects.requireNonNullElse(img, ""); + } + + private @NotNull String deleteTable(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("deleteTable", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + MapTool.getCampaign().getLookupTableMap().remove(name); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private @NotNull String createTable(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("createTable", params, 3, 4); + String name = params.get(0).toString(); + String visible = params.get(1).toString(); + String lookups = params.get(2).toString(); + MD5Key asset = null; + if (params.size() > 3) { + asset = FunctionUtil.getAssetKeyFromString(params.get(3).toString()); + } + LookupTable lookupTable = new LookupTable(); + lookupTable.setName(name); + lookupTable.setVisible(FunctionUtil.getBooleanValue(visible)); + lookupTable.setAllowLookup(FunctionUtil.getBooleanValue(lookups)); + if (asset != null) lookupTable.setTableImage(asset); + MapTool.getCampaign().getLookupTableMap().put(name, lookupTable); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private @NotNull String deleteTableEntry(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("deleteTableEntry", params, 2, 2); + String name = params.get(0).toString(); + String roll = params.get(1).toString(); + LookupTable lookupTable = checkTableAccess(name, function); + LookupEntry entry = lookupTable.getLookup(roll); + if (entry != null) { + List oldlist = new ArrayList<>(lookupTable.getEntryList()); + lookupTable.clearEntries(); + oldlist.stream() + .filter((e) -> (e != entry)) + .forEachOrdered( + (e) -> lookupTable.addEntry(e.getMin(), e.getMax(), e.getValue(), e.getImageId())); + } + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private @NotNull String addTableEntry(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("addTableEntry", params, 4, 5); + String name = params.get(0).toString(); + String min = params.get(1).toString(); + String max = params.get(2).toString(); + String value = params.get(3).toString(); + MD5Key asset = null; + if (params.size() > 4) { + asset = FunctionUtil.getAssetKeyFromString(params.get(4).toString()); + } + LookupTable lookupTable = checkTableAccess(name, function); + lookupTable.addEntry(Integer.parseInt(min), Integer.parseInt(max), value, asset); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private @NotNull String clearTable(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("clearTable", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + lookupTable.clearEntries(); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return ""; + } + + private String setTableRoll(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("setTableRoll", params, 2, 2); + String name = params.get(0).toString(); + String roll = params.get(1).toString(); + LookupTable lookupTable = checkTableAccess(name, function); + lookupTable.setRoll(roll); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return lookupTable.getRoll(); + } + + private String getTableRoll(String function, List params) throws ParserException { + FunctionUtil.checkNumberParam("getTableRoll", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + return lookupTable.getRoll(); + } + + private @NotNull BigDecimal setTableAccess(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("setTableAccess", params, 2, 2); + String name = params.get(0).toString(); + String access = params.get(1).toString(); + LookupTable lookupTable = checkTableAccess(name, function); + lookupTable.setAllowLookup(FunctionUtil.getBooleanValue(access)); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return lookupTable.getAllowLookup() ? BigDecimal.ONE : BigDecimal.ZERO; + } + + private @NotNull BigDecimal getTableAccess(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("getTableAccess", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + return lookupTable.getAllowLookup() ? BigDecimal.ONE : BigDecimal.ZERO; + } + + private @NotNull BigDecimal setTableVisible(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("setTableVisible", params, 2, 2); + String name = params.get(0).toString(); + String visible = params.get(1).toString(); + LookupTable lookupTable = checkTableAccess(name, function); + lookupTable.setVisible(FunctionUtil.getBooleanValue(visible)); + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return lookupTable.getVisible() ? BigDecimal.ONE : BigDecimal.ZERO; + } + + private @NotNull BigDecimal getTableVisible(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam("getTableVisible", params, 1, 1); + String name = params.getFirst().toString(); + LookupTable lookupTable = checkTableAccess(name, function); + return lookupTable.getVisible() ? BigDecimal.ONE : BigDecimal.ZERO; + } + + private Object getTableNames(List params) throws ParserException { + FunctionUtil.checkNumberParam("getTableNames", params, 0, 1); + String delim = ","; + if (!params.isEmpty()) { + delim = params.getFirst().toString(); + } + if ("json".equalsIgnoreCase(delim)) { + JsonArray jsonArray = new JsonArray(); + getTableList(MapTool.getPlayer().isGM()).forEach(jsonArray::add); + return jsonArray; + } + return StringUtils.join(getTableList(MapTool.getPlayer().isGM()), delim); + } + /** * Checks whether or not the function is trusted * @@ -474,38 +535,4 @@ private List getTableList(boolean isGm) { .forEachOrdered((lt) -> tables.add(lt.getName())); return tables; } - - /** - * Function to return a MapTool table. - * - * @param tableName String containing the name of the desired table - * @param functionName String containing the name of the calling function, used by the error - * message. - * @return LookupTable The desired MapTool table object - * @throws ParserException if there were more or less parameters than allowed - */ - private LookupTable getMaptoolTable(String tableName, String functionName) - throws ParserException { - - LookupTable lookupTable = MapTool.getCampaign().getLookupTableMap().get(tableName); - if (!MapTool.getPlayer().isGM() && !lookupTable.getAllowLookup()) { - if (lookupTable.getVisible()) { - throw new ParserException( - functionName + "(): " + I18N.getText("msg.error.tableUnknown") + tableName); - } else { - throw new ParserException( - functionName - + "(): " - + I18N.getText("msg.error.tableAccessProhibited") - + ": " - + tableName); - } - } - if (lookupTable == null) { - throw new ParserException( - I18N.getText( - "macro.function.LookupTableFunctions.unknownTable", functionName, tableName)); - } - return lookupTable; - } } diff --git a/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java b/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java index c691a0a845..90f555856b 100644 --- a/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java @@ -17,15 +17,6 @@ import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonObject; -import java.awt.*; -import java.math.BigDecimal; -import java.text.SimpleDateFormat; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; -import java.util.*; -import java.util.List; -import java.util.concurrent.ConcurrentSkipListSet; -import javax.swing.*; import net.rptools.maptool.client.AppPreferences; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.MapToolExpressionParser; @@ -35,17 +26,7 @@ import net.rptools.maptool.client.ui.token.*; import net.rptools.maptool.client.ui.zone.renderer.ZoneRenderer; import net.rptools.maptool.language.I18N; -import net.rptools.maptool.model.Campaign; -import net.rptools.maptool.model.CampaignProperties; -import net.rptools.maptool.model.Grid; -import net.rptools.maptool.model.GridFactory; -import net.rptools.maptool.model.Light; -import net.rptools.maptool.model.LightSource; -import net.rptools.maptool.model.LookupTable; -import net.rptools.maptool.model.ShapeType; -import net.rptools.maptool.model.SightType; -import net.rptools.maptool.model.Token; -import net.rptools.maptool.model.Zone; +import net.rptools.maptool.model.*; import net.rptools.maptool.model.drawing.DrawableColorPaint; import net.rptools.maptool.model.drawing.DrawableTexturePaint; import net.rptools.maptool.server.ServerPolicy; @@ -57,6 +38,16 @@ import net.rptools.parser.VariableResolver; import net.rptools.parser.function.AbstractFunction; +import javax.swing.*; +import java.awt.*; +import java.math.BigDecimal; +import java.text.SimpleDateFormat; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.util.List; +import java.util.*; +import java.util.concurrent.ConcurrentSkipListSet; + public class getInfoFunction extends AbstractFunction { /** The singleton instance. */ diff --git a/src/main/java/net/rptools/maptool/util/FunctionUtil.java b/src/main/java/net/rptools/maptool/util/FunctionUtil.java index e2ab99ea21..e0d31cd81b 100644 --- a/src/main/java/net/rptools/maptool/util/FunctionUtil.java +++ b/src/main/java/net/rptools/maptool/util/FunctionUtil.java @@ -18,10 +18,6 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; -import java.math.BigDecimal; -import java.util.List; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.MapToolUtil; @@ -41,6 +37,12 @@ import net.rptools.parser.VariableResolver; import net.rptools.parser.function.Function; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.math.BigDecimal; +import java.util.List; +import java.util.Locale; + /** * Provides static methods to help handle macro functions. * @@ -525,14 +527,15 @@ public static DrawablePaint getPaintFromString(String paint) { */ public static @Nullable MD5Key getAssetKeyFromString(String assetUrlOrId) { String id = null; - if (assetUrlOrId.toLowerCase().startsWith("asset://")) { + final String paramAsLC = assetUrlOrId.toLowerCase(Locale.ROOT); + if (paramAsLC.startsWith("asset://")) { id = assetUrlOrId.substring("asset://".length()); - } else if (assetUrlOrId.toLowerCase().startsWith("lib://")) { + } else if (paramAsLC.startsWith("lib://")) { var assetKey = new AssetResolver().getAssetKey(assetUrlOrId); if (assetKey.isPresent()) { id = assetKey.get().toString(); } - } else if (assetUrlOrId.toLowerCase().startsWith("image:")) { + } else if (paramAsLC.startsWith("image:")) { for (ZoneRenderer z : MapTool.getFrame().getZoneRenderers()) { Token t = z.getZone().getTokenByName(assetUrlOrId); if (t != null) { From d2ac3a448c3f691a95e898270f1a7f43a4319fc9 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Wed, 8 May 2024 23:50:32 -0400 Subject: [PATCH 03/11] Unit tests and performance fixes for LookupTable Part 1, laying the groundwork by adding documentation and correcting warnings in the affected file(s). WIP. --- build.gradle | 2 +- .../client/functions/LookupTableFunction.java | 162 +++++++--- .../rptools/maptool/model/LookupTable.java | 278 +++++++++++++----- .../client/functions/LookupTableTests.java | 101 +++++++ .../client/functions/TableFunctionsTest.java | 124 ++++++++ 5 files changed, 553 insertions(+), 114 deletions(-) create mode 100644 src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java create mode 100644 src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java diff --git a/build.gradle b/build.gradle index e0ff2138fe..276f307178 100644 --- a/build.gradle +++ b/build.gradle @@ -585,4 +585,4 @@ test { task createWrapper(type: Wrapper) { gradleVersion = '8.2.1' -} \ No newline at end of file +} diff --git a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java index dfec967ffb..ea1d7af64b 100644 --- a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java @@ -15,7 +15,9 @@ package net.rptools.maptool.client.functions; import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.functions.json.JSONMacroFunctions; @@ -29,6 +31,7 @@ import net.rptools.parser.function.AbstractFunction; import org.apache.commons.lang.StringUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.io.Serializable; import java.math.BigDecimal; @@ -50,6 +53,7 @@ public LookupTableFunction() { "getTableRoll", "setTableRoll", "clearTable", + "loadTable", // bulk table import "addTableEntry", "deleteTableEntry", "createTable", @@ -102,6 +106,8 @@ public Object childEvaluate( return setTableRoll(function, params); } else if ("clearTable".equalsIgnoreCase(function)) { return clearTable(function, params); + } else if ("loadTable".equalsIgnoreCase(function)) { + return loadTable(function, params); } else if ("addTableEntry".equalsIgnoreCase(function)) { return addTableEntry(function, params); } else if ("deleteTableEntry".equalsIgnoreCase(function)) { @@ -129,54 +135,58 @@ public Object childEvaluate( } else if ("getTablePicksLeft".equalsIgnoreCase(function)) { return getTablePicksLeft(function, params); } else { // if tbl, table, tblImage or tableImage - FunctionUtil.checkNumberParam(function, params, 1, 3); - String name = params.get(0).toString(); + return tbl_and_table(function, params); + } + } - String roll = null; - if (params.size() > 1) { - roll = params.get(1).toString().isEmpty() ? null : params.get(1).toString(); - } + private @Nullable Serializable tbl_and_table(String function, List params) throws ParserException { + FunctionUtil.checkNumberParam(function, params, 1, 3); + String name = params.get(0).toString(); - LookupTable lookupTable = checkTableAccess(function, name); - LookupEntry result = lookupTable.getLookup(roll); - if (result == null) { - return null; - } + String roll = null; + if (params.size() > 1) { + roll = params.get(1).toString().isEmpty() ? null : params.get(1).toString(); + } - assert result.getValue() != null; - if (result.getValue().equals(LookupTable.NO_PICKS_LEFT)) { - return result.getValue(); + LookupTable lookupTable = checkTableAccess(function, name); + LookupEntry result = lookupTable.getLookup(roll); + if (result == null) { + return null; + } + + assert result.getValue() != null; + if (result.getValue().equals(LookupTable.NO_PICKS_LEFT)) { + return result.getValue(); + } + + if (function.equalsIgnoreCase("table") || function.equalsIgnoreCase("tbl")) { + String val = result.getValue(); + try { + return new BigDecimal(val); + } catch (NumberFormatException nfe) { + return val; + } + } else if (function.equalsIgnoreCase("tableImage") || function.equalsIgnoreCase("tblImage")) { // We want the image URI through tblImage or tableImage + if (result.getImageId() == null) { + return ""; } + StringBuilder assetId = new StringBuilder("asset://"); + assetId.append(result.getImageId().toString()); - if (function.equalsIgnoreCase("table") || function.equalsIgnoreCase("tbl")) { - String val = result.getValue(); + if (params.size() > 2) { try { - return new BigDecimal(val); - } catch (NumberFormatException nfe) { - return val; + Integer imageSize = FunctionUtil.paramAsInteger(function, params, 2, false); + int i = Math.max(imageSize, 1); // Constrain to a minimum of 1 + assetId.append("-"); + assetId.append(i); + } catch (ParserException pe) { + throw new ParserException( + I18N.getText("macro.function.LookupTableFunctions.invalidSize", function)); } - } else if (function.equalsIgnoreCase("tableImage") || function.equalsIgnoreCase("tblImage")) { // We want the image URI through tblImage or tableImage - if (result.getImageId() == null) { - return ""; // empty string if no image is found (#538) - } - StringBuilder assetId = new StringBuilder("asset://"); - assetId.append(result.getImageId().toString()); - - if (params.size() > 2) { - try { - Integer imageSize = FunctionUtil.paramAsInteger(function, params, 2, false); - int i = Math.max(imageSize, 1); // Constrain to a minimum of 1 - assetId.append("-"); - assetId.append(i); - } catch (ParserException pe) { - throw new ParserException( - I18N.getText("macro.function.LookupTableFunctions.invalidSize", function)); - } - } - return assetId.toString(); - } else { - throw new ParserException(I18N.getText("macro.function.general.unknownFunction", function)); } + return assetId.toString(); + } else { + throw new ParserException(I18N.getText("macro.function.general.unknownFunction", function)); } } @@ -316,7 +326,7 @@ private int setTableEntry(String function, List params) throws ParserExc return 0; List oldlist = new ArrayList<>(lookupTable.getEntryList()); lookupTable.clearEntries(); - for (LookupEntry e : oldlist) + for (LookupEntry e : oldlist) { if (e != entry) { lookupTable.addEntry(e.getMin(), e.getMax(), e.getValue(), e.getImageId()); } else { @@ -324,6 +334,7 @@ private int setTableEntry(String function, List params) throws ParserExc lookupTable.addEntry(e.getMin(), e.getMax(), result, imageId); } + } MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); return 1; } @@ -438,6 +449,75 @@ private int setTableEntry(String function, List params) throws ParserExc return ""; } + /** + * Loads bulk data into a table. The table must already exist. Usage: + *

+ * {@code [h: loadTable(name, jsondata)]} + *

+ *

+ * The {@code jsondata} must be a JSON Array containing table entries, and + * each table entry must be a JSON Array containing: + *

    + *
  1. an integer representing the lower range of the die roll (decimals are truncated),
    2. + *
  2. an integer representing the upper range of the die roll (decimals are truncated),
    3. + *
  3. a String acting as the content of the row, and
    4. + *
  4. a String acting as an image reference (as {@code asset://}, {@code image:}, or {@code MD5Key})
    5. + *
+ *

+ * @param function name of this MTscript function, "{@code loadTable}" + * @param params parameters of the MTscript function + * @return a number indicating the successful record count, or + * a {@link JsonArray} of all elements that failed to load + * @throws ParserException Thrown when syntax errors are detected in the input data. + * This includes negative numbers in either of the numeric fields of a table entry, + * a lower die roll that is greater than the upper die roll of a table entry and vice versa, + * missing fields, too many fields, and potentially many more. + */ + private @NotNull JsonElement loadTable(String function, List params) throws ParserException { + checkTrusted(function); + FunctionUtil.checkNumberParam(function, params, 3, 3); + String name = params.get(0).toString(); + LookupTable lookupTable = checkTableAccess(name, function); + + Object data = params.get(1); + if (data instanceof String) { + data = com.google.gson.JsonParser.parseString(data.toString()); + } + if (data instanceof JsonArray json) { + JsonArray errorRows = new JsonArray(); + long counter = 0, rowindex = 0; + // Verify that all records are themselves JSONArray objects and each has 4 fields + for (JsonElement row : json) { + try { + if (row instanceof JsonArray array && array.size() == 4) { + int min = array.get(0).getAsInt(); + int max = array.get(1).getAsInt(); + String value = array.get(2).getAsString(); + String image = array.get(3).getAsString(); + if (min >= 1 && max >= min && !value.isEmpty()) { + // Image is allowed to be empty, but if given, it must be an asset ID + MD5Key imageID = null; + if (!image.isEmpty()) { + imageID = FunctionUtil.getAssetKeyFromString(image); + } + // All checks complete, add this row to the table + lookupTable.addEntry(min, max, value, imageID); + counter++; + } + } + } catch (UnsupportedOperationException|NumberFormatException|IllegalStateException e) { + // Make a list of all failing rows to return to the user. + errorRows.add(rowindex); + } + rowindex++; + } + MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); + return counter == rowindex ? new JsonPrimitive(counter) : errorRows; + } + // FIXME Report error in standardized format + throw new ParserException("Second parameter must be a JSON Array"); + } + private String setTableRoll(String function, List params) throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("setTableRoll", params, 2, 2); diff --git a/src/main/java/net/rptools/maptool/model/LookupTable.java b/src/main/java/net/rptools/maptool/model/LookupTable.java index 4ed696ac9b..6e5d4c14b1 100644 --- a/src/main/java/net/rptools/maptool/model/LookupTable.java +++ b/src/main/java/net/rptools/maptool/model/LookupTable.java @@ -15,10 +15,6 @@ package net.rptools.maptool.model; import com.google.protobuf.StringValue; -import java.util.*; -import java.util.stream.Collectors; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; import net.rptools.dicelib.expression.ExpressionParser; import net.rptools.dicelib.expression.Result; import net.rptools.lib.MD5Key; @@ -26,21 +22,76 @@ import net.rptools.maptool.server.proto.LookupEntryDto; import net.rptools.maptool.server.proto.LookupTableDto; import net.rptools.parser.ParserException; +import org.jetbrains.annotations.Contract; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.util.*; +import java.util.stream.Collectors; +/** + * LookupTable represents a table of die roll ranges and a String value (with an optional + * asset ID). + *

+ * The general idea is that random picks from a list are often a die roll, such as "2d6", + * that is then used to perform a lookup on a table. The typical random encounter table, + * for example, might look something like this: + * + * + * + * + * + * + * + * + * + *
Die range (2d6)ValueTable image
24Orcs, group of 1d4null
56Goblins, group of 2d4+2null
7Hobgoblins, family unit of 3null
89Bandits, brother/sister teamnull
1012City guard, squad of 4null
+ *

+ * If the Die range contains only a single value, then only that value will select that row.
+ * The default roll is the "2d6" shown in the table header (column one).
+ * The Value column is the String returned for a given lookup.
+ * The Table image column contains either null or an {@link MD5Key} for an asset. + *

+ */ public class LookupTable { - private static ExpressionParser expressionParser = new ExpressionParser(); + private static final ExpressionParser expressionParser = new ExpressionParser(); + /** + * The complete list of entries in the table. + * (Future implementation changes will likely be sorted so that binary searches + * can find entries more quickly.) + */ private @Nonnull List entryList = new ArrayList<>(); private @Nullable String name; + /** + * The die expression to use when one isn't provided when retrieving values. + *

+ * If no value is set, the default implementation will choose a row randomly, + * ignoring the min/max values (so each row has an equal chance of being selected). + */ private @Nullable String defaultRoll; private @Nullable MD5Key tableImage; + /** + * Whether the table is visible for players in the Table panel. + */ private @Nonnull Boolean visible = true; + /** + * Whether players can read elements from the table. + */ 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(). + /** + * Flags a table as "Pick Once", i.e. each entry can only be chosen once + * before the table must be reset(). + */ private @Nonnull Boolean pickOnce = false; + /** + * Unique string returned when all picks from a Pick Once table have + * been selected. + *

+ * DO NOT use this string as a table value! 😁 + */ public static final String NO_PICKS_LEFT = "NO_PICKS_LEFT"; public LookupTable() {} @@ -55,31 +106,70 @@ public LookupTable(LookupTable table) { entryList.addAll(table.entryList); } + /** + * Sets the defaultRoll field that is used to choose a random element + * from the table when no die roll expression is provided. See {@link #getLookup()}. + * + * @param roll String expression representing default roll + */ public void setRoll(String roll) { defaultRoll = roll; } + /** + * Removes all entries from the table, but does not reset the default roll + * or other fields. (Note that for Pick Once tables, the pick status is + * part of each entry, so those are cleared by this method as well.) + */ public void clearEntries() { entryList.clear(); } + /** + * Adds a single row to the table using the specified parameters. + * + * @param min lower bound of the die roll + * @param max upper bound of the die roll + * @param result a non-null String to store into the table + * @param imageId the asset ID (may be null) + */ public void addEntry(int min, int max, String result, MD5Key imageId) { entryList.add(new LookupEntry(min, max, result, imageId)); } + /** + * Calls {@link #getLookup(String)} and passes it a null parameter. + * + * @return the result of calling {@link #getLookup(String)} + * @throws ParserException thrown when the default roll expression isn't valid + */ public LookupEntry getLookup() throws ParserException { return getLookup(null); } - public String getRoll() { - return getDefaultRoll(); - } +// public String getRoll() { +// return getDefaultRoll(); +// } - public void setName(String name) { + /** + * Set the table name. + *

+ * This method should not be called after the table is added to + * the campaign properties, as the Map<> therein relies on the + * name for the hash code. + * + * @param name name of the table + */ + public void setName(@org.jetbrains.annotations.Nullable String name) { this.name = name; } - public String getName() { + /** + * Retrieves the name of this table. + * + * @return name of the table + */ + public @org.jetbrains.annotations.Nullable String getName() { return name; } @@ -131,6 +221,14 @@ public LookupEntry getLookupDirect(String roll) throws ParserException { return entry; } + /** + * Evaluates the roll expression and returns the appropriate table entry. + * The {@link #getLookup(String)} method delegates here for non-Pick Once tables. + * + * @param roll dice expression + * @return selected table entry; returns null if there's no matching range + * @throws ParserException thrown if the dice expression is invalid + */ private LookupEntry getStandardLookup(String roll) throws ParserException { int tableResult = 0; LookupEntry retEntry = null; @@ -154,6 +252,18 @@ private LookupEntry getStandardLookup(String roll) throws ParserException { return retEntry; } + /** + * Returns an entry from a Pick Once table that hasn't already been selected. + * The {@link #getLookup(String)} method delegates here for Pick Once tables. + *

+ * For this method, the roll parameter must be a string that + * can be converted to integer via {@link Integer#parseInt(String)} -- a + * dice expression is not allowed. + * + * @param roll String representing which entry to retrieve + * @return the table entry corresponding to the roll parameter + * @throws ParserException thrown if roll cannot be parsed as an integer + */ private LookupEntry getPickOnceLookup(String roll) throws ParserException { try { int entryNum = Integer.parseInt(roll); @@ -161,7 +271,7 @@ private LookupEntry getPickOnceLookup(String roll) throws ParserException { if (entryNum < entryList.size()) { LookupEntry entry = entryList.get(entryNum); entry.setPicked(true); - entryList.set(entryNum, entry); + entryList.set(entryNum, entry); // TODO Isn't this redundant?? return entry; } else { @@ -169,10 +279,22 @@ private LookupEntry getPickOnceLookup(String roll) throws ParserException { } } catch (NumberFormatException nfe) { - throw new ParserException("Expected integer value for pick once table: " + roll); + throw new ParserException("Expected integer value for Pick Once table: " + roll); } } + /** + * Constrains the parameter to be within the range of values depicted by the table entries. + * Values less than the lowest minimum entry are set the minimum, while values larger than + * the largest maximum entry are set to the maximum. + *

+ * There is no check to determine whether val actually identifies a specific + * table entry per the min/max values, so the return value (if unmodified) may not + * represent an actual table entry. + * + * @param val value to be constrained + * @return the constrained value + */ private int constrainRoll(int val) { int minmin = Integer.MAX_VALUE; int maxmax = Integer.MIN_VALUE; @@ -194,33 +316,39 @@ private int constrainRoll(int val) { return val; } - private String getDefaultRoll() { + /** + * Has two modes of operation. + *

    + *
  • For standard tables, it retrieves the {@link #defaultRoll} + * if it has been set, or calculates a dice expression to use and returns that. + *
  • For Pick Once tables, it ignores the {@link #defaultRoll} + * and instead rolls a random number which indexes into a subset of the table made up only + * of entries which have not been previously selected. The value of that + * entry is returned. + *
+ * + * @return the value field from the table for the associated entry; Pick Once + * tables also have their picked flag set so that they are not picked again + */ + public String getDefaultRoll() { if (getPickOnce()) { // For Pick Once tables this returns a random pick from those entries in the list that // have not been picked. - List le = entryList; - LookupEntry entry; - int len = le.size(); - List unpicked = new ArrayList(); - for (int i = 0; i < len; i++) { - entry = le.get(i); - if (!entry.picked) { - unpicked.add(i); - } - } - if (unpicked.isEmpty()) { - return (NO_PICKS_LEFT); - } + LookupEntry[] unpicked = entryList.stream().filter(e -> !e.picked).toArray(LookupEntry[]::new); try { - Result result = expressionParser.evaluate("d" + unpicked.size()); - int index = Integer.parseInt(result.getValue().toString()) - 1; - return unpicked.get(index).toString(); + if (unpicked.length != 0) { + Result result = expressionParser.evaluate("d" + unpicked.length); + int index = Integer.parseInt(result.getValue().toString()) - 1; + unpicked[index].picked = true; + return unpicked[index].getValue(); + } } catch (ParserException e) { - MapTool.showError("Error getting default roll for Pick Once table ", e); - return (NO_PICKS_LEFT); + MapTool.showError("Error getting default roll for Pick Once table: " + name, e); } + return (NO_PICKS_LEFT); } else { - if (defaultRoll != null && defaultRoll.length() > 0) { + // If the defaultRoll hasn't been set or is an empty String, return it. + if (defaultRoll != null && !defaultRoll.isEmpty()) { return defaultRoll; } @@ -241,15 +369,21 @@ private String getDefaultRoll() { } } - /** Sets the picked flag on each table entry to false. */ + /** + * Sets the picked flag on each table entry to false. + *

+ * This method returns immediately if the table is not a Pick Once table. + */ public void reset() { - List curList = entryList; - List newList = new ArrayList<>(); - for (LookupEntry entry : curList) { - entry.setPicked(false); - newList.add(entry); + if (pickOnce) { + List curList = entryList; + List newList = new ArrayList<>(); + for (LookupEntry entry : curList) { + entry.setPicked(false); + newList.add(entry); + } + entryList = newList; } - entryList = newList; } /** @@ -281,7 +415,7 @@ public void reset(List entriesToReset) { /** * Get a List of the LookupEntrys for this table. * - * @return List of LookupEntrys + * @return unmodifiable list of LookupEntrys */ public List getEntryList() { return Collections.unmodifiableList(entryList); @@ -292,7 +426,7 @@ public List getEntryList() { * * @return MD5Key */ - public MD5Key getTableImage() { + public @org.jetbrains.annotations.Nullable MD5Key getTableImage() { return tableImage; } @@ -301,29 +435,24 @@ public MD5Key getTableImage() { * * @param tableImage The MD5Key (Asset ID) for the image. */ - public void setTableImage(MD5Key tableImage) { + public void setTableImage(@org.jetbrains.annotations.Nullable MD5Key tableImage) { this.tableImage = tableImage; } /** * Gets whether a table is flagged as Pick Once or not. * - * @return Boolean - true if table is Pick Once + * @return true if table is Pick Once */ public boolean getPickOnce() { - // Older tables won't have it set. - if (pickOnce == null) { - pickOnce = false; - } - return pickOnce; } /** - * Set whether a table as Pick Once (true/false). Automatically resets the pick once status of + * Set whether a table is Pick Once (true/false). Automatically resets the pick once status of * entries. * - * @param pickOnce - Boolean + * @param pickOnce true if the table should be treated as Pick Once */ public void setPickOnce(boolean pickOnce) { this.pickOnce = pickOnce; @@ -366,26 +495,30 @@ public String toString() { public static class LookupEntry { - private int min; - private int max; + private final int min; + private final int max; // For Pick Once tables each entry is flagged as picked (true) or not (false). private @Nonnull Boolean picked = false; private @Nullable String value; - private @Nullable MD5Key imageId; + private final @Nullable MD5Key imageId; /** * @deprecated here to prevent xstream from breaking b24-b25 */ + @SuppressWarnings("DeprecatedIsStillUsed") @Deprecated private @Nullable String result; - public LookupEntry(int min, int max, String value, MD5Key imageId) { + public LookupEntry(int min, int max, + @org.jetbrains.annotations.Nullable String value, + @org.jetbrains.annotations.Nullable MD5Key imageId) { this.min = min; this.max = max; this.value = value; this.imageId = imageId; } - @SuppressWarnings("ConstantValue") + @Contract(value = " -> this") + @SuppressWarnings({"ConstantValue", "unused"}) private Object readResolve() { if (picked == null) { picked = false; @@ -399,7 +532,7 @@ private Object readResolve() { return this; } - public MD5Key getImageId() { + public @org.jetbrains.annotations.Nullable MD5Key getImageId() { return imageId; } @@ -419,7 +552,7 @@ public int getMin() { return min; } - public String getValue() { + public @org.jetbrains.annotations.Nullable String getValue() { return value; } @@ -466,8 +599,8 @@ public Set getAllAssetIds() { /** * Retrieves the visible flag for the LookupTable. * - * @return Boolean -- True indicates that the table will be visible to players. False indicates - * that the table will be hidden from players. + * @return true indicates that the table will be visible to players. + * Otherwise, the table will be hidden from players. */ public boolean getVisible() { return visible; @@ -476,8 +609,8 @@ public boolean getVisible() { /** * Sets the visible flag for the LookupTable. * - * @param value(Boolean) -- True specifies that the table will be visible to players. False - * indicates that the table will be hidden from players. + * @param value true specifies that the table will be visible to players. + * Otherwise, the table will be hidden from players. */ public void setVisible(boolean value) { visible = value; @@ -486,9 +619,9 @@ public void setVisible(boolean value) { /** * Retrieves the allowLookup flag for the LookupTable. * - * @return Boolean -- True indicates that players can call for values from this table. False - * indicates that players will be prevented from calling values from this table. GM's can - * ALWAYS perform lookups against a table. + * @return true indicates that players can call for values from this table. + * Otherwise, players will be prevented from calling values from this table. + * GM's can ALWAYS perform lookups against a table. */ public boolean getAllowLookup() { return allowLookup; @@ -497,15 +630,16 @@ public boolean getAllowLookup() { /** * Sets the allowLookup flag for the LookupTable. * - * @param value(Boolean) -- True indicates that players can call for values from this table. False - * indicates that players will be prevented from calling values from this table. GM's can - * ALWAYS perform lookups against a table. + * @param value true indicates that players can call for values from this table. + * Otherwise, players will be prevented from calling values from this table. + * GM's can ALWAYS perform lookups against a table. */ public void setAllowLookup(boolean value) { allowLookup = value; } - @SuppressWarnings("ConstantValue") + @Contract(value = " -> this") + @SuppressWarnings({"ConstantValue", "unused"}) private Object readResolve() { if (visible == null) { visible = true; @@ -526,7 +660,7 @@ public static LookupTable fromDto(LookupTableDto dto) { var table = new LookupTable(); table.name = dto.hasName() ? dto.getName().getValue() : null; table.entryList = - dto.getEntriesList().stream().map(e -> LookupEntry.fromDto(e)).collect(Collectors.toList()); + dto.getEntriesList().stream().map(LookupEntry::fromDto).collect(Collectors.toList()); table.defaultRoll = dto.hasDefaultRoll() ? dto.getDefaultRoll().getValue() : null; table.tableImage = dto.hasTableImage() ? new MD5Key(dto.getTableImage().getValue()) : null; table.setVisible(dto.getVisible()); @@ -537,7 +671,7 @@ 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.addAllEntries(entryList.stream().map(LookupEntry::toDto).collect(Collectors.toList())); if (name != null) { dto.setName(StringValue.of(name)); } diff --git a/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java b/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java new file mode 100644 index 0000000000..56cba2a182 --- /dev/null +++ b/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java @@ -0,0 +1,101 @@ +/* + * This software Copyright by the RPTools.net development team, and + * licensed under the Affero GPL Version 3 or, at your option, any later + * version. + * + * MapTool Source Code is distributed in the hope that it will be + * useful, but WITHOUT ANY WARRANTY; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * + * You should have received a copy of the GNU Affero General Public + * License * along with this source Code. If not, please visit + * and specifically the Affero license + * text at . + */ +package net.rptools.maptool.client.functions; + +import net.rptools.lib.MD5Key; +import net.rptools.maptool.model.LookupTable; +import net.rptools.parser.ParserException; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class LookupTableTests { + private static final LookupTable table = new LookupTable(); + + private void buildTestTable() { + table.clearEntries(); + table.setName("test"); + table.setRoll("1d6"); + table.setPickOnce(false); + table.setVisible(false); + table.setAllowLookup(true); + } + + /** + * Doesn't register with the campaign's table map, so no unregistering + * is necessary. + */ + @Test + void table_creation() { + // Kind of silly to check all these, but there could be + // new logic added in the future that does processing in + // the various get/set methods, so... + buildTestTable(); + // Must start empty + assertTrue(table.getEntryList().isEmpty()); + // Check that individual fields can be set and retrieved + assertEquals("test", table.getName()); + assertEquals("1d6", table.getDefaultRoll()); + MD5Key md5 = new MD5Key("1234"); + table.setTableImage(md5); + assertEquals(md5, table.getTableImage()); + assertFalse(table.getPickOnce()); + assertFalse(table.getVisible()); + assertTrue(table.getAllowLookup()); + } + + @Test + void table_addOneEntry() throws ParserException { + buildTestTable(); + // This entry must be the first one + table.addEntry(10, 20, "result", null); + + // Check a roll that's in the middle of the range. + LookupTable.LookupEntry le = table.getLookup("15"); + assertEquals(10, le.getMin()); + assertEquals(20, le.getMax()); + assertEquals("result", le.getValue()); + assertNull(le.getImageId()); + } + + private void populateTable() { + buildTestTable(); + table.addEntry(10, 20, "result 1", null); + table.addEntry(25, 30, "result 2", null); + table.addEntry(40, 50, "result 3", null); + } + + @Test + void table_countEntries() throws ParserException { + populateTable(); + + assertEquals(3, table.getEntryList().size()); + } + + @Test + void table_addMultipleEntries() throws ParserException { + populateTable(); + + LookupTable.LookupEntry le = table.getLookup("25"); + assertEquals(25, le.getMin()); + assertEquals(30, le.getMax()); + assertEquals("result 2", le.getValue()); + assertNull(le.getImageId()); + + // Make sure both lookups return the same table entry + LookupTable.LookupEntry le2 = table.getLookup("30"); + assertEquals(le, le2); + } +} diff --git a/src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java b/src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java new file mode 100644 index 0000000000..475a5db885 --- /dev/null +++ b/src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java @@ -0,0 +1,124 @@ +/* + * This software Copyright by the RPTools.net development team, and + * licensed under the Affero GPL Version 3 or, at your option, any later + * version. + * + * MapTool Source Code is distributed in the hope that it will be + * useful, but WITHOUT ANY WARRANTY; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * + * You should have received a copy of the GNU Affero General Public + * License * along with this source Code. If not, please visit + * and specifically the Affero license + * text at . + */ +package net.rptools.maptool.client.functions; + +import net.rptools.lib.MD5Key; +import net.rptools.maptool.model.LookupTable; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests the following table operations: + *

    + *
  1. tbl, table
    2. + *
  2. tblImage, tableImage
    3. + *
  3. addTableEntry
    4. + *
  4. clearTable
    5. + *
  5. copyTable
    6. + *
  6. createTable
    7. + *
  7. deleteTable
    8. + *
  8. deleteTableEntry
    9. + *
  9. getTableAccess
    10. + *
  10. getTableEntry
    11. + *
  11. getTableImage
    12. + *
  12. getTableNames
    13. + *
  13. getTablePickOnce
    14. + *
  14. getTablePicksLeft
    15. + *
  15. getTableRoll
    16. + *
  16. getTableVisible
    17. + *
  17. loadTable
    18. + *
  18. resetTablePicks
    19. + *
  19. setTableAccess
    20. + *
  20. setTableEntry
    21. + *
  21. setTableImage
    22. + *
  22. setTablePickOnce
    23. + *
  23. setTableRoll
    24. + *
  24. setTableVisible
    25. + *
+ */ +class TableFunctionsTest { + private static final LookupTable table = new LookupTable(); + + private LookupTable populateTestTable() { + table.clearEntries(); + table.setName("test"); + table.setRoll("1d6"); + table.setPickOnce(false); + table.setVisible(false); + table.setAllowLookup(true); + return table; + } + + @Test + void table_creation() { + // Kind of silly to check all these, but there could be + // new logic added in the future that does processing in + // the various get/set methods, so... + LookupTable t = new LookupTable(); + // Must start empty + assertTrue(t.getEntryList().isEmpty()); + // Check that individual fields can be set and retrieved + t.setName("test"); + assertEquals("test", t.getName()); + t.setRoll("1d6"); + assertEquals("1d6", t.getRoll()); + MD5Key md5 = new MD5Key("1234"); + t.setTableImage(md5); + assertEquals(md5, t.getTableImage()); + t.setPickOnce(true); + assertTrue(t.getPickOnce()); + t.setVisible(true); + assertTrue(t.getVisible()); + t.setAllowLookup(true); + assertTrue(t.getAllowLookup()); + } + + @Test + void table_tbl() { + LookupTable t = populateTestTable(); + LookupTableFunction.getInstance(). + } + + @Test + void parse_twoPropsFirstWithSpace() { + String testProps = "a 1=1; nospace=2"; + + StrPropFunctions.parse(testProps, map, oldKeys, oldKeysNormalized, DEFAULT_DELIMITER); + + inMap(map).key("A 1").hasValue("1"); + inMap(map).key("NOSPACE").hasValue("2"); + } + + @Test + void parse_twoPropsLastWithSpace() { + String testProps = "nospace=2; a 1=1"; + + StrPropFunctions.parse(testProps, map, oldKeys, oldKeysNormalized, DEFAULT_DELIMITER); + + inMap(map).key("A 1").hasValue("1"); + inMap(map).key("NOSPACE").hasValue("2"); + } + + @Test + void parse_onePropWithTwoSpaces() { + String testProps = "a b 1=1"; + + StrPropFunctions.parse(testProps, map, oldKeys, oldKeysNormalized, DEFAULT_DELIMITER); + + inMap(map).key("A B 1").hasValue("1"); + } +} From 391f073e85a493ef9fa363dfa33c18eb11b664b4 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Tue, 14 May 2024 19:27:48 -0400 Subject: [PATCH 04/11] Continue with updating LookupTable Finished adding tests of the model. I'm not thrilled with the ExpressionParser class being used directly by LookupTable, but changing it to use DI is beyond the scope of this issue. When the LookupTable changes its implementation to a sorted array, the dependency can be changed (perhaps). Should the MTscript functions have tests as well? Or should those be done via a macro within the tool? It would certainly be easier to automate if they were in Java, but putting a bunch of testing macros into a campaign would (potentially) allow others to build tests as well and contribute to the project. (TODO) With the use of `macro.catch`, maybe exceptions thrown within MTscript can actually be caught and handled in a sufficient way? --- .../client/functions/LookupTableFunction.java | 4 +- .../ui/lookuptable/EditLookupTablePanel.java | 32 ++--- .../rptools/maptool/model/LookupTable.java | 1 - .../client/functions/LookupTableTests.java | 2 +- .../client/functions/TableFunctionsTest.java | 124 ------------------ 5 files changed, 15 insertions(+), 148 deletions(-) delete mode 100644 src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java diff --git a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java index ea1d7af64b..1fde7bf809 100644 --- a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java @@ -526,14 +526,14 @@ private String setTableRoll(String function, List params) throws ParserE LookupTable lookupTable = checkTableAccess(name, function); lookupTable.setRoll(roll); MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); - return lookupTable.getRoll(); + return lookupTable.getDefaultRoll(); } private String getTableRoll(String function, List params) throws ParserException { FunctionUtil.checkNumberParam("getTableRoll", params, 1, 1); String name = params.getFirst().toString(); LookupTable lookupTable = checkTableAccess(name, function); - return lookupTable.getRoll(); + return lookupTable.getDefaultRoll(); } private @NotNull BigDecimal setTableAccess(String function, List params) throws ParserException { diff --git a/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java b/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java index 71fe7b9b17..1daddd87e0 100644 --- a/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java +++ b/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java @@ -14,25 +14,6 @@ */ package net.rptools.maptool.client.ui.lookuptable; -import java.awt.Color; -import java.awt.Component; -import java.awt.Dimension; -import java.awt.EventQueue; -import java.awt.event.MouseAdapter; -import java.awt.event.MouseEvent; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import javax.swing.BorderFactory; -import javax.swing.JButton; -import javax.swing.JCheckBox; -import javax.swing.JList; -import javax.swing.JTable; -import javax.swing.JTextField; -import javax.swing.SwingUtilities; -import javax.swing.table.AbstractTableModel; -import javax.swing.table.TableCellRenderer; -import javax.swing.table.TableModel; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.MapToolUtil; @@ -45,6 +26,17 @@ import net.rptools.maptool.model.LookupTable; import net.rptools.maptool.model.LookupTable.LookupEntry; +import javax.swing.*; +import javax.swing.table.AbstractTableModel; +import javax.swing.table.TableCellRenderer; +import javax.swing.table.TableModel; +import java.awt.*; +import java.awt.event.MouseAdapter; +import java.awt.event.MouseEvent; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + public class EditLookupTablePanel extends AbeillePanel { private static final long serialVersionUID = 2341539768448195059L; @@ -126,7 +118,7 @@ public void attach(LookupTable luTable) { accepted = false; getTableNameTextField().setText(this.lookupTable.getName()); - getTableRollTextField().setText(this.lookupTable.getRoll()); + getTableRollTextField().setText(this.lookupTable.getDefaultRoll()); tableImageAssetPanel.setImageId(this.lookupTable.getTableImage()); getVisibleCheckbox().setSelected(this.lookupTable.getVisible()); getAllowLookupCheckbox().setSelected(this.lookupTable.getAllowLookup()); diff --git a/src/main/java/net/rptools/maptool/model/LookupTable.java b/src/main/java/net/rptools/maptool/model/LookupTable.java index 6e5d4c14b1..d4504e1176 100644 --- a/src/main/java/net/rptools/maptool/model/LookupTable.java +++ b/src/main/java/net/rptools/maptool/model/LookupTable.java @@ -272,7 +272,6 @@ private LookupEntry getPickOnceLookup(String roll) throws ParserException { LookupEntry entry = entryList.get(entryNum); entry.setPicked(true); entryList.set(entryNum, entry); // TODO Isn't this redundant?? - return entry; } else { return new LookupEntry(0, 0, NO_PICKS_LEFT, null); diff --git a/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java b/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java index 56cba2a182..6ac4081f67 100644 --- a/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java +++ b/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java @@ -78,7 +78,7 @@ private void populateTable() { } @Test - void table_countEntries() throws ParserException { + void table_countEntries() { populateTable(); assertEquals(3, table.getEntryList().size()); diff --git a/src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java b/src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java deleted file mode 100644 index 475a5db885..0000000000 --- a/src/test/java/net/rptools/maptool/client/functions/TableFunctionsTest.java +++ /dev/null @@ -1,124 +0,0 @@ -/* - * This software Copyright by the RPTools.net development team, and - * licensed under the Affero GPL Version 3 or, at your option, any later - * version. - * - * MapTool Source Code is distributed in the hope that it will be - * useful, but WITHOUT ANY WARRANTY; without even the implied warranty - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * - * You should have received a copy of the GNU Affero General Public - * License * along with this source Code. If not, please visit - * and specifically the Affero license - * text at . - */ -package net.rptools.maptool.client.functions; - -import net.rptools.lib.MD5Key; -import net.rptools.maptool.model.LookupTable; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - -/** - * Tests the following table operations: - *
    - *
  1. tbl, table
    2. - *
  2. tblImage, tableImage
    3. - *
  3. addTableEntry
    4. - *
  4. clearTable
    5. - *
  5. copyTable
    6. - *
  6. createTable
    7. - *
  7. deleteTable
    8. - *
  8. deleteTableEntry
    9. - *
  9. getTableAccess
    10. - *
  10. getTableEntry
    11. - *
  11. getTableImage
    12. - *
  12. getTableNames
    13. - *
  13. getTablePickOnce
    14. - *
  14. getTablePicksLeft
    15. - *
  15. getTableRoll
    16. - *
  16. getTableVisible
    17. - *
  17. loadTable
    18. - *
  18. resetTablePicks
    19. - *
  19. setTableAccess
    20. - *
  20. setTableEntry
    21. - *
  21. setTableImage
    22. - *
  22. setTablePickOnce
    23. - *
  23. setTableRoll
    24. - *
  24. setTableVisible
    25. - *
- */ -class TableFunctionsTest { - private static final LookupTable table = new LookupTable(); - - private LookupTable populateTestTable() { - table.clearEntries(); - table.setName("test"); - table.setRoll("1d6"); - table.setPickOnce(false); - table.setVisible(false); - table.setAllowLookup(true); - return table; - } - - @Test - void table_creation() { - // Kind of silly to check all these, but there could be - // new logic added in the future that does processing in - // the various get/set methods, so... - LookupTable t = new LookupTable(); - // Must start empty - assertTrue(t.getEntryList().isEmpty()); - // Check that individual fields can be set and retrieved - t.setName("test"); - assertEquals("test", t.getName()); - t.setRoll("1d6"); - assertEquals("1d6", t.getRoll()); - MD5Key md5 = new MD5Key("1234"); - t.setTableImage(md5); - assertEquals(md5, t.getTableImage()); - t.setPickOnce(true); - assertTrue(t.getPickOnce()); - t.setVisible(true); - assertTrue(t.getVisible()); - t.setAllowLookup(true); - assertTrue(t.getAllowLookup()); - } - - @Test - void table_tbl() { - LookupTable t = populateTestTable(); - LookupTableFunction.getInstance(). - } - - @Test - void parse_twoPropsFirstWithSpace() { - String testProps = "a 1=1; nospace=2"; - - StrPropFunctions.parse(testProps, map, oldKeys, oldKeysNormalized, DEFAULT_DELIMITER); - - inMap(map).key("A 1").hasValue("1"); - inMap(map).key("NOSPACE").hasValue("2"); - } - - @Test - void parse_twoPropsLastWithSpace() { - String testProps = "nospace=2; a 1=1"; - - StrPropFunctions.parse(testProps, map, oldKeys, oldKeysNormalized, DEFAULT_DELIMITER); - - inMap(map).key("A 1").hasValue("1"); - inMap(map).key("NOSPACE").hasValue("2"); - } - - @Test - void parse_onePropWithTwoSpaces() { - String testProps = "a b 1=1"; - - StrPropFunctions.parse(testProps, map, oldKeys, oldKeysNormalized, DEFAULT_DELIMITER); - - inMap(map).key("A B 1").hasValue("1"); - } -} From 967afa759269a4cdd43eed1352ee6cd30e2f58f4 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Sun, 26 May 2024 15:53:27 -0400 Subject: [PATCH 05/11] Removed redundant call --- src/main/java/net/rptools/maptool/model/LookupTable.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/net/rptools/maptool/model/LookupTable.java b/src/main/java/net/rptools/maptool/model/LookupTable.java index d4504e1176..6e9f0a3396 100644 --- a/src/main/java/net/rptools/maptool/model/LookupTable.java +++ b/src/main/java/net/rptools/maptool/model/LookupTable.java @@ -271,7 +271,6 @@ private LookupEntry getPickOnceLookup(String roll) throws ParserException { if (entryNum < entryList.size()) { LookupEntry entry = entryList.get(entryNum); entry.setPicked(true); - entryList.set(entryNum, entry); // TODO Isn't this redundant?? return entry; } else { return new LookupEntry(0, 0, NO_PICKS_LEFT, null); From 7b74dc441f7974152b45021c5ada6a7cab19324b Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Sun, 26 May 2024 17:19:25 -0400 Subject: [PATCH 06/11] Fixed parameter order for checkTableAccess() private method --- .../rptools/maptool/client/functions/LookupTableFunction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java index 1fde7bf809..14c3d9574b 100644 --- a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java @@ -198,7 +198,7 @@ public Object childEvaluate( * @return table identified by name * @throws ParserException Thrown for any access errors to the named table (unknown, access prohibited, or table doesn't exist) */ - private LookupTable checkTableAccess(String function, String name) throws ParserException { + private LookupTable checkTableAccess(String name, String function) throws ParserException { LookupTable lookupTable = MapTool.getCampaign().getLookupTableMap().get(name); if (!MapTool.getPlayer().isGM() && !lookupTable.getAllowLookup()) { if (lookupTable.getVisible()) { From a78d49fcf897a6144bdba6139d02c551b181b55a Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Sun, 26 May 2024 17:24:12 -0400 Subject: [PATCH 07/11] Grammatical changes suggested by IntelliJ and removal of a redundant .toString() --- .../client/functions/LookupTableFunction.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java index 14c3d9574b..610ae91876 100644 --- a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java @@ -244,13 +244,18 @@ private int getTablePicksLeft(String function, List params) throws Parse } /** - * Resets the entries on a table in regards to whether such entries have been + * Resets the entries on a table with regard to whether such entries have been * selected already. This is used in the PickOnce family of tables. - * - * resetTablePicks(tblName) - reset all entries on a table - * resetTablePicks(tblName, entriesToReset) - reset specific entries from a String List with "," delim - * resetTablePicks(tblName, entriesToReset, delim) - use custom delimiter - * resetTablePicks(tblName, entriesToReset, "json") - entriesToReset is a JsonArray + *
    + *
  • + * {@code resetTablePicks(tblName)} - reset all entries on a table + *
  • + * {@code resetTablePicks(tblName, entriesToReset)} - reset specific entries from a String List with "," delim + *
  • + * {@code resetTablePicks(tblName, entriesToReset, delim)} - use custom delimiter + *
  • + * {@code resetTablePicks(tblName, entriesToReset, "json")} - entriesToReset is a JsonArray + *
* * @param function name of the MTscript function (used for error messages) * @param params list of function parameters @@ -301,7 +306,7 @@ private int getTablePicksLeft(String function, List params) throws Parse MD5Key imageId = entry.getImageId(); if (imageId != null) { - entryDetails.addProperty("assetid", "asset://" + imageId.toString()); + entryDetails.addProperty("assetid", "asset://" + imageId); } else { entryDetails.addProperty("assetid", ""); } @@ -377,7 +382,7 @@ private int setTableEntry(String function, List params) throws ParserExc checkTrusted(function); FunctionUtil.checkNumberParam("deleteTable", params, 1, 1); String name = params.getFirst().toString(); - LookupTable lookupTable = checkTableAccess(name, function); + checkTableAccess(name, function); MapTool.getCampaign().getLookupTableMap().remove(name); MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); return ""; @@ -514,7 +519,7 @@ private int setTableEntry(String function, List params) throws ParserExc MapTool.serverCommand().updateCampaign(MapTool.getCampaign().getCampaignProperties()); return counter == rowindex ? new JsonPrimitive(counter) : errorRows; } - // FIXME Report error in standardized format + // FIXME Report error using I18N throw new ParserException("Second parameter must be a JSON Array"); } From 5eb4b757d6b4d40c5093c6b37cee21b8c5f2f4b6 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Sun, 26 May 2024 17:40:40 -0400 Subject: [PATCH 08/11] Ran spotlessApply --- .../client/functions/LookupTableFunction.java | 130 +++++++------ .../client/functions/getInfoFunction.java | 19 +- .../ui/lookuptable/EditLookupTablePanel.java | 21 +-- .../rptools/maptool/model/LookupTable.java | 174 +++++++++--------- .../rptools/maptool/util/FunctionUtil.java | 11 +- .../client/functions/LookupTableTests.java | 133 +++++++------ 6 files changed, 249 insertions(+), 239 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java index 610ae91876..2175ab53e3 100644 --- a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java @@ -18,6 +18,11 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; +import java.io.Serializable; +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.functions.json.JSONMacroFunctions; @@ -33,12 +38,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.io.Serializable; -import java.math.BigDecimal; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; - public class LookupTableFunction extends AbstractFunction { public LookupTableFunction() { @@ -53,7 +52,7 @@ public LookupTableFunction() { "getTableRoll", "setTableRoll", "clearTable", - "loadTable", // bulk table import + "loadTable", // bulk table import "addTableEntry", "deleteTableEntry", "createTable", @@ -139,7 +138,8 @@ public Object childEvaluate( } } - private @Nullable Serializable tbl_and_table(String function, List params) throws ParserException { + private @Nullable Serializable tbl_and_table(String function, List params) + throws ParserException { FunctionUtil.checkNumberParam(function, params, 1, 3); String name = params.get(0).toString(); @@ -166,7 +166,9 @@ public Object childEvaluate( } catch (NumberFormatException nfe) { return val; } - } else if (function.equalsIgnoreCase("tableImage") || function.equalsIgnoreCase("tblImage")) { // We want the image URI through tblImage or tableImage + } else if (function.equalsIgnoreCase("tableImage") + || function.equalsIgnoreCase( + "tblImage")) { // We want the image URI through tblImage or tableImage if (result.getImageId() == null) { return ""; } @@ -191,12 +193,14 @@ public Object childEvaluate( } /** - * Returns the lookupTable object identified by name if - * it exists in the campaign. Otherwise, throws ParserException. + * Returns the lookupTable object identified by name if it exists in the campaign. + * Otherwise, throws ParserException. + * * @param function name of MTscript function (for error message) * @param name name of table * @return table identified by name - * @throws ParserException Thrown for any access errors to the named table (unknown, access prohibited, or table doesn't exist) + * @throws ParserException Thrown for any access errors to the named table (unknown, access + * prohibited, or table doesn't exist) */ private LookupTable checkTableAccess(String name, String function) throws ParserException { LookupTable lookupTable = MapTool.getCampaign().getLookupTableMap().get(name); @@ -224,7 +228,8 @@ private int getTablePicksLeft(String function, List params) throws Parse return lookupTable.getPicksLeft(); } - private @NotNull BigDecimal getTablePickOnce(String function, List params) throws ParserException { + private @NotNull BigDecimal getTablePickOnce(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("getTablePickOnce", params, 1, 1); String name = params.getFirst().toString(); @@ -232,7 +237,8 @@ private int getTablePicksLeft(String function, List params) throws Parse return lookupTable.getPickOnce() ? BigDecimal.ONE : BigDecimal.ZERO; } - private @NotNull BigDecimal setTablePickOnce(String function, List params) throws ParserException { + private @NotNull BigDecimal setTablePickOnce(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("setTablePickOnce", params, 2, 2); String name = params.get(0).toString(); @@ -244,24 +250,23 @@ private int getTablePicksLeft(String function, List params) throws Parse } /** - * Resets the entries on a table with regard to whether such entries have been - * selected already. This is used in the PickOnce family of tables. + * Resets the entries on a table with regard to whether such entries have been selected already. + * This is used in the PickOnce family of tables. + * *
    - *
  • - * {@code resetTablePicks(tblName)} - reset all entries on a table - *
  • - * {@code resetTablePicks(tblName, entriesToReset)} - reset specific entries from a String List with "," delim - *
  • - * {@code resetTablePicks(tblName, entriesToReset, delim)} - use custom delimiter - *
  • - * {@code resetTablePicks(tblName, entriesToReset, "json")} - entriesToReset is a JsonArray + *
  • {@code resetTablePicks(tblName)} - reset all entries on a table + *
  • {@code resetTablePicks(tblName, entriesToReset)} - reset specific entries from a String + * List with "," delim + *
  • {@code resetTablePicks(tblName, entriesToReset, delim)} - use custom delimiter + *
  • {@code resetTablePicks(tblName, entriesToReset, "json")} - entriesToReset is a JsonArray *
* * @param function name of the MTscript function (used for error messages) * @param params list of function parameters * @throws ParserException Thrown when an invalid or missing parameter is supplied */ - private @NotNull String resetTablePicks(String function, List params) throws ParserException { + private @NotNull String resetTablePicks(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam(function, params, 1, 3); String tblName = params.get(0).toString(); @@ -286,7 +291,8 @@ private int getTablePicksLeft(String function, List params) throws Parse return ""; } - private @NotNull Object getTableEntry(String function, List params) throws ParserException { + private @NotNull Object getTableEntry(String function, List params) + throws ParserException { FunctionUtil.checkNumberParam(function, params, 2, 2); String name = params.get(0).toString(); LookupTable lookupTable = checkTableAccess(name, function); @@ -295,8 +301,7 @@ private int getTablePicksLeft(String function, List params) throws Parse if (entry == null) return ""; int rollInt = Integer.parseInt(roll); - if (rollInt < entry.getMin() || rollInt > entry.getMax()) - return ""; + if (rollInt < entry.getMin() || rollInt > entry.getMax()) return ""; JsonObject entryDetails = new JsonObject(); entryDetails.addProperty("min", entry.getMin()); @@ -327,8 +332,7 @@ private int setTableEntry(String function, List params) throws ParserExc LookupEntry entry = lookupTable.getLookup(roll); if (entry == null) return 0; int rollInt = Integer.parseInt(roll); - if (rollInt < entry.getMin() || rollInt > entry.getMax()) - return 0; + if (rollInt < entry.getMin() || rollInt > entry.getMax()) return 0; List oldlist = new ArrayList<>(lookupTable.getEntryList()); lookupTable.clearEntries(); for (LookupEntry e : oldlist) { @@ -357,7 +361,8 @@ private int setTableEntry(String function, List params) throws ParserExc return ""; } - private @NotNull String setTableImage(String function, List params) throws ParserException { + private @NotNull String setTableImage(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("setTableImage", params, 2, 2); String name = params.get(0).toString(); @@ -368,7 +373,8 @@ private int setTableEntry(String function, List params) throws ParserExc return ""; } - private @NotNull Serializable getTableImage(String function, List params) throws ParserException { + private @NotNull Serializable getTableImage(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("getTableImage", params, 1, 1); String name = params.getFirst().toString(); @@ -408,7 +414,8 @@ private int setTableEntry(String function, List params) throws ParserExc return ""; } - private @NotNull String deleteTableEntry(String function, List params) throws ParserException { + private @NotNull String deleteTableEntry(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("deleteTableEntry", params, 2, 2); String name = params.get(0).toString(); @@ -427,7 +434,8 @@ private int setTableEntry(String function, List params) throws ParserExc return ""; } - private @NotNull String addTableEntry(String function, List params) throws ParserException { + private @NotNull String addTableEntry(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("addTableEntry", params, 4, 5); String name = params.get(0).toString(); @@ -455,30 +463,32 @@ private int setTableEntry(String function, List params) throws ParserExc } /** - * Loads bulk data into a table. The table must already exist. Usage: - *

- * {@code [h: loadTable(name, jsondata)]} - *

- *

- * The {@code jsondata} must be a JSON Array containing table entries, and - * each table entry must be a JSON Array containing: + * Loads bulk data into a table. The table must already exist. Usage: + * + *

{@code [h: loadTable(name, jsondata)]} + * + *

The {@code jsondata} must be a JSON Array containing table entries, and each table entry + * must be a JSON Array containing: + * *

    - *
  1. an integer representing the lower range of the die roll (decimals are truncated),
    2. - *
  2. an integer representing the upper range of the die roll (decimals are truncated),
    3. - *
  3. a String acting as the content of the row, and
    4. - *
  4. a String acting as an image reference (as {@code asset://}, {@code image:}, or {@code MD5Key})
    5. + *
  5. an integer representing the lower range of the die roll (decimals are truncated), + *
  6. an integer representing the upper range of the die roll (decimals are truncated), + *
  7. a String acting as the content of the row, and + *
  8. a String acting as an image reference (as {@code asset://}, {@code image:}, or {@code + * MD5Key}) *
- *

+ * * @param function name of this MTscript function, "{@code loadTable}" * @param params parameters of the MTscript function - * @return a number indicating the successful record count, or - * a {@link JsonArray} of all elements that failed to load - * @throws ParserException Thrown when syntax errors are detected in the input data. - * This includes negative numbers in either of the numeric fields of a table entry, - * a lower die roll that is greater than the upper die roll of a table entry and vice versa, - * missing fields, too many fields, and potentially many more. + * @return a number indicating the successful record count, or a {@link JsonArray} of all elements + * that failed to load + * @throws ParserException Thrown when syntax errors are detected in the input data. This includes + * negative numbers in either of the numeric fields of a table entry, a lower die roll that is + * greater than the upper die roll of a table entry and vice versa, missing fields, too many + * fields, and potentially many more. */ - private @NotNull JsonElement loadTable(String function, List params) throws ParserException { + private @NotNull JsonElement loadTable(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam(function, params, 3, 3); String name = params.get(0).toString(); @@ -510,7 +520,7 @@ private int setTableEntry(String function, List params) throws ParserExc counter++; } } - } catch (UnsupportedOperationException|NumberFormatException|IllegalStateException e) { + } catch (UnsupportedOperationException | NumberFormatException | IllegalStateException e) { // Make a list of all failing rows to return to the user. errorRows.add(rowindex); } @@ -541,7 +551,8 @@ private String getTableRoll(String function, List params) throws ParserE return lookupTable.getDefaultRoll(); } - private @NotNull BigDecimal setTableAccess(String function, List params) throws ParserException { + private @NotNull BigDecimal setTableAccess(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("setTableAccess", params, 2, 2); String name = params.get(0).toString(); @@ -552,7 +563,8 @@ private String getTableRoll(String function, List params) throws ParserE return lookupTable.getAllowLookup() ? BigDecimal.ONE : BigDecimal.ZERO; } - private @NotNull BigDecimal getTableAccess(String function, List params) throws ParserException { + private @NotNull BigDecimal getTableAccess(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("getTableAccess", params, 1, 1); String name = params.getFirst().toString(); @@ -560,7 +572,8 @@ private String getTableRoll(String function, List params) throws ParserE return lookupTable.getAllowLookup() ? BigDecimal.ONE : BigDecimal.ZERO; } - private @NotNull BigDecimal setTableVisible(String function, List params) throws ParserException { + private @NotNull BigDecimal setTableVisible(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("setTableVisible", params, 2, 2); String name = params.get(0).toString(); @@ -571,7 +584,8 @@ private String getTableRoll(String function, List params) throws ParserE return lookupTable.getVisible() ? BigDecimal.ONE : BigDecimal.ZERO; } - private @NotNull BigDecimal getTableVisible(String function, List params) throws ParserException { + private @NotNull BigDecimal getTableVisible(String function, List params) + throws ParserException { checkTrusted(function); FunctionUtil.checkNumberParam("getTableVisible", params, 1, 1); String name = params.getFirst().toString(); diff --git a/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java b/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java index 90f555856b..5ef4d432b3 100644 --- a/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java @@ -17,6 +17,15 @@ import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonObject; +import java.awt.*; +import java.math.BigDecimal; +import java.text.SimpleDateFormat; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.util.*; +import java.util.List; +import java.util.concurrent.ConcurrentSkipListSet; +import javax.swing.*; import net.rptools.maptool.client.AppPreferences; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.MapToolExpressionParser; @@ -38,16 +47,6 @@ import net.rptools.parser.VariableResolver; import net.rptools.parser.function.AbstractFunction; -import javax.swing.*; -import java.awt.*; -import java.math.BigDecimal; -import java.text.SimpleDateFormat; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; -import java.util.List; -import java.util.*; -import java.util.concurrent.ConcurrentSkipListSet; - public class getInfoFunction extends AbstractFunction { /** The singleton instance. */ diff --git a/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java b/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java index 1daddd87e0..68275682e2 100644 --- a/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java +++ b/src/main/java/net/rptools/maptool/client/ui/lookuptable/EditLookupTablePanel.java @@ -14,6 +14,16 @@ */ package net.rptools.maptool.client.ui.lookuptable; +import java.awt.*; +import java.awt.event.MouseAdapter; +import java.awt.event.MouseEvent; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import javax.swing.*; +import javax.swing.table.AbstractTableModel; +import javax.swing.table.TableCellRenderer; +import javax.swing.table.TableModel; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.MapToolUtil; @@ -26,17 +36,6 @@ import net.rptools.maptool.model.LookupTable; import net.rptools.maptool.model.LookupTable.LookupEntry; -import javax.swing.*; -import javax.swing.table.AbstractTableModel; -import javax.swing.table.TableCellRenderer; -import javax.swing.table.TableModel; -import java.awt.*; -import java.awt.event.MouseAdapter; -import java.awt.event.MouseEvent; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - public class EditLookupTablePanel extends AbeillePanel { private static final long serialVersionUID = 2341539768448195059L; diff --git a/src/main/java/net/rptools/maptool/model/LookupTable.java b/src/main/java/net/rptools/maptool/model/LookupTable.java index 6e9f0a3396..d9db906100 100644 --- a/src/main/java/net/rptools/maptool/model/LookupTable.java +++ b/src/main/java/net/rptools/maptool/model/LookupTable.java @@ -15,6 +15,10 @@ package net.rptools.maptool.model; import com.google.protobuf.StringValue; +import java.util.*; +import java.util.stream.Collectors; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import net.rptools.dicelib.expression.ExpressionParser; import net.rptools.dicelib.expression.Result; import net.rptools.lib.MD5Key; @@ -24,18 +28,13 @@ import net.rptools.parser.ParserException; import org.jetbrains.annotations.Contract; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import java.util.*; -import java.util.stream.Collectors; - /** - * LookupTable represents a table of die roll ranges and a String value (with an optional - * asset ID). - *

- * The general idea is that random picks from a list are often a die roll, such as "2d6", - * that is then used to perform a lookup on a table. The typical random encounter table, - * for example, might look something like this: + * LookupTable represents a table of die roll ranges and a String value (with an optional asset ID). + * + *

The general idea is that random picks from a list are often a die roll, such as "2d6", that is + * then used to perform a lookup on a table. The typical random encounter table, for example, might + * look something like this: + * * * * @@ -46,51 +45,52 @@ * * *
Die range (2d6)ValueTable image
89Bandits, brother/sister teamnull
1012City guard, squad of 4null
- *

- * If the Die range contains only a single value, then only that value will select that row.
- * The default roll is the "2d6" shown in the table header (column one).
- * The Value column is the String returned for a given lookup.
- * The Table image column contains either null or an {@link MD5Key} for an asset. - *

+ * + *

If the Die range contains only a single value, then only that value will select that + * row.
+ * The default roll is the "2d6" shown in the table header (column one).
+ * The Value column is the String returned for a given lookup.
+ * The Table image column contains either null or an {@link MD5Key} for an + * asset. */ public class LookupTable { private static final ExpressionParser expressionParser = new ExpressionParser(); /** - * The complete list of entries in the table. - * (Future implementation changes will likely be sorted so that binary searches - * can find entries more quickly.) + * The complete list of entries in the table. (Future implementation changes will likely be sorted + * so that binary searches can find entries more quickly.) */ private @Nonnull List entryList = new ArrayList<>(); + private @Nullable String name; + /** * The die expression to use when one isn't provided when retrieving values. - *

- * If no value is set, the default implementation will choose a row randomly, - * ignoring the min/max values (so each row has an equal chance of being selected). + * + *

If no value is set, the default implementation will choose a row randomly, ignoring the + * min/max values (so each row has an equal chance of being selected). */ private @Nullable String defaultRoll; + private @Nullable MD5Key tableImage; - /** - * Whether the table is visible for players in the Table panel. - */ + + /** Whether the table is visible for players in the Table panel. */ private @Nonnull Boolean visible = true; - /** - * Whether players can read elements from the table. - */ + + /** Whether players can read elements from the table. */ 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(). + * Flags a table as "Pick Once", i.e. each entry can only be chosen once before the table must be + * reset(). */ private @Nonnull Boolean pickOnce = false; /** - * Unique string returned when all picks from a Pick Once table have - * been selected. - *

- * DO NOT use this string as a table value! 😁 + * Unique string returned when all picks from a Pick Once table have been selected. + * + *

DO NOT use this string as a table value! 😁 */ public static final String NO_PICKS_LEFT = "NO_PICKS_LEFT"; @@ -107,8 +107,8 @@ public LookupTable(LookupTable table) { } /** - * Sets the defaultRoll field that is used to choose a random element - * from the table when no die roll expression is provided. See {@link #getLookup()}. + * Sets the defaultRoll field that is used to choose a random element from the table + * when no die roll expression is provided. See {@link #getLookup()}. * * @param roll String expression representing default roll */ @@ -117,9 +117,9 @@ public void setRoll(String roll) { } /** - * Removes all entries from the table, but does not reset the default roll - * or other fields. (Note that for Pick Once tables, the pick status is - * part of each entry, so those are cleared by this method as well.) + * Removes all entries from the table, but does not reset the default roll or other fields. (Note + * that for Pick Once tables, the pick status is part of each entry, so those are cleared by this + * method as well.) */ public void clearEntries() { entryList.clear(); @@ -147,16 +147,15 @@ public LookupEntry getLookup() throws ParserException { return getLookup(null); } -// public String getRoll() { -// return getDefaultRoll(); -// } + // public String getRoll() { + // return getDefaultRoll(); + // } /** * Set the table name. - *

- * This method should not be called after the table is added to - * the campaign properties, as the Map<> therein relies on the - * name for the hash code. + * + *

This method should not be called after the table is added to the campaign properties, as the + * Map<> therein relies on the name for the hash code. * * @param name name of the table */ @@ -222,8 +221,8 @@ public LookupEntry getLookupDirect(String roll) throws ParserException { } /** - * Evaluates the roll expression and returns the appropriate table entry. - * The {@link #getLookup(String)} method delegates here for non-Pick Once tables. + * Evaluates the roll expression and returns the appropriate table entry. The {@link + * #getLookup(String)} method delegates here for non-Pick Once tables. * * @param roll dice expression * @return selected table entry; returns null if there's no matching range @@ -253,12 +252,11 @@ private LookupEntry getStandardLookup(String roll) throws ParserException { } /** - * Returns an entry from a Pick Once table that hasn't already been selected. - * The {@link #getLookup(String)} method delegates here for Pick Once tables. - *

- * For this method, the roll parameter must be a string that - * can be converted to integer via {@link Integer#parseInt(String)} -- a - * dice expression is not allowed. + * Returns an entry from a Pick Once table that hasn't already been selected. The {@link + * #getLookup(String)} method delegates here for Pick Once tables. + * + *

For this method, the roll parameter must be a string that can be converted to + * integer via {@link Integer#parseInt(String)} -- a dice expression is not allowed. * * @param roll String representing which entry to retrieve * @return the table entry corresponding to the roll parameter @@ -282,13 +280,13 @@ private LookupEntry getPickOnceLookup(String roll) throws ParserException { } /** - * Constrains the parameter to be within the range of values depicted by the table entries. - * Values less than the lowest minimum entry are set the minimum, while values larger than - * the largest maximum entry are set to the maximum. - *

- * There is no check to determine whether val actually identifies a specific - * table entry per the min/max values, so the return value (if unmodified) may not - * represent an actual table entry. + * Constrains the parameter to be within the range of values depicted by the table entries. Values + * less than the lowest minimum entry are set the minimum, while values larger than the largest + * maximum entry are set to the maximum. + * + *

There is no check to determine whether val actually identifies a specific table + * entry per the min/max values, so the return value (if unmodified) may not represent an actual + * table entry. * * @param val value to be constrained * @return the constrained value @@ -316,23 +314,24 @@ private int constrainRoll(int val) { /** * Has two modes of operation. + * *

    - *
  • For standard tables, it retrieves the {@link #defaultRoll} - * if it has been set, or calculates a dice expression to use and returns that. - *
  • For Pick Once tables, it ignores the {@link #defaultRoll} - * and instead rolls a random number which indexes into a subset of the table made up only - * of entries which have not been previously selected. The value of that - * entry is returned. + *
  • For standard tables, it retrieves the {@link #defaultRoll} if it has been set, or + * calculates a dice expression to use and returns that. + *
  • For Pick Once tables, it ignores the {@link #defaultRoll} and instead rolls a random + * number which indexes into a subset of the table made up only of entries which have not + * been previously selected. The value of that entry is returned. *
* - * @return the value field from the table for the associated entry; Pick Once - * tables also have their picked flag set so that they are not picked again + * @return the value field from the table for the associated entry; Pick Once tables + * also have their picked flag set so that they are not picked again */ public String getDefaultRoll() { if (getPickOnce()) { // For Pick Once tables this returns a random pick from those entries in the list that // have not been picked. - LookupEntry[] unpicked = entryList.stream().filter(e -> !e.picked).toArray(LookupEntry[]::new); + LookupEntry[] unpicked = + entryList.stream().filter(e -> !e.picked).toArray(LookupEntry[]::new); try { if (unpicked.length != 0) { Result result = expressionParser.evaluate("d" + unpicked.length); @@ -369,8 +368,8 @@ public String getDefaultRoll() { /** * Sets the picked flag on each table entry to false. - *

- * This method returns immediately if the table is not a Pick Once table. + * + *

This method returns immediately if the table is not a Pick Once table. */ public void reset() { if (pickOnce) { @@ -504,11 +503,14 @@ public static class LookupEntry { * @deprecated here to prevent xstream from breaking b24-b25 */ @SuppressWarnings("DeprecatedIsStillUsed") - @Deprecated private @Nullable String result; - - public LookupEntry(int min, int max, - @org.jetbrains.annotations.Nullable String value, - @org.jetbrains.annotations.Nullable MD5Key imageId) { + @Deprecated + private @Nullable String result; + + public LookupEntry( + int min, + int max, + @org.jetbrains.annotations.Nullable String value, + @org.jetbrains.annotations.Nullable MD5Key imageId) { this.min = min; this.max = max; this.value = value; @@ -597,8 +599,8 @@ public Set getAllAssetIds() { /** * Retrieves the visible flag for the LookupTable. * - * @return true indicates that the table will be visible to players. - * Otherwise, the table will be hidden from players. + * @return true indicates that the table will be visible to players. Otherwise, the + * table will be hidden from players. */ public boolean getVisible() { return visible; @@ -607,8 +609,8 @@ public boolean getVisible() { /** * Sets the visible flag for the LookupTable. * - * @param value true specifies that the table will be visible to players. - * Otherwise, the table will be hidden from players. + * @param value true specifies that the table will be visible to players. Otherwise, + * the table will be hidden from players. */ public void setVisible(boolean value) { visible = value; @@ -618,8 +620,8 @@ public void setVisible(boolean value) { * Retrieves the allowLookup flag for the LookupTable. * * @return true indicates that players can call for values from this table. - * Otherwise, players will be prevented from calling values from this table. - * GM's can ALWAYS perform lookups against a table. + * Otherwise, players will be prevented from calling values from this table. GM's can ALWAYS + * perform lookups against a table. */ public boolean getAllowLookup() { return allowLookup; @@ -629,8 +631,8 @@ public boolean getAllowLookup() { * Sets the allowLookup flag for the LookupTable. * * @param value true indicates that players can call for values from this table. - * Otherwise, players will be prevented from calling values from this table. - * GM's can ALWAYS perform lookups against a table. + * Otherwise, players will be prevented from calling values from this table. GM's can ALWAYS + * perform lookups against a table. */ public void setAllowLookup(boolean value) { allowLookup = value; diff --git a/src/main/java/net/rptools/maptool/util/FunctionUtil.java b/src/main/java/net/rptools/maptool/util/FunctionUtil.java index e0d31cd81b..e5913fc0ab 100644 --- a/src/main/java/net/rptools/maptool/util/FunctionUtil.java +++ b/src/main/java/net/rptools/maptool/util/FunctionUtil.java @@ -18,6 +18,11 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; +import java.math.BigDecimal; +import java.util.List; +import java.util.Locale; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.MapToolUtil; @@ -37,12 +42,6 @@ import net.rptools.parser.VariableResolver; import net.rptools.parser.function.Function; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import java.math.BigDecimal; -import java.util.List; -import java.util.Locale; - /** * Provides static methods to help handle macro functions. * diff --git a/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java b/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java index 6ac4081f67..c3673daad8 100644 --- a/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java +++ b/src/test/java/net/rptools/maptool/client/functions/LookupTableTests.java @@ -14,88 +14,85 @@ */ package net.rptools.maptool.client.functions; +import static org.junit.jupiter.api.Assertions.*; + import net.rptools.lib.MD5Key; import net.rptools.maptool.model.LookupTable; import net.rptools.parser.ParserException; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; - public class LookupTableTests { - private static final LookupTable table = new LookupTable(); + private static final LookupTable table = new LookupTable(); - private void buildTestTable() { - table.clearEntries(); - table.setName("test"); - table.setRoll("1d6"); - table.setPickOnce(false); - table.setVisible(false); - table.setAllowLookup(true); - } + private void buildTestTable() { + table.clearEntries(); + table.setName("test"); + table.setRoll("1d6"); + table.setPickOnce(false); + table.setVisible(false); + table.setAllowLookup(true); + } - /** - * Doesn't register with the campaign's table map, so no unregistering - * is necessary. - */ - @Test - void table_creation() { - // Kind of silly to check all these, but there could be - // new logic added in the future that does processing in - // the various get/set methods, so... - buildTestTable(); - // Must start empty - assertTrue(table.getEntryList().isEmpty()); - // Check that individual fields can be set and retrieved - assertEquals("test", table.getName()); - assertEquals("1d6", table.getDefaultRoll()); - MD5Key md5 = new MD5Key("1234"); - table.setTableImage(md5); - assertEquals(md5, table.getTableImage()); - assertFalse(table.getPickOnce()); - assertFalse(table.getVisible()); - assertTrue(table.getAllowLookup()); - } + /** Doesn't register with the campaign's table map, so no unregistering is necessary. */ + @Test + void table_creation() { + // Kind of silly to check all these, but there could be + // new logic added in the future that does processing in + // the various get/set methods, so... + buildTestTable(); + // Must start empty + assertTrue(table.getEntryList().isEmpty()); + // Check that individual fields can be set and retrieved + assertEquals("test", table.getName()); + assertEquals("1d6", table.getDefaultRoll()); + MD5Key md5 = new MD5Key("1234"); + table.setTableImage(md5); + assertEquals(md5, table.getTableImage()); + assertFalse(table.getPickOnce()); + assertFalse(table.getVisible()); + assertTrue(table.getAllowLookup()); + } - @Test - void table_addOneEntry() throws ParserException { - buildTestTable(); - // This entry must be the first one - table.addEntry(10, 20, "result", null); + @Test + void table_addOneEntry() throws ParserException { + buildTestTable(); + // This entry must be the first one + table.addEntry(10, 20, "result", null); - // Check a roll that's in the middle of the range. - LookupTable.LookupEntry le = table.getLookup("15"); - assertEquals(10, le.getMin()); - assertEquals(20, le.getMax()); - assertEquals("result", le.getValue()); - assertNull(le.getImageId()); - } + // Check a roll that's in the middle of the range. + LookupTable.LookupEntry le = table.getLookup("15"); + assertEquals(10, le.getMin()); + assertEquals(20, le.getMax()); + assertEquals("result", le.getValue()); + assertNull(le.getImageId()); + } - private void populateTable() { - buildTestTable(); - table.addEntry(10, 20, "result 1", null); - table.addEntry(25, 30, "result 2", null); - table.addEntry(40, 50, "result 3", null); - } + private void populateTable() { + buildTestTable(); + table.addEntry(10, 20, "result 1", null); + table.addEntry(25, 30, "result 2", null); + table.addEntry(40, 50, "result 3", null); + } - @Test - void table_countEntries() { - populateTable(); + @Test + void table_countEntries() { + populateTable(); - assertEquals(3, table.getEntryList().size()); - } + assertEquals(3, table.getEntryList().size()); + } - @Test - void table_addMultipleEntries() throws ParserException { - populateTable(); + @Test + void table_addMultipleEntries() throws ParserException { + populateTable(); - LookupTable.LookupEntry le = table.getLookup("25"); - assertEquals(25, le.getMin()); - assertEquals(30, le.getMax()); - assertEquals("result 2", le.getValue()); - assertNull(le.getImageId()); + LookupTable.LookupEntry le = table.getLookup("25"); + assertEquals(25, le.getMin()); + assertEquals(30, le.getMax()); + assertEquals("result 2", le.getValue()); + assertNull(le.getImageId()); - // Make sure both lookups return the same table entry - LookupTable.LookupEntry le2 = table.getLookup("30"); - assertEquals(le, le2); - } + // Make sure both lookups return the same table entry + LookupTable.LookupEntry le2 = table.getLookup("30"); + assertEquals(le, le2); + } } From 6ee6f5641a216f3b0cf16be958a7b5fc33ad7686 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Sun, 26 May 2024 18:30:46 -0400 Subject: [PATCH 09/11] Revert "Update nightly-publish.yml" This reverts commit ee9a5358d0e2fc7f9360bd0825e92c3860cb0437. --- .github/workflows/nightly-publish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nightly-publish.yml b/.github/workflows/nightly-publish.yml index eb60a76f95..19e53f118b 100644 --- a/.github/workflows/nightly-publish.yml +++ b/.github/workflows/nightly-publish.yml @@ -1,7 +1,7 @@ -name: Nightly Release (disabled) +name: Nightly Release on: - watch: # Change to 'schedule' to re-enable this build + schedule: - cron: '13 2 * * *' # Odd time so that it doesn't run afoul of busy periods everyone picks jobs: From 1fee87eacafe7281e736e0b491d50cf1de4efc47 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Sun, 26 May 2024 18:42:32 -0400 Subject: [PATCH 10/11] Revert "Fixup of setTableEntry performance" This reverts commit e1796d2b822903c0ed222033a4d0cc170a4a89e2. --- .../client/functions/LookupTableFunction.java | 34 +++++++++++++++++++ .../client/functions/getInfoFunction.java | 12 ++++++- .../rptools/maptool/util/FunctionUtil.java | 7 ++-- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java index 2175ab53e3..54919bef66 100644 --- a/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/LookupTableFunction.java @@ -634,4 +634,38 @@ private List getTableList(boolean isGm) { .forEachOrdered((lt) -> tables.add(lt.getName())); return tables; } + + /** + * Function to return a MapTool table. + * + * @param tableName String containing the name of the desired table + * @param functionName String containing the name of the calling function, used by the error + * message. + * @return LookupTable The desired MapTool table object + * @throws ParserException if there were more or less parameters than allowed + */ + private LookupTable getMaptoolTable(String tableName, String functionName) + throws ParserException { + + LookupTable lookupTable = MapTool.getCampaign().getLookupTableMap().get(tableName); + if (!MapTool.getPlayer().isGM() && !lookupTable.getAllowLookup()) { + if (lookupTable.getVisible()) { + throw new ParserException( + functionName + "(): " + I18N.getText("msg.error.tableUnknown") + tableName); + } else { + throw new ParserException( + functionName + + "(): " + + I18N.getText("msg.error.tableAccessProhibited") + + ": " + + tableName); + } + } + if (lookupTable == null) { + throw new ParserException( + I18N.getText( + "macro.function.LookupTableFunctions.unknownTable", functionName, tableName)); + } + return lookupTable; + } } diff --git a/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java b/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java index 5ef4d432b3..c691a0a845 100644 --- a/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java +++ b/src/main/java/net/rptools/maptool/client/functions/getInfoFunction.java @@ -35,7 +35,17 @@ import net.rptools.maptool.client.ui.token.*; import net.rptools.maptool.client.ui.zone.renderer.ZoneRenderer; import net.rptools.maptool.language.I18N; -import net.rptools.maptool.model.*; +import net.rptools.maptool.model.Campaign; +import net.rptools.maptool.model.CampaignProperties; +import net.rptools.maptool.model.Grid; +import net.rptools.maptool.model.GridFactory; +import net.rptools.maptool.model.Light; +import net.rptools.maptool.model.LightSource; +import net.rptools.maptool.model.LookupTable; +import net.rptools.maptool.model.ShapeType; +import net.rptools.maptool.model.SightType; +import net.rptools.maptool.model.Token; +import net.rptools.maptool.model.Zone; import net.rptools.maptool.model.drawing.DrawableColorPaint; import net.rptools.maptool.model.drawing.DrawableTexturePaint; import net.rptools.maptool.server.ServerPolicy; diff --git a/src/main/java/net/rptools/maptool/util/FunctionUtil.java b/src/main/java/net/rptools/maptool/util/FunctionUtil.java index e5913fc0ab..0c51881c60 100644 --- a/src/main/java/net/rptools/maptool/util/FunctionUtil.java +++ b/src/main/java/net/rptools/maptool/util/FunctionUtil.java @@ -526,15 +526,14 @@ public static DrawablePaint getPaintFromString(String paint) { */ public static @Nullable MD5Key getAssetKeyFromString(String assetUrlOrId) { String id = null; - final String paramAsLC = assetUrlOrId.toLowerCase(Locale.ROOT); - if (paramAsLC.startsWith("asset://")) { + if (assetUrlOrId.toLowerCase().startsWith("asset://")) { id = assetUrlOrId.substring("asset://".length()); - } else if (paramAsLC.startsWith("lib://")) { + } else if (assetUrlOrId.toLowerCase().startsWith("lib://")) { var assetKey = new AssetResolver().getAssetKey(assetUrlOrId); if (assetKey.isPresent()) { id = assetKey.get().toString(); } - } else if (paramAsLC.startsWith("image:")) { + } else if (assetUrlOrId.toLowerCase().startsWith("image:")) { for (ZoneRenderer z : MapTool.getFrame().getZoneRenderers()) { Token t = z.getZone().getTokenByName(assetUrlOrId); if (t != null) { From 973dd4e59ad723e3ef65a2436be7d79560fe0272 Mon Sep 17 00:00:00 2001 From: Frank Edwards Date: Sun, 26 May 2024 18:45:38 -0400 Subject: [PATCH 11/11] Ran spotlessApply after reverting a couple commits --- src/main/java/net/rptools/maptool/util/FunctionUtil.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/net/rptools/maptool/util/FunctionUtil.java b/src/main/java/net/rptools/maptool/util/FunctionUtil.java index 0c51881c60..e2ab99ea21 100644 --- a/src/main/java/net/rptools/maptool/util/FunctionUtil.java +++ b/src/main/java/net/rptools/maptool/util/FunctionUtil.java @@ -20,7 +20,6 @@ import com.google.gson.JsonPrimitive; import java.math.BigDecimal; import java.util.List; -import java.util.Locale; import javax.annotation.Nonnull; import javax.annotation.Nullable; import net.rptools.lib.MD5Key;