From 7e44819dd9efbb5605c27d764baa483023decff0 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 19 Jun 2026 10:28:02 -0700 Subject: [PATCH 1/3] Prefer named column access (avoids indexing bug that caused our issue); recalculate row heights on any table change --- .../ui/lookuptable/EditLookupTablePanel.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) 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 89575ec387..5c16c8f7de 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 @@ -123,12 +123,14 @@ public void initTableDefinitionTable() { new MouseAdapter() { @Override public void mousePressed(MouseEvent e) { - int column = definitionTable.columnAtPoint(e.getPoint()); + var model = (LookupTableTableModel) definitionTable.getModel(); + + int column = model.getCanonicalColumn(definitionTable.columnAtPoint(e.getPoint())); if (column != IMAGE_COLUMN_INDEX) { return; } - int row = definitionTable.rowAtPoint(e.getPoint()); - String imageIdStr = (String) definitionTable.getModel().getValueAt(row, column); + var row = model.getRowAt(definitionTable.rowAtPoint(e.getPoint())); + String imageIdStr = row.imageId; // HACK: this is a hacky way to figure out if the button was pushed :P if (e.getPoint().x > definitionTable.getSize().width - 15) { @@ -158,7 +160,9 @@ public void mousePressed(MouseEvent e) { } imageIdStr = imageId.toString(); } - definitionTable.getModel().setValueAt(imageIdStr, row, column); + + // Commit our changes back. + row.imageId = imageIdStr; updateDefinitionTableRowHeights(); definitionTable.repaint(); } @@ -190,7 +194,10 @@ public void bind(LookupTable lookupTable) { view.getTableName().requestFocusInWindow(); - view.getDefinitionTable().setModel(createLookupTableModel(lookupTable)); + var model = createLookupTableModel(lookupTable); + // Ensure our row heights changge as the table is modified. + model.addTableModelListener(e -> updateDefinitionTableRowHeights()); + view.getDefinitionTable().setModel(model); updateDefinitionTableRowHeights(); } @@ -293,8 +300,10 @@ public boolean commit() { private void updateDefinitionTableRowHeights() { JTable table = view.getDefinitionTable(); + var model = (LookupTableTableModel) table.getModel(); + for (int row = 0; row < table.getRowCount(); row++) { - String imageId = (String) table.getModel().getValueAt(row, IMAGE_COLUMN_INDEX); + String imageId = model.getRowAt(row).imageId; table.setRowHeight(row, imageId != null && !imageId.isEmpty() ? 100 : defaultRowHeight); } } @@ -361,7 +370,7 @@ public LookupTableTableModel(List rowList) { this.rowList = rowList; } - private int getCanonicalColumn(int columnIndex) { + public int getCanonicalColumn(int columnIndex) { // When the "Picked?" column is not present, the column indices don't line up with our // predefined constants. So increment the index to match those. return columnIndex + (showPicks ? 0 : 1); From 570cc43baef031ac2d430fb53742a31dd5b3eb4d Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 19 Jun 2026 10:43:46 -0700 Subject: [PATCH 2/3] Encapsulate LookupTableRow fields behind getters and settings --- .../ui/lookuptable/EditLookupTablePanel.java | 87 +++++++++++++------ 1 file changed, 60 insertions(+), 27 deletions(-) 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 5c16c8f7de..d80c9ead07 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 @@ -130,7 +130,7 @@ public void mousePressed(MouseEvent e) { return; } var row = model.getRowAt(definitionTable.rowAtPoint(e.getPoint())); - String imageIdStr = row.imageId; + String imageIdStr = row.getImageId(); // HACK: this is a hacky way to figure out if the button was pushed :P if (e.getPoint().x > definitionTable.getSize().width - 15) { @@ -162,7 +162,7 @@ public void mousePressed(MouseEvent e) { } // Commit our changes back. - row.imageId = imageIdStr; + row.setImageId(imageIdStr); updateDefinitionTableRowHeights(); definitionTable.repaint(); } @@ -236,36 +236,37 @@ public boolean commit() { var entries = new ArrayList(); for (int i = 0; i < tableModel.getRowCount(); i++) { var row = tableModel.getRowAt(i); - if (row.range.isEmpty()) { + var range = row.getRange(); + if (range.isEmpty()) { continue; } int min; int max; // Allow negative numbers - int split = row.range.indexOf('-', row.range.charAt(0) == '-' ? 1 : 0); + int split = range.indexOf('-', range.charAt(0) == '-' ? 1 : 0); try { if (split < 0) { - min = Integer.parseInt(row.range); + min = Integer.parseInt(range); max = min; } else { - min = Integer.parseInt(row.range.substring(0, split).trim()); - max = Integer.parseInt(row.range.substring(split + 1).trim()); + min = Integer.parseInt(range.substring(0, split).trim()); + max = Integer.parseInt(range.substring(split + 1).trim()); } } catch (NumberFormatException nfe) { - MapTool.showError(I18N.getText("EditLookupTablePanel.error.badRange", name, row.range, i)); + MapTool.showError(I18N.getText("EditLookupTablePanel.error.badRange", name, range, i)); return false; } MD5Key image = null; - if (row.imageId != null && !row.imageId.isEmpty()) { - image = new MD5Key(row.imageId); + if (row.getImageId() != null && !row.getImageId().isEmpty()) { + image = new MD5Key(row.getImageId()); MapToolUtil.uploadAsset(AssetManager.getAsset(image)); } - var entry = new LookupEntry(min, max, row.value, image); + var entry = new LookupEntry(min, max, row.getValue(), image); if (isPickOnce) { - entry.setPicked(row.picked); + entry.setPicked(row.isPicked()); } entries.add(entry); } @@ -303,7 +304,7 @@ private void updateDefinitionTableRowHeights() { var model = (LookupTableTableModel) table.getModel(); for (int row = 0; row < table.getRowCount(); row++) { - String imageId = model.getRowAt(row).imageId; + String imageId = model.getRowAt(row).getImageId(); table.setRowHeight(row, imageId != null && !imageId.isEmpty() ? 100 : defaultRowHeight); } } @@ -337,10 +338,10 @@ public Component getTableCellRendererComponent( } private static final class LookupTableRow { - public boolean picked; - public String range; - public String value; - public @Nullable String imageId; + private boolean picked; + private String range; + private String value; + private @Nullable String imageId; public LookupTableRow() { this(false, "", "", null); @@ -352,6 +353,38 @@ public LookupTableRow(boolean picked, String range, String value, @Nullable Stri this.value = value; this.imageId = imageId; } + + public boolean isPicked() { + return picked; + } + + public void setPicked(boolean picked) { + this.picked = picked; + } + + public String getRange() { + return range; + } + + public void setRange(String range) { + this.range = range; + } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + public @Nullable String getImageId() { + return imageId; + } + + public void setImageId(@Nullable String imageId) { + this.imageId = imageId; + } } private static final class LookupTableTableModel extends AbstractTableModel { @@ -385,9 +418,9 @@ public void showPicks(boolean show) { public void clearAllPicks() { for (var row : rowList) { - row.picked = false; + row.setPicked(false); } - newRow.picked = false; + newRow.setPicked(false); fireTableRowsUpdated(0, getRowCount()); } @@ -419,10 +452,10 @@ public Object getValueAt(int rowIndex, int columnIndex) { LookupTableRow row = rowIndex < rowList.size() ? rowList.get(rowIndex) : newRow; return switch (columnIndex) { - case PICKED_COLUMN_INDEX -> row.picked; - case RANGE_COLUMN_INDEX -> row.range; - case VALUE_COLUMN_INDEX -> row.value; - case IMAGE_COLUMN_INDEX -> row.imageId; + case PICKED_COLUMN_INDEX -> row.isPicked(); + case RANGE_COLUMN_INDEX -> row.getRange(); + case VALUE_COLUMN_INDEX -> row.getValue(); + case IMAGE_COLUMN_INDEX -> row.getImageId(); default -> ""; }; } @@ -434,10 +467,10 @@ public void setValueAt(Object aValue, int rowIndex, int columnIndex) { LookupTableRow row = rowIndex < rowList.size() ? rowList.get(rowIndex) : newRow; switch (columnIndex) { - case PICKED_COLUMN_INDEX -> row.picked = (boolean) aValue; - case RANGE_COLUMN_INDEX -> row.range = (String) aValue; - case VALUE_COLUMN_INDEX -> row.value = (String) aValue; - case IMAGE_COLUMN_INDEX -> row.imageId = (String) aValue; + case PICKED_COLUMN_INDEX -> row.setPicked((boolean) aValue); + case RANGE_COLUMN_INDEX -> row.setRange((String) aValue); + case VALUE_COLUMN_INDEX -> row.setValue((String) aValue); + case IMAGE_COLUMN_INDEX -> row.setImageId((String) aValue); } if (row == newRow) { From 3fefb3fa63ec1ed92eee85eb68b65346dd4969b9 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 19 Jun 2026 11:23:03 -0700 Subject: [PATCH 3/3] Make LookupTableRow observable, with LookupTableTableModel reacting to changes --- .../ui/lookuptable/EditLookupTablePanel.java | 59 ++++++++++++++----- 1 file changed, 44 insertions(+), 15 deletions(-) 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 d80c9ead07..b345e79fe1 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 @@ -19,6 +19,9 @@ import java.awt.Dimension; import java.awt.event.MouseAdapter; import java.awt.event.MouseEvent; +import java.beans.PropertyChangeEvent; +import java.beans.PropertyChangeListener; +import java.beans.PropertyChangeSupport; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -118,7 +121,7 @@ public void initTableDefinitionTable() { defaultRowHeight = definitionTable.getRowHeight(); definitionTable.setDefaultRenderer(ImageAssetPanel.class, new ImageCellRenderer()); - definitionTable.setModel(createLookupTableModel(new LookupTable())); + definitionTable.setModel(new LookupTableTableModel()); definitionTable.addMouseListener( new MouseAdapter() { @Override @@ -163,8 +166,6 @@ public void mousePressed(MouseEvent e) { // Commit our changes back. row.setImageId(imageIdStr); - updateDefinitionTableRowHeights(); - definitionTable.repaint(); } }); } @@ -194,11 +195,10 @@ public void bind(LookupTable lookupTable) { view.getTableName().requestFocusInWindow(); - var model = createLookupTableModel(lookupTable); - // Ensure our row heights changge as the table is modified. + var model = new LookupTableTableModel(); model.addTableModelListener(e -> updateDefinitionTableRowHeights()); view.getDefinitionTable().setModel(model); - updateDefinitionTableRowHeights(); + populateLookupTableModel(lookupTable, model); } @Override @@ -309,8 +309,7 @@ private void updateDefinitionTableRowHeights() { } } - private LookupTableTableModel createLookupTableModel(LookupTable lookupTable) { - var rows = new ArrayList(); + private void populateLookupTableModel(LookupTable lookupTable, LookupTableTableModel model) { for (LookupEntry entry : lookupTable.getEntryList()) { boolean picked = entry.getPicked(); String range = @@ -320,12 +319,9 @@ private LookupTableTableModel createLookupTableModel(LookupTable lookupTable) { String value = entry.getValue(); MD5Key imageId = entry.getImageId(); - rows.add( - new LookupTableRow(picked, range, value, imageId != null ? imageId.toString() : null)); + model.addRow(picked, range, value, imageId != null ? imageId.toString() : null); } - var model = new LookupTableTableModel(rows); model.showPicks(lookupTable.getPickOnce()); - return model; } private class ImageCellRenderer extends ImageAssetPanel implements TableCellRenderer { @@ -338,6 +334,7 @@ public Component getTableCellRendererComponent( } private static final class LookupTableRow { + private final PropertyChangeSupport pcs; private boolean picked; private String range; private String value; @@ -352,6 +349,15 @@ public LookupTableRow(boolean picked, String range, String value, @Nullable Stri this.range = range; this.value = value; this.imageId = imageId; + this.pcs = new PropertyChangeSupport(this); + } + + public void addPropertyChangeListener(PropertyChangeListener listener) { + this.pcs.addPropertyChangeListener(listener); + } + + public void removePropertyChangeListener(PropertyChangeListener listener) { + this.pcs.removePropertyChangeListener(listener); } public boolean isPicked() { @@ -359,7 +365,9 @@ public boolean isPicked() { } public void setPicked(boolean picked) { + var oldValue = this.picked; this.picked = picked; + this.pcs.firePropertyChange("picked", oldValue, picked); } public String getRange() { @@ -367,7 +375,9 @@ public String getRange() { } public void setRange(String range) { + var oldValue = this.range; this.range = range; + this.pcs.firePropertyChange("range", oldValue, range); } public String getValue() { @@ -375,7 +385,9 @@ public String getValue() { } public void setValue(String value) { + var oldValue = this.value; this.value = value; + this.pcs.firePropertyChange("value", oldValue, value); } public @Nullable String getImageId() { @@ -383,11 +395,14 @@ public void setValue(String value) { } public void setImageId(@Nullable String imageId) { + var oldValue = this.imageId; this.imageId = imageId; + this.pcs.firePropertyChange("imageId", oldValue, imageId); } } - private static final class LookupTableTableModel extends AbstractTableModel { + private static final class LookupTableTableModel extends AbstractTableModel + implements PropertyChangeListener { private LookupTableRow newRow = new LookupTableRow(); private final List rowList; @@ -399,8 +414,21 @@ private static final class LookupTableTableModel extends AbstractTableModel { I18N.getText("Label.image")); private boolean showPicks = false; - public LookupTableTableModel(List rowList) { - this.rowList = rowList; + public LookupTableTableModel() { + this.rowList = new ArrayList<>(); + newRow.addPropertyChangeListener(this); + } + + @Override + public void propertyChange(PropertyChangeEvent evt) { + fireTableDataChanged(); + } + + public void addRow(boolean isPicked, String range, String value, @Nullable String imageId) { + var row = new LookupTableRow(isPicked, range, value, imageId); + rowList.add(row); + row.addPropertyChangeListener(this); + fireTableRowsInserted(rowList.size(), rowList.size()); } public int getCanonicalColumn(int columnIndex) { @@ -477,6 +505,7 @@ public void setValueAt(Object aValue, int rowIndex, int columnIndex) { // Need to make the row permanent, and add a new placeholder. rowList.add(newRow); newRow = new LookupTableRow(); + newRow.addPropertyChangeListener(this); fireTableRowsInserted(rowList.size(), rowList.size()); } }