Merge pull request #617 from teyrana/fix/536-fin-npe

[Fixes #536] NullPointerException while editing FreeformFinSet Components
This commit is contained in:
Daniel Williams 2020-04-06 21:33:40 -04:00 committed by GitHub
commit d61d0e47c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 49 additions and 48 deletions

View File

@ -1,11 +1,6 @@
package net.sf.openrocket.rocketcomponent; package net.sf.openrocket.rocketcomponent;
import java.util.Collection; import java.util.*;
import java.util.EventListener;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@ -39,7 +34,7 @@ public class Rocket extends ComponentAssembly {
/** /**
* List of component change listeners. * List of component change listeners.
*/ */
private List<EventListener> listenerList = new ArrayList<>(); private Set<EventListener> listenerList = new HashSet<>();
/** /**
* When freezeList != null, events are not dispatched but stored in the list. * When freezeList != null, events are not dispatched but stored in the list.
@ -332,7 +327,7 @@ public class Rocket extends ComponentAssembly {
copy.stageMap = new HashMap<Integer, AxialStage>(); copy.stageMap = new HashMap<Integer, AxialStage>();
copy.configSet = new FlightConfigurableParameterSet<FlightConfiguration>( this.configSet ); copy.configSet = new FlightConfigurableParameterSet<FlightConfiguration>( this.configSet );
copy.selectedConfiguration = copy.configSet.get( this.getSelectedConfiguration().getId()); copy.selectedConfiguration = copy.configSet.get( this.getSelectedConfiguration().getId());
copy.listenerList = new ArrayList<EventListener>(); copy.listenerList = new HashSet<EventListener>();
return copy; return copy;
} }
@ -397,7 +392,7 @@ public class Rocket extends ComponentAssembly {
*/ */
public void resetListeners() { public void resetListeners() {
// System.out.println("RESETTING LISTENER LIST of Rocket "+this); // System.out.println("RESETTING LISTENER LIST of Rocket "+this);
listenerList = new ArrayList<EventListener>(); listenerList = new HashSet<EventListener>();
} }
@ -413,16 +408,16 @@ public class Rocket extends ComponentAssembly {
@Override @Override
public void addComponentChangeListener(ComponentChangeListener l) { public void addComponentChangeListener(ComponentChangeListener l) {
checkState(); checkState();
listenerList.add(l); listenerList.add(l);
log.trace("Added ComponentChangeListener " + l + ", current number of listeners is " +
listenerList.size()); log.trace("Added ComponentChangeListener " + l + ", current number of listeners is " + listenerList.size());
} }
@Override @Override
public void removeComponentChangeListener(ComponentChangeListener l) { public void removeComponentChangeListener(ComponentChangeListener l) {
listenerList.remove(l); listenerList.remove(l);
log.trace("Removed ComponentChangeListener " + l + ", current number of listeners is " + log.trace("Removed ComponentChangeListener " + l + ", current number of listeners is " + listenerList.size());
listenerList.size());
} }
@Override @Override

View File

@ -2,6 +2,8 @@ package net.sf.openrocket.gui.configdialog;
import java.awt.Window; import java.awt.Window;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.lang.reflect.Constructor; import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException; import java.lang.reflect.InvocationTargetException;
@ -30,10 +32,9 @@ public class ComponentConfigDialog extends JDialog implements ComponentChangeLis
private static final String CONFIGDIALOGPACKAGE = "net.sf.openrocket.gui.configdialog"; private static final String CONFIGDIALOGPACKAGE = "net.sf.openrocket.gui.configdialog";
private static final String CONFIGDIALOGPOSTFIX = "Config"; private static final String CONFIGDIALOGPOSTFIX = "Config";
// Static Value -- This is a singleton value, and we should only have zero or one active at any time
private static ComponentConfigDialog dialog = null; private static ComponentConfigDialog dialog = null;
private OpenRocketDocument document = null; private OpenRocketDocument document = null;
private RocketComponent component = null; private RocketComponent component = null;
private RocketComponentConfig configurator = null; private RocketComponentConfig configurator = null;
@ -41,8 +42,7 @@ public class ComponentConfigDialog extends JDialog implements ComponentChangeLis
private final Window parent; private final Window parent;
private static final Translator trans = Application.getTranslator(); private static final Translator trans = Application.getTranslator();
private ComponentConfigDialog(Window parent, OpenRocketDocument document, private ComponentConfigDialog(Window parent, OpenRocketDocument document, RocketComponent component) {
RocketComponent component) {
super(parent); super(parent);
this.parent = parent; this.parent = parent;
@ -50,6 +50,22 @@ public class ComponentConfigDialog extends JDialog implements ComponentChangeLis
GUIUtil.setDisposableDialogOptions(this, null); GUIUtil.setDisposableDialogOptions(this, null);
GUIUtil.rememberWindowPosition(this); GUIUtil.rememberWindowPosition(this);
// overrides common defaults in 'GUIUTIL.setDisposableDialogOptions', above
setDefaultCloseOperation(JDialog.DO_NOTHING_ON_CLOSE);
addWindowListener(new WindowAdapter() {
/**
* Triggered by the 'Close' Button on the ConfigDialogs. AND Esc. AND the [x] button (on Windows)
* In fact, it should trigger for any method of closing the dialog.
*/
public void windowClosed(WindowEvent e){
configurator.invalidate();
document.getRocket().removeComponentChangeListener(ComponentConfigDialog.this);
ComponentConfigDialog.this.dispose();
}
public void windowClosing(WindowEvent e){}
});
} }
@ -60,10 +76,6 @@ public class ComponentConfigDialog extends JDialog implements ComponentChangeLis
* @param component Component to configure. * @param component Component to configure.
*/ */
private void setComponent(OpenRocketDocument document, RocketComponent component) { private void setComponent(OpenRocketDocument document, RocketComponent component) {
if (this.document != null) {
this.document.getRocket().removeComponentChangeListener(this);
}
if (configurator != null) { if (configurator != null) {
// Remove listeners by setting all applicable models to null // Remove listeners by setting all applicable models to null
GUIUtil.setNullModels(configurator); // null-safe GUIUtil.setNullModels(configurator); // null-safe
@ -106,14 +118,6 @@ public class ComponentConfigDialog extends JDialog implements ComponentChangeLis
throw new BugException("Unable to find any configurator for " + component); throw new BugException("Unable to find any configurator for " + component);
} }
private void closeDialog() {
this.setVisible(false);
this.dispose();
this.configurator.invalidateModels();
}
@Override @Override
public void componentChanged(ComponentChangeEvent e) { public void componentChanged(ComponentChangeEvent e) {
if (e.isTreeChange() || e.isUndoChange()) { if (e.isTreeChange() || e.isUndoChange()) {
@ -204,7 +208,7 @@ public class ComponentConfigDialog extends JDialog implements ComponentChangeLis
*/ */
public static void hideDialog() { public static void hideDialog() {
if (dialog != null) { if (dialog != null) {
dialog.closeDialog(); dialog.setVisible(false);
} }
} }

View File

@ -69,7 +69,6 @@ public class FreeformFinSetConfig extends FinSetConfig {
private static final Logger log = LoggerFactory.getLogger(FreeformFinSetConfig.class); private static final Logger log = LoggerFactory.getLogger(FreeformFinSetConfig.class);
private static final Translator trans = Application.getTranslator(); private static final Translator trans = Application.getTranslator();
private final FreeformFinSet finset;
private JTable table = null; private JTable table = null;
private FinPointTableModel tableModel = null; private FinPointTableModel tableModel = null;
@ -80,7 +79,6 @@ public class FreeformFinSetConfig extends FinSetConfig {
public FreeformFinSetConfig(OpenRocketDocument d, RocketComponent component) { public FreeformFinSetConfig(OpenRocketDocument d, RocketComponent component) {
super(d, component); super(d, component);
this.finset = (FreeformFinSet) component;
//// General and General properties //// General and General properties
tabbedPane.insertTab(trans.get("FreeformFinSetCfg.tab.General"), null, generalPane(), trans.get("FreeformFinSetCfg.tab.ttip.General"), 0); tabbedPane.insertTab(trans.get("FreeformFinSetCfg.tab.General"), null, generalPane(), trans.get("FreeformFinSetCfg.tab.ttip.General"), 0);
@ -206,6 +204,7 @@ public class FreeformFinSetConfig extends FinSetConfig {
private JPanel shapePane() { private JPanel shapePane() {
JPanel panel = new JPanel(null); JPanel panel = new JPanel(null);
final FreeformFinSet finset = (FreeformFinSet)component;
// Create the figure // Create the figure
figure = new FinPointFigure(finset); figure = new FinPointFigure(finset);
@ -324,6 +323,8 @@ public class FreeformFinSetConfig extends FinSetConfig {
private void importImage() { private void importImage() {
final FreeformFinSet finset = (FreeformFinSet)component;
JFileChooser chooser = new JFileChooser(); JFileChooser chooser = new JFileChooser();
chooser.setFileFilter(FileHelper.getImageFileFilter()); chooser.setFileFilter(FileHelper.getImageFileFilter());
chooser.setFileSelectionMode(JFileChooser.FILES_ONLY); chooser.setFileSelectionMode(JFileChooser.FILES_ONLY);
@ -394,6 +395,8 @@ public class FreeformFinSetConfig extends FinSetConfig {
public void mousePressed(MouseEvent event) { public void mousePressed(MouseEvent event) {
int mods = event.getModifiersEx(); int mods = event.getModifiersEx();
final FreeformFinSet finset = (FreeformFinSet)component;
final int pressIndex = getPoint(event); final int pressIndex = getPoint(event);
if ( pressIndex >= 0) { if ( pressIndex >= 0) {
dragIndex = pressIndex; dragIndex = pressIndex;
@ -423,6 +426,8 @@ public class FreeformFinSetConfig extends FinSetConfig {
} }
Point2D.Double point = getCoordinates(event); Point2D.Double point = getCoordinates(event);
final FreeformFinSet finset = (FreeformFinSet)component;
finset.setPoint(dragIndex, point.x, point.y); finset.setPoint(dragIndex, point.x, point.y);
updateFields(); updateFields();
@ -443,6 +448,7 @@ public class FreeformFinSetConfig extends FinSetConfig {
// if ctrl+click, delete point // if ctrl+click, delete point
try { try {
Point2D.Double point = getCoordinates(event); Point2D.Double point = getCoordinates(event);
final FreeformFinSet finset = (FreeformFinSet)component;
finset.removePoint(clickIndex); finset.removePoint(clickIndex);
} catch (IllegalFinPointException ignore) { } catch (IllegalFinPointException ignore) {
log.error("Ignoring IllegalFinPointException while dragging, dragIndex=" + dragIndex + ". This is likely an internal error."); log.error("Ignoring IllegalFinPointException while dragging, dragIndex=" + dragIndex + ". This is likely an internal error.");
@ -527,12 +533,12 @@ public class FreeformFinSetConfig extends FinSetConfig {
@Override @Override
public int getRowCount() { public int getRowCount() {
return finset.getPointCount(); return ((FreeformFinSet)component).getPointCount();
} }
@Override @Override
public Object getValueAt(int rowIndex, int columnIndex) { public Object getValueAt(int rowIndex, int columnIndex) {
return Columns.values()[columnIndex].getValue(finset, rowIndex); return Columns.values()[columnIndex].getValue( (FreeformFinSet)component, rowIndex);
} }
@Override @Override
@ -554,6 +560,8 @@ public class FreeformFinSetConfig extends FinSetConfig {
if (!(o instanceof String)) if (!(o instanceof String))
return; return;
final FreeformFinSet finset = (FreeformFinSet)component;
// bounds check that indices are valid // bounds check that indices are valid
if (rowIndex < 0 || rowIndex >= finset.getFinPoints().length || columnIndex < 0 || columnIndex >= Columns.values().length) { if (rowIndex < 0 || rowIndex >= finset.getFinPoints().length || columnIndex < 0 || columnIndex >= Columns.values().length) {
throw new IllegalArgumentException("Index out of bounds, row=" + rowIndex + " column=" + columnIndex + " fin point count=" + finset.getFinPoints().length); throw new IllegalArgumentException("Index out of bounds, row=" + rowIndex + " column=" + columnIndex + " fin point count=" + finset.getFinPoints().length);

View File

@ -3,10 +3,7 @@ package net.sf.openrocket.gui.configdialog;
import java.awt.Component; import java.awt.Component;
import java.awt.Container; import java.awt.Container;
import java.awt.event.ActionEvent; import java.awt.event.*;
import java.awt.event.ActionListener;
import java.awt.event.FocusEvent;
import java.awt.event.FocusListener;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
@ -43,11 +40,8 @@ import net.sf.openrocket.l10n.Translator;
import net.sf.openrocket.material.Material; import net.sf.openrocket.material.Material;
import net.sf.openrocket.preset.ComponentPreset; import net.sf.openrocket.preset.ComponentPreset;
import net.sf.openrocket.rocketcomponent.ComponentAssembly; import net.sf.openrocket.rocketcomponent.ComponentAssembly;
import net.sf.openrocket.rocketcomponent.ExternalComponent; import net.sf.openrocket.rocketcomponent.*;
import net.sf.openrocket.rocketcomponent.ExternalComponent.Finish; import net.sf.openrocket.rocketcomponent.ExternalComponent.Finish;
import net.sf.openrocket.rocketcomponent.Instanceable;
import net.sf.openrocket.rocketcomponent.NoseCone;
import net.sf.openrocket.rocketcomponent.RocketComponent;
import net.sf.openrocket.rocketcomponent.position.AxialMethod; import net.sf.openrocket.rocketcomponent.position.AxialMethod;
import net.sf.openrocket.startup.Application; import net.sf.openrocket.startup.Application;
import net.sf.openrocket.unit.UnitGroup; import net.sf.openrocket.unit.UnitGroup;
@ -111,11 +105,11 @@ public class RocketComponentConfig extends JPanel {
//// Override and Mass and CG override options //// Override and Mass and CG override options
tabbedPane.addTab(trans.get("RocketCompCfg.tab.Override"), null, overrideTab(), tabbedPane.addTab(trans.get("RocketCompCfg.tab.Override"), null, overrideTab(),
trans.get("RocketCompCfg.tab.MassandCGoverride")); trans.get("RocketCompCfg.tab.MassandCGoverride"));
if (component.isMassive()) if (component.isMassive()) {
//// Appearance options //// Appearance options
tabbedPane.addTab(trans.get("RocketCompCfg.tab.Appearance"), null, new AppearancePanel(document, component), tabbedPane.addTab(trans.get("RocketCompCfg.tab.Appearance"), null, new AppearancePanel(document, component),
"Appearance Tool Tip"); "Appearance Tool Tip");
}
//// Comment and Specify a comment for the component //// Comment and Specify a comment for the component
tabbedPane.addTab(trans.get("RocketCompCfg.tab.Comment"), null, commentTab(), tabbedPane.addTab(trans.get("RocketCompCfg.tab.Comment"), null, commentTab(),
@ -622,12 +616,12 @@ public class RocketComponentConfig extends JPanel {
this.invalidatables.add(model); this.invalidatables.add(model);
} }
public void invalidateModels() { public void invalidate() {
super.invalidate();
for (Invalidatable i : invalidatables) { for (Invalidatable i : invalidatables) {
i.invalidate(); i.invalidate();
} }
((ComponentPresetDatabase) Application.getComponentPresetDao()).removeChangeListener(presetModel); ((ComponentPresetDatabase) Application.getComponentPresetDao()).removeChangeListener(presetModel);
} }