[Bugfix] Cleaned up event-handling, esp. with active stages & graphics

- added new ComponentChangeEvent type: GRAPHIC
  - does not change the rocket structure, nor behavior, but requires updates to the UI
- removed event handling from FlightConfiguration
  - caused a circular event loop
  - also, collected too many responsibilities in the class (bad code smell)
- minor refactor of event handling in Rocket. (no change in behavior)
- Fixed StageSelector behavior
  - should now pull selected configuration from rocket
  - should now correctly notify rocket (and indirectly, the graphics)
This commit is contained in:
Daniel_M_Williams 2016-01-23 20:03:53 -05:00
parent 39420ddfc0
commit fbd40859a5
8 changed files with 68 additions and 61 deletions

View File

@ -14,7 +14,9 @@ public class ComponentChangeEvent extends EventObject {
UNDO( 16, "UNDO"), UNDO( 16, "UNDO"),
MOTOR( 32, "Motor"), MOTOR( 32, "Motor"),
EVENT( 64, "Event"), EVENT( 64, "Event"),
TEXTURE ( 128, "Texture"); TEXTURE ( 128, "Texture")
, GRAPHIC( 256, "Configuration")
;
protected int value; protected int value;
protected String name; protected String name;
@ -39,8 +41,7 @@ public class ComponentChangeEvent extends EventObject {
/** A change that affects the mass and aerodynamic properties of the rocket */ /** A change that affects the mass and aerodynamic properties of the rocket */
public static final int AEROMASS_CHANGE = (TYPE.MASS.value | TYPE.AERODYNAMIC.value ); public static final int AEROMASS_CHANGE = (TYPE.MASS.value | TYPE.AERODYNAMIC.value );
public static final int BOTH_CHANGE = AEROMASS_CHANGE; // syntactic sugar / backward compatibility public static final int BOTH_CHANGE = AEROMASS_CHANGE; // syntactic sugar / backward compatibility
/** when a flight configuration fires an event, it is of this type */
public static final int CONFIG_CHANGE = (TYPE.MASS.value | TYPE.AERODYNAMIC.value | TYPE.TREE.value | TYPE.MOTOR.value | TYPE.EVENT.value);
/** A change that affects the rocket tree structure */ /** A change that affects the rocket tree structure */
public static final int TREE_CHANGE = TYPE.TREE.value; public static final int TREE_CHANGE = TYPE.TREE.value;
@ -52,6 +53,9 @@ public class ComponentChangeEvent extends EventObject {
public static final int EVENT_CHANGE = TYPE.EVENT.value; public static final int EVENT_CHANGE = TYPE.EVENT.value;
/** A change to the 3D texture assigned to a component*/ /** A change to the 3D texture assigned to a component*/
public static final int TEXTURE_CHANGE = TYPE.TEXTURE.value; public static final int TEXTURE_CHANGE = TYPE.TEXTURE.value;
// when a flight configuration fires an event, it is of this type
// UI-only change, but does not effect the true
public static final int GRAPHIC_CHANGE = TYPE.GRAPHIC.value;
//// A bit-field that contains all possible change types. //// A bit-field that contains all possible change types.
//// Will output as -1. for an explanation, see "twos-complement" representation of signed integers //// Will output as -1. for an explanation, see "twos-complement" representation of signed integers

View File

@ -122,7 +122,7 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
* @param stageNumber stage number to inactivate * @param stageNumber stage number to inactivate
*/ */
public void clearStage(final int stageNumber) { public void clearStage(final int stageNumber) {
setStageActive( stageNumber, false, true); setStageActive( stageNumber, false );
} }
/** /**
@ -132,7 +132,7 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
*/ */
public void setOnlyStage(final int stageNumber) { public void setOnlyStage(final int stageNumber) {
setAllStages(false, false); setAllStages(false, false);
setStageActive(stageNumber, true, true); setStageActive(stageNumber, true);
} }
/** /**
@ -141,16 +141,9 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
* @param stageNumber stage number to flag * @param stageNumber stage number to flag
* @param _active inactive (<code>false</code>) or active (<code>true</code>) * @param _active inactive (<code>false</code>) or active (<code>true</code>)
*/ */
public void setStageActive(final int stageNumber, final boolean _active ) { private void setStageActive(final int stageNumber, final boolean _active ) {
this.setStageActive(stageNumber, _active, true );
}
private void setStageActive(final int stageNumber, final boolean _active, final boolean updateRequired ) {
if ((0 <= stageNumber) && (stages.containsKey(stageNumber))) { if ((0 <= stageNumber) && (stages.containsKey(stageNumber))) {
stages.get(stageNumber).active = _active; stages.get(stageNumber).active = _active;
if( updateRequired ){
update();
}
return; return;
} }
log.error("error: attempt to retrieve via a bad stage number: " + stageNumber); log.error("error: attempt to retrieve via a bad stage number: " + stageNumber);
@ -396,6 +389,7 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
updateStages(); updateStages();
updateMotors(); updateMotors();
} }
/////////////// Helper methods /////////////// /////////////// Helper methods ///////////////
/** /**

View File

@ -423,6 +423,10 @@ public class Rocket extends RocketComponent {
try { try {
checkState(); checkState();
{ // vvvv DEVEL vvvv
System.err.println("fireEvent@rocket.");
} // ^^^^ DEVEL ^^^^
// Update modification ID's only for normal (not undo/redo) events // Update modification ID's only for normal (not undo/redo) events
if (!cce.isUndoChange()) { if (!cce.isUndoChange()) {
modID = UniqueID.next(); modID = UniqueID.next();
@ -436,18 +440,6 @@ public class Rocket extends RocketComponent {
functionalModID = modID; functionalModID = modID;
} }
// Update modification ID's only for normal (not undo/redo) events
{ // vvvv DEVEL vvvv
// String changeString;
// if (cce.isUndoChange()) {
// changeString = "an 'undo' change from: "+cce.getSource().getName()+" as:"+cce.toString();
// }else{
// changeString = "a normal change from: "+cce.getSource().getName()+" as:"+cce.toString();
// }
//
// log.error("Processing a rocket change: "+changeString, new IllegalArgumentException());
} // ^^^^ DEVEL ^^^^
// Check whether frozen // Check whether frozen
if (freezeList != null) { if (freezeList != null) {
log.debug("Rocket is in frozen state, adding event " + cce + " info freeze list"); log.debug("Rocket is in frozen state, adding event " + cce + " info freeze list");
@ -461,10 +453,29 @@ public class Rocket extends RocketComponent {
iterator.next().componentChanged(cce); iterator.next().componentChanged(cce);
} }
// notify all configurations updateConfigurations();
this.update();
// Notify all listeners notifyAllListeners(cce);
} finally {
mutex.unlock("fireComponentChangeEvent");
}
}
@Override
public void update(){
updateConfigurations();
}
private void updateConfigurations(){
this.selectedConfiguration.update();
for( FlightConfiguration config : configSet.values() ){
config.update();
}
}
private void notifyAllListeners(final ComponentChangeEvent cce){
// Copy the list before iterating to prevent concurrent modification exceptions. // Copy the list before iterating to prevent concurrent modification exceptions.
EventListener[] list = listenerList.toArray(new EventListener[0]); EventListener[] list = listenerList.toArray(new EventListener[0]);
for (EventListener l : list) { for (EventListener l : list) {
@ -474,17 +485,6 @@ public class Rocket extends RocketComponent {
((StateChangeListener) l).stateChanged(cce); ((StateChangeListener) l).stateChanged(cce);
} }
} }
} finally {
mutex.unlock("fireComponentChangeEvent");
}
}
@Override
public void update(){
this.selectedConfiguration.update();
for( FlightConfiguration config : configSet.values() ){
config.update();
}
} }
/** /**

View File

@ -1747,7 +1747,7 @@ public abstract class RocketComponent implements ChangeSource, Cloneable, Iterab
* @param type Type of event * @param type Type of event
* @see #fireComponentChangeEvent(ComponentChangeEvent) * @see #fireComponentChangeEvent(ComponentChangeEvent)
*/ */
protected void fireComponentChangeEvent(int type) { public void fireComponentChangeEvent(int type) {
fireComponentChangeEvent(new ComponentChangeEvent(this, type)); fireComponentChangeEvent(new ComponentChangeEvent(this, type));
} }

View File

@ -12,29 +12,30 @@ import javax.swing.JToggleButton;
import net.miginfocom.swing.MigLayout; import net.miginfocom.swing.MigLayout;
import net.sf.openrocket.l10n.Translator; import net.sf.openrocket.l10n.Translator;
import net.sf.openrocket.rocketcomponent.AxialStage; import net.sf.openrocket.rocketcomponent.AxialStage;
import net.sf.openrocket.rocketcomponent.ComponentChangeEvent;
import net.sf.openrocket.rocketcomponent.FlightConfiguration; import net.sf.openrocket.rocketcomponent.FlightConfiguration;
import net.sf.openrocket.rocketcomponent.Rocket;
import net.sf.openrocket.startup.Application; import net.sf.openrocket.startup.Application;
import net.sf.openrocket.util.StateChangeListener; import net.sf.openrocket.util.StateChangeListener;
@SuppressWarnings("serial")
public class StageSelector extends JPanel implements StateChangeListener { public class StageSelector extends JPanel implements StateChangeListener {
private static final long serialVersionUID = -2898763402479628711L;
private static final Translator trans = Application.getTranslator(); private static final Translator trans = Application.getTranslator();
private final FlightConfiguration configuration; private final Rocket rocket;
private List<JToggleButton> buttons = new ArrayList<JToggleButton>(); private List<JToggleButton> buttons = new ArrayList<JToggleButton>();
public StageSelector(FlightConfiguration configuration) { public StageSelector(Rocket _rkt) {
super(new MigLayout("gap 0!")); super(new MigLayout("gap 0!"));
this.configuration = configuration; this.rocket = _rkt;
updateButtons();
updateButtons( this.rocket.getSelectedConfiguration() );
} }
private void updateButtons() { private void updateButtons( final FlightConfiguration configuration ) {
int stages = configuration.getStageCount(); int stages = configuration.getStageCount();
if (buttons.size() == stages) if (buttons.size() == stages)
return; return;
@ -44,6 +45,7 @@ public class StageSelector extends JPanel implements StateChangeListener {
for(AxialStage stage : configuration.getRocket().getStageList()){ for(AxialStage stage : configuration.getRocket().getStageList()){
int stageNum = stage.getStageNumber(); int stageNum = stage.getStageNumber();
JToggleButton button = new JToggleButton(new StageAction(stageNum)); JToggleButton button = new JToggleButton(new StageAction(stageNum));
button.setSelected(true);
this.add(button); this.add(button);
buttons.add(button); buttons.add(button);
} }
@ -51,20 +53,20 @@ public class StageSelector extends JPanel implements StateChangeListener {
this.revalidate(); this.revalidate();
} }
@Override @Override
public void stateChanged(EventObject e) { public void stateChanged(EventObject eo) {
updateButtons(); Object source = eo.getSource();
if( source instanceof Rocket ){
Rocket rkt = (Rocket) eo.getSource();
updateButtons( rkt.getSelectedConfiguration() );
}
} }
private class StageAction extends AbstractAction {
private class StageAction extends AbstractAction implements StateChangeListener {
private static final long serialVersionUID = 7433006728984943763L;
private final int stageNumber; private final int stageNumber;
public StageAction(final int stage) { public StageAction(final int stage) {
this.stageNumber = stage; this.stageNumber = stage;
stateChanged(null);
} }
@Override @Override
@ -78,12 +80,9 @@ public class StageSelector extends JPanel implements StateChangeListener {
@Override @Override
public void actionPerformed(ActionEvent e) { public void actionPerformed(ActionEvent e) {
configuration.toggleStage(stageNumber); rocket.getSelectedConfiguration().toggleStage(stageNumber);
rocket.fireComponentChangeEvent(ComponentChangeEvent.GRAPHIC_CHANGE);
} }
@Override
public void stateChanged(EventObject e) {
this.putValue(SELECTED_KEY, configuration.isStageActive(stageNumber));
}
} }
} }

View File

@ -166,11 +166,11 @@ public class ComponentAnalysisDialog extends JDialog implements StateChangeListe
panel.add(new BasicSlider(roll.getSliderModel(-20 * 2 * Math.PI, 20 * 2 * Math.PI)), panel.add(new BasicSlider(roll.getSliderModel(-20 * 2 * Math.PI, 20 * 2 * Math.PI)),
"growx, wrap"); "growx, wrap");
// Stage and motor selection: // Stage and motor selection:
//// Active stages: //// Active stages:
panel.add(new JLabel(trans.get("componentanalysisdlg.lbl.activestages")), "spanx, split, gapafter rel"); panel.add(new JLabel(trans.get("componentanalysisdlg.lbl.activestages")), "spanx, split, gapafter rel");
panel.add(new StageSelector(configuration), "gapafter paragraph"); Rocket rkt = rocketPanel.getDocument().getRocket();
panel.add(new StageSelector( rkt), "gapafter paragraph");
//// Motor configuration: //// Motor configuration:
JLabel label = new JLabel(trans.get("componentanalysisdlg.lbl.motorconf")); JLabel label = new JLabel(trans.get("componentanalysisdlg.lbl.motorconf"));

View File

@ -664,6 +664,7 @@ public class RocketActions {
RocketComponent parent = selected.getParent(); RocketComponent parent = selected.getParent();
document.addUndoPosition("Move "+selected.getComponentName()); document.addUndoPosition("Move "+selected.getComponentName());
parent.moveChild(selected, parent.getChildPosition(selected)-1); parent.moveChild(selected, parent.getChildPosition(selected)-1);
rocket.fireComponentChangeEvent( ComponentChangeEvent.TREE_CHANGE );
selectionModel.setSelectedComponent(selected); selectionModel.setSelectedComponent(selected);
} }
@ -709,6 +710,7 @@ public class RocketActions {
RocketComponent parent = selected.getParent(); RocketComponent parent = selected.getParent();
document.addUndoPosition("Move "+selected.getComponentName()); document.addUndoPosition("Move "+selected.getComponentName());
parent.moveChild(selected, parent.getChildPosition(selected)+1); parent.moveChild(selected, parent.getChildPosition(selected)+1);
rocket.fireComponentChangeEvent( ComponentChangeEvent.TREE_CHANGE );
selectionModel.setSelectedComponent(selected); selectionModel.setSelectedComponent(selected);
} }

View File

@ -146,6 +146,7 @@ public class RocketPanel extends JPanel implements TreeSelectionListener, Change
private List<EventListener> listeners = new ArrayList<EventListener>(); private List<EventListener> listeners = new ArrayList<EventListener>();
/** /**
* The executor service used for running the background simulations. * The executor service used for running the background simulations.
* This uses a fixed-sized thread pool for all background simulations * This uses a fixed-sized thread pool for all background simulations
@ -167,6 +168,11 @@ public class RocketPanel extends JPanel implements TreeSelectionListener, Change
}); });
} }
public OpenRocketDocument getDocument(){
return this.document;
}
public RocketPanel(OpenRocketDocument document) { public RocketPanel(OpenRocketDocument document) {
this.document = document; this.document = document;
Rocket rkt = document.getRocket(); Rocket rkt = document.getRocket();
@ -295,8 +301,9 @@ public class RocketPanel extends JPanel implements TreeSelectionListener, Change
add(scaleSelector); add(scaleSelector);
// Stage selector // Stage selector
final FlightConfiguration configuration = document.getDefaultConfiguration(); final Rocket rkt = document.getRocket();
StageSelector stageSelector = new StageSelector(configuration); StageSelector stageSelector = new StageSelector( rkt );
rkt.addChangeListener(stageSelector);
add(stageSelector); add(stageSelector);
// Flight configuration selector // Flight configuration selector
@ -847,4 +854,5 @@ public class RocketPanel extends JPanel implements TreeSelectionListener, Change
// putValue(Action.SELECTED_KEY, figure.getType() == type && !is3d); // putValue(Action.SELECTED_KEY, figure.getType() == type && !is3d);
// } // }
// } // }
} }