Merge pull request #1688 from SiboVG/issue-1680

[#1680] Update flight config stagemap according to unique stage ID

core/src/net/sf/openrocket/rocketcomponent/AxialStage.java

@@ -8,8 +8,7 @@ import net.sf.openrocket.util.Coordinate;
 public class AxialStage extends ComponentAssembly implements FlightConfigurableComponent {
 	
 	private static final Translator trans = Application.getTranslator();
-	//private static final Logger log = LoggerFactory.getLogger(AxialStage.class);
+
-	
 	/** list of separations to be happening*/
 	protected FlightConfigurableParameterSet<StageSeparationConfiguration> separations;
 	/** number of stages */
@@ -93,7 +92,6 @@ public class AxialStage extends ComponentAssembly implements FlightConfigurableC
 		copy.separations = new FlightConfigurableParameterSet<StageSeparationConfiguration>(separations);
 		return copy;
 	}
-
 	
 	/** 
 	 * Stages may be positioned relative to other stages. In that case, this will set the stage number 

core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java

@@ -46,22 +46,23 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
 
 	private class StageFlags implements Cloneable {
 		public boolean active = true;
-		//public int prev = -1;
 		public int stageNumber = -1;
-		
+		public String stageId;
-		public StageFlags( int _num, boolean _active) {
+
+		public StageFlags(int _num, String stageId, boolean _active) {
 			this.stageNumber = _num;
+			this.stageId = stageId;
 			this.active = _active;
 		}
 		
 		@Override
 		public StageFlags clone(){
-			return new StageFlags( this.stageNumber, true);
+			return new StageFlags(this.stageNumber, this.stageId, true);
 		}
 	}
 	
 	/* Cached data */
-	final protected Map<Integer, StageFlags> stages = new HashMap<Integer, StageFlags>();	// Map of stage number to StageFlags of the corresponding stage
+	final protected Map<Integer, StageFlags> stages = new HashMap<>();	// Map of stage number to StageFlags of the corresponding stage
 	final protected Map<MotorConfigurationId, MotorConfiguration> motors = new HashMap<MotorConfigurationId, MotorConfiguration>();
 	final private Collection<MotorConfiguration> activeMotors = new ConcurrentLinkedQueue<MotorConfiguration>();
 	final private InstanceMap activeInstances = new InstanceMap();
@@ -130,7 +131,7 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
 
 	public void copyStages(FlightConfiguration other) {
 		for (StageFlags cur : other.stages.values())
-			stages.put(cur.stageNumber, new StageFlags(cur.stageNumber, cur.active));
+			stages.put(cur.stageNumber, new StageFlags(cur.stageNumber, cur.stageId, cur.active));
 		updateMotors();
 		updateActiveInstances();
 	}
@@ -372,7 +373,7 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
 	public List<AxialStage> getAllStages() {
 		List<AxialStage> stages = new ArrayList<>();
 		for (StageFlags flags : this.stages.values()) {
-			stages.add( rocket.getStage(flags.stageNumber));
+			stages.add( rocket.getStage(flags.stageId));
 		}
 		return stages;
 	}
@@ -382,7 +383,11 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
 		
 		for (StageFlags flags : this.stages.values()) {
 			if (isStageActive(flags.stageNumber)) {
-				activeStages.add( rocket.getStage(flags.stageNumber));
+				AxialStage stage = rocket.getStage(flags.stageId);
+				if (stage == null) {
+					continue;
+				}
+				activeStages.add(stage);
 			}
 		}
 		
@@ -450,14 +455,18 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
 	}
 	
 	private void updateStages() {
-		if (this.rocket.getStageCount() == this.stages.size()) {
+		Map<Integer, FlightConfiguration.StageFlags> stagesBackup = new HashMap<>(this.stages);
-			return;
-		}
-
 		this.stages.clear();
 		for (AxialStage curStage : this.rocket.getStageList()) {
 			if (curStage == null) continue;
-			StageFlags flagsToAdd = new StageFlags( curStage.getStageNumber(), true);
+			boolean active = true;
+			for (FlightConfiguration.StageFlags flag : stagesBackup.values()) {
+				if (flag.stageId.equals(curStage.getID())) {
+					active = flag.active;
+					break;
+				}
+			}
+			StageFlags flagsToAdd = new StageFlags(curStage.getStageNumber(), curStage.getID(), active);
 			this.stages.put(curStage.getStageNumber(), flagsToAdd);
 		}
 	}
@@ -842,8 +851,8 @@ public class FlightConfiguration implements FlightConfigurableParameter<FlightCo
 		final String fmt = "    [%-2s][%4s]: %6s \n";
 		buf.append(String.format(fmt, "#", "?actv", "Name"));
 		for (StageFlags flags : stages.values()) {
-			final int stageNumber = flags.stageNumber;
+			final String stageId = flags.stageId;
-			buf.append(String.format(fmt, stageNumber, (isStageActive(flags.stageNumber) ?" on": "off"), rocket.getStage( stageNumber).getName()));
+			buf.append(String.format(fmt, stageId, (flags.active?" on": "off"), rocket.getStage(stageId).getName()));
 		}
 		buf.append("\n");
 		return buf.toString();

core/src/net/sf/openrocket/rocketcomponent/Rocket.java

@@ -198,6 +198,15 @@ public class Rocket extends ComponentAssembly {
 		return this.stageMap.get( stageNumber);
 	}
 
+	public AxialStage getStage(final String stageId) {
+		for (AxialStage stage : getStageList()) {
+			if (stage.getID().equals(stageId)) {
+				return stage;
+			}
+		}
+		return null;
+	}
+
 	/**
 	 * Get the topmost stage, only taking into account active stages from the flight configuration.
 	 * @param config flight configuration dictating which stages are active
@@ -537,21 +546,17 @@ public class Rocket extends ComponentAssembly {
 	 * Update all the stage numbers based on their position in the component tree
 	 */
 	private void updateStageNumbers() {
-		for (RocketComponent component : getChildren()) {
+		int stageNr = 0;
-			if (component instanceof AxialStage) {
+		for (AxialStage stage : getSubStages()) {
-				AxialStage stage = (AxialStage) component;
+			forgetStage(stage);
-				forgetStage(stage);
+			stage.setStageNumber(stageNr);
-				stage.setStageNumber(getChildPosition(stage));
+			stageNr++;
-			}
 		}
 	}
 
 	private void updateStageMap(){
-		for( RocketComponent component : getChildren() ){
+		for (AxialStage stage : getSubStages() ){
-			if (component instanceof AxialStage) {
+			trackStage(stage);
-				AxialStage stage = (AxialStage) component;
-				trackStage(stage);
-			}
 		}
 	}
 	

core/test/net/sf/openrocket/rocketcomponent/AxialStageTest.java

@@ -0,0 +1,225 @@
+package net.sf.openrocket.rocketcomponent;
+
+import net.sf.openrocket.util.BaseTestCase.BaseTestCase;
+import net.sf.openrocket.util.TestRockets;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AxialStageTest extends BaseTestCase {
+    /**
+     * Test whether we can disable a stage, and that it is registered only in the target flight configuration and not
+     * in the others.
+     */
+    @Test
+    public void testDisableStage() {
+        final Rocket rocket = TestRockets.makeFalcon9Heavy();
+        final FlightConfiguration config = rocket.getSelectedConfiguration();
+        final FlightConfigurationId fcid = rocket.createFlightConfiguration(new FlightConfigurationId());
+        final FlightConfiguration config2 = rocket.getFlightConfiguration(fcid);
+
+        // Disable the payload stage
+        config._setStageActive(0, false);
+        assertFalse(" Payload stage in selected configuration should be disabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Enable the payload stage
+        config._setStageActive(0, true);
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Toggle the payload stage to False
+        config.toggleStage(0);
+        assertFalse(" Payload stage in selected configuration should be disabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Toggle the payload stage to True
+        config.toggleStage(0);
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Set only stage
+        config.setOnlyStage(1);
+        assertFalse(" Payload stage in selected configuration should be disabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertFalse(" Booster stage in selected configuration should be disabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Change stage activeness in other configuration
+        config2.toggleStage(1);
+        assertFalse(" Payload stage in selected configuration should be disabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertFalse(" Booster stage in selected configuration should be disabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertFalse(" Core stage in other configuration should be disabled", config2.isStageActive(1));
+        assertFalse(" Booster stage in other configuration should be disabled", config2.isStageActive(2));
+        config.setAllStages();
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertFalse(" Core stage in other configuration should be disabled", config2.isStageActive(1));
+        assertFalse(" Booster stage in other configuration should be disabled", config2.isStageActive(2));
+
+        // Toggle stage with activateSubStages edited
+        config.setAllStages();
+        config2.setAllStages();
+        config._setStageActive(1, false, true);
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertFalse(" Core stage in selected configuration should be disabled", config.isStageActive(1));
+        assertFalse(" Booster stage in selected configuration should be disabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+        config._setStageActive(1, true, false);
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertFalse(" Booster stage in selected configuration should be disabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+    }
+
+    /**
+     * Test disabling a stage and then moving the stage in the component tree.
+     */
+    @Test
+    public void testDisableStageAndMove() {
+        final Rocket rocket = TestRockets.makeFalcon9Heavy();
+        final FlightConfiguration config = rocket.getSelectedConfiguration();
+        final FlightConfigurationId fcid = rocket.createFlightConfiguration(new FlightConfigurationId());
+        final FlightConfiguration config2 = rocket.getFlightConfiguration(fcid);
+
+        // Disable the payload stage
+        config.setAllStages();
+        config._setStageActive(0, false);
+        AxialStage payloadStage = rocket.getStage(0);
+
+        // Move the payload stage to the back of the rocket
+        try {
+            rocket.freeze();
+            rocket.removeChild(payloadStage);
+            rocket.addChild(payloadStage);      // Moves to the back
+        } finally {
+            rocket.thaw();	// Unfreeze
+        }
+
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(0));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(1));
+        assertFalse(" Payload stage in selected configuration should be disabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Re-enable the payload stage
+        config._setStageActive(payloadStage.getStageNumber(), true);
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(0));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(1));
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Disable the core stage (and booster stage)
+        config._setStageActive(0, false);
+        assertFalse(" Core stage in selected configuration should be disabled", config.isStageActive(0));
+        assertFalse(" Booster stage in selected configuration should be disabled ", config.isStageActive(1));
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+
+        // Move the core stage to the back of the rocket
+        AxialStage coreStage = rocket.getStage(0);
+        try {
+            rocket.freeze();
+            rocket.removeChild(coreStage);
+            rocket.addChild(coreStage);      // Moves to the back
+        } finally {
+            rocket.thaw();	// Unfreeze
+        }
+
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertFalse(" Core stage in selected configuration should be disabled", config.isStageActive(1));
+        assertFalse(" Booster stage in selected configuration should be disabled ", config.isStageActive(2));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+    }
+
+    /**
+     * Test disabling a stage and then copying that stage.
+     */
+    @Test
+    public void testDisableStageAndCopy() {
+        final Rocket rocket = TestRockets.makeFalcon9Heavy();
+        final FlightConfiguration config = rocket.getSelectedConfiguration();
+        final FlightConfigurationId fcid = rocket.createFlightConfiguration(new FlightConfigurationId());
+        final FlightConfiguration config2 = rocket.getFlightConfiguration(fcid);
+
+        // Disable the core stage
+        config.setAllStages();
+        config._setStageActive(1, false);
+        AxialStage coreStage = rocket.getStage(1);
+
+        // Copy the core stage to the back of the rocket
+        AxialStage coreStageCopy = (AxialStage) coreStage.copy();
+        rocket.addChild(coreStageCopy);
+
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertFalse(" Core stage in selected configuration should be disabled", config.isStageActive(1));
+        assertFalse(" Booster stage in selected configuration should be disabled", config.isStageActive(2));
+        assertTrue(" Core copy stage in selected configuration should be enabled", config.isStageActive(3));
+        assertTrue(" Booster copy stage in selected configuration should be enabled", config.isStageActive(4));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+        assertTrue(" Core copy stage in selected configuration should be enabled", config2.isStageActive(3));
+        assertTrue(" Booster copy stage in selected configuration should be enabled", config2.isStageActive(4));
+
+        // Disable the copied core stage (not the booster copy stage)
+        config._setStageActive(3, false, false);
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertFalse(" Core stage in selected configuration should be disabled", config.isStageActive(1));
+        assertFalse(" Booster stage in selected configuration should be disabled", config.isStageActive(2));
+        assertFalse(" Core copy stage in selected configuration should be disabled", config.isStageActive(3));
+        assertTrue(" Booster copy stage in selected configuration should be enabled", config.isStageActive(4));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+        assertTrue(" Core copy stage in selected configuration should be enabled", config2.isStageActive(3));
+        assertTrue(" Booster copy stage in selected configuration should be enabled", config2.isStageActive(4));
+
+        // Toggle the original core stage back
+        config.toggleStage(1);
+        assertTrue(" Payload stage in selected configuration should be enabled", config.isStageActive(0));
+        assertTrue(" Core stage in selected configuration should be enabled", config.isStageActive(1));
+        assertTrue(" Booster stage in selected configuration should be enabled", config.isStageActive(2));
+        assertFalse(" Core copy stage in selected configuration should be disabled", config.isStageActive(3));
+        assertTrue(" Booster copy stage in selected configuration should be enabled", config.isStageActive(4));
+        assertTrue(" Payload stage in other configuration should be enabled", config2.isStageActive(0));
+        assertTrue(" Core stage in other configuration should be enabled", config2.isStageActive(1));
+        assertTrue(" Booster stage in other configuration should be enabled", config2.isStageActive(2));
+        assertTrue(" Core copy stage in selected configuration should be enabled", config2.isStageActive(3));
+        assertTrue(" Booster copy stage in selected configuration should be enabled", config2.isStageActive(4));
+    }
+}

core/test/net/sf/openrocket/rocketcomponent/ParallelStageTest.java

@@ -644,15 +644,15 @@ public class ParallelStageTest extends BaseTestCase {
 
 		expectedStageNumber = 2;
 		actualStageNumber = boosterA.getStageNumber();
-		assertEquals(" init order error: core: resultant positions: ", expectedStageNumber, actualStageNumber);
+		assertEquals(" init order error: Booster A: resultant positions: ", expectedStageNumber, actualStageNumber);
 
 		expectedStageNumber = 3;
 		actualStageNumber = boosterB.getStageNumber();
-		assertEquals(" init order error: Booster A: resultant positions: ", expectedStageNumber, actualStageNumber);
+		assertEquals(" init order error: Booster B: resultant positions: ", expectedStageNumber, actualStageNumber);
 		
 		expectedStageNumber = 4;
 		actualStageNumber = boosterC.getStageNumber();
-		assertEquals(" init order error: Booster B: resultant positions: ", expectedStageNumber, actualStageNumber);
+		assertEquals(" init order error: Booster C: resultant positions: ", expectedStageNumber, actualStageNumber);
 		
 		// remove Booster B
 		coreBody.removeChild(1);
@@ -666,11 +666,11 @@ public class ParallelStageTest extends BaseTestCase {
 		assertEquals(" Stage tracking error:  removed booster A, but configuration not updated: " + treedump, expectedStageCount, actualStageCount);
 		
 		ParallelStage boosterD = createExtraBooster();
-		boosterC.setName("Booster D Stage");
+		boosterD.setName("Booster D Stage");
 		coreBody.addChild(boosterD);
-		boosterC.setAxialOffset(AxialMethod.BOTTOM, 0);
+		boosterD.setAxialOffset(AxialMethod.BOTTOM, 0);
 		
-		expectedStageNumber = 3;
+		expectedStageNumber = 4;
 		actualStageNumber = boosterD.getStageNumber();
 		assertEquals(" init order error: Booster D: resultant positions: ", expectedStageNumber, actualStageNumber);