Merge pull request #2563 from SiboVG/issue-2463

[#2463] Fix shoulder radius scaling issue when smaller than 1 + fin tab length/offset scaling

core/src/main/java/info/openrocket/core/rocketcomponent/FinSet.java

@@ -311,6 +311,10 @@ public abstract class FinSet extends ExternalComponent
 	 * 
 	 */
 	public void setTabHeight(final double newTabHeight) {
+		setTabHeight(newTabHeight, true);
+	}
+
+	public void setTabHeight(double newTabHeight, boolean validateTabHeight) {
 		for (RocketComponent listener : configListeners) {
 			if (listener instanceof FinSet) {
 				((FinSet) listener).setTabHeight(newTabHeight);
@@ -322,8 +326,11 @@ public abstract class FinSet extends ExternalComponent
 		}
 
 		tabHeight = newTabHeight;
-		double maxTabHeight = getMaxTabHeight();
+		if (validateTabHeight) {
-		this.tabHeight = Math.min(this.tabHeight,  maxTabHeight);
+			double maxTabHeight = getMaxTabHeight();
+			this.tabHeight = Math.min(this.tabHeight, maxTabHeight);
+		}
+
 		fireComponentChangeEvent(ComponentChangeEvent.MASS_CHANGE);
 	}
 	
@@ -336,6 +343,10 @@ public abstract class FinSet extends ExternalComponent
 	 * set tab length
 	 */
 	public void setTabLength(final double lengthRequest) {
+		setTabLength(lengthRequest, true);
+	}
+
+	public void setTabLength(final double lengthRequest, boolean updateTabPosition) {
 		for (RocketComponent listener : configListeners) {
 			if (listener instanceof FinSet) {
 				((FinSet) listener).setTabLength(lengthRequest);
@@ -348,12 +359,14 @@ public abstract class FinSet extends ExternalComponent
 
 		tabLength = lengthRequest;
 
-		updateTabPosition();
+		if (updateTabPosition) {
+			updateTabPosition();
+		}
 
 		fireComponentChangeEvent(ComponentChangeEvent.MASS_CHANGE);
 	}
 
-	public void updateTabPosition(){
+	public void updateTabPosition() {
 		this.tabPosition = this.tabOffsetMethod.getAsPosition(tabOffset, tabLength, length);
 	}
 	
@@ -362,7 +375,7 @@ public abstract class FinSet extends ExternalComponent
 	 * 
 	 * @param offsetRequest new requested tab offset
 	 */
-	public void setTabOffset( final double offsetRequest) {
+	public void setTabOffset(final double offsetRequest) {
 		for (RocketComponent listener : configListeners) {
 			if (listener instanceof FinSet) {
 				((FinSet) listener).setTabOffset(offsetRequest);
@@ -375,6 +388,18 @@ public abstract class FinSet extends ExternalComponent
 		fireComponentChangeEvent(ComponentChangeEvent.MASS_CHANGE);
 	}
 
+	public double getTabOffset(AxialMethod method) {
+		return method.getAsOffset(tabPosition, tabLength, length);
+	}
+
+	public double getTabOffset() {
+		return getTabOffset(this.tabOffsetMethod);
+	}
+
+	public double getTabPosition(AxialMethod method) {
+		return method.getAsPosition(tabOffset, tabLength, length);
+	}
+	
 	public AxialMethod getTabOffsetMethod() {
 		return tabOffsetMethod;
 	}
@@ -403,18 +428,6 @@ public abstract class FinSet extends ExternalComponent
 		return tabPosition;
 	}
 
-	public double getTabOffset(AxialMethod method){
-		return method.getAsOffset(tabPosition, tabLength, length);
-	}
-
-	public double getTabOffset(){
-		return getTabOffset(this.tabOffsetMethod);
-	}
-
-	public double getTabPosition(AxialMethod method) {
-		return method.getAsPosition(tabOffset, tabLength, length);
-	}
-
 	/**
 	 * Return the tab trailing edge position *from the front of the fin*.
 	 */
@@ -435,16 +448,12 @@ public abstract class FinSet extends ExternalComponent
 	}
 	
 	public void validateFinTabLength() {
-		//System.err.println(String.format("    >> Fin Tab Length: %.6f @ %.6f", tabLength, tabOffset));
-		
 		final double xTabBack = getTabTrailingEdge();
 		if (this.length < xTabBack) {
 			this.tabLength -= (xTabBack - this.length);
 		}
 		
 		tabLength = Math.max(0, tabLength);
-		
-		//System.err.println(String.format("    << Fin Tab Length: %.6f @ %.6f", tabLength, tabOffset));
 	}
 
 	/**

core/src/main/java/info/openrocket/core/rocketcomponent/FreeformFinSet.java

@@ -242,13 +242,17 @@ public class FreeformFinSet extends FinSet {
 	/** maintained just for backwards compatibility:
 	 */
 	public void setPoints(Coordinate[] newPoints) {
+		setPoints(newPoints, true);
+	}
+
+	public void setPoints(Coordinate[] newPoints, boolean validateFinTab) {
 		for (RocketComponent listener : configListeners) {
 			if (listener instanceof FreeformFinSet) {
 				((FreeformFinSet) listener).setPoints(newPoints);
 			}
 		}
 
-		setPoints(new ArrayList<>(Arrays.asList(newPoints)));
+		setPoints(new ArrayList<>(Arrays.asList(newPoints)), validateFinTab);
 	}
 	
 	/**
@@ -256,7 +260,11 @@ public class FreeformFinSet extends FinSet {
 	 * 
 	 * @param newPoints New points to set as the exposed edges of the fin
 	 */
-	public void setPoints( ArrayList<Coordinate> newPoints) {
+	public void setPoints(ArrayList<Coordinate> newPoints) {
+		setPoints(newPoints, true);
+	}
+
+	public void setPoints( ArrayList<Coordinate> newPoints, boolean validateFinTab) {
 		for (RocketComponent listener : configListeners) {
 			if (listener instanceof FreeformFinSet) {
 				((FreeformFinSet) listener).setPoints(newPoints);
@@ -264,16 +272,16 @@ public class FreeformFinSet extends FinSet {
 		}
 
 		final Coordinate delta = newPoints.get(0).multiply(-1);
-		if( IGNORE_SMALLER_THAN < delta.length2()){
+		if (IGNORE_SMALLER_THAN < delta.length2()) {
-			newPoints = translatePoints( newPoints, delta);
+			newPoints = translatePoints(newPoints, delta);
 		}
 
-		for ( int i =0; i < newPoints.size(); ++i ) {
+		for (int i = 0; i < newPoints.size(); ++i) {
 			final Coordinate p = newPoints.get(i);
-			if( p.x > SNAP_LARGER_THAN){
+			if (p.x > SNAP_LARGER_THAN) {
 				newPoints.set(i, p.setX(SNAP_LARGER_THAN));
 			}
-			if( p.y > SNAP_LARGER_THAN){
+			if (p.y > SNAP_LARGER_THAN) {
 				newPoints.set(i, p.setY(SNAP_LARGER_THAN));
 			}
 		}
@@ -284,9 +292,9 @@ public class FreeformFinSet extends FinSet {
 
 		this.points = newPoints;
 
-		update();
+		update(validateFinTab);
 
-		if( intersects()){
+		if (intersects()) {
 			// on error, reset to the old points
 			this.points = pointsCopy;
 			this.length = lengthCopy;
@@ -432,21 +440,26 @@ public class FreeformFinSet extends FinSet {
 
 	@Override
 	public void update() {
+		update(true);
+	}
+
+	public void update(boolean validateFinTab) {
 		final double oldLength = this.length;
 		this.length = points.get(points.size() -1).x - points.get(0).x;
 		this.setAxialOffset(this.axialMethod, this.axialOffset);
 
-		if(null != this.getParent()) {
+		if (this.getParent() != null) {
 			clampFirstPoint();
 
-			for(int i=1; i < points.size()-1; i++) {
+			for (int i=1; i < points.size()-1; i++) {
 				clampInteriorPoint(i);
 			}
 
 			clampLastPoint();
 
-			if (oldLength != this.length)
+			if (oldLength != this.length && validateFinTab) {
 				validateFinTabLength();
+			}
 		}
 	}
 

core/src/main/java/info/openrocket/core/rocketcomponent/NoseCone.java

@@ -130,6 +130,19 @@ public class NoseCone extends Transition implements InsideColorComponent {
 		return isFlipped ? getForeShoulderRadius() : getAftShoulderRadius();
 	}
 
+	/**
+	 * Sets the shoulder radius (independent of whether the nose cone is flipped or not).
+	 * @param radius the new shoulder radius
+	 * @param doClamping whether to clamp the shoulder radius to the nose cone radius
+	 */
+	public void setShoulderRadius(double radius, boolean doClamping) {
+		if (isFlipped) {
+			setForeShoulderRadius(radius, doClamping);
+		} else {
+			setAftShoulderRadius(radius, doClamping);
+		}
+	}
+
 	/**
 	 * Sets the shoulder radius (independent of whether the nose cone is flipped or
 	 * not).

swing/src/main/java/info/openrocket/swing/gui/dialogs/ScaleDialog.java

@@ -47,6 +47,8 @@ import info.openrocket.core.rocketcomponent.SymmetricComponent;
 import info.openrocket.core.rocketcomponent.ThicknessRingComponent;
 import info.openrocket.core.rocketcomponent.Transition;
 import info.openrocket.core.rocketcomponent.TrapezoidFinSet;
+import info.openrocket.core.rocketcomponent.position.AxialMethod;
+import org.apache.commons.lang3.ClassUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -107,18 +109,18 @@ public class ScaleDialog extends JDialog {
 		addScaler(SymmetricComponent.class, "Thickness", "isFilled", SCALERS_NO_OFFSET);
 		
 		// Transition
-		addScaler(Transition.class, "ForeRadius", "isForeRadiusAutomatic", SCALERS_NO_OFFSET);
+		list = new ArrayList<>(1);
-		addScaler(Transition.class, "AftRadius", "isAftRadiusAutomatic", SCALERS_NO_OFFSET);
+		list.add(new TransitionScaler());
-		addScaler(Transition.class, "ForeShoulderRadius", SCALERS_NO_OFFSET);
+		SCALERS_NO_OFFSET.put(Transition.class, list);
 		addScaler(Transition.class, "ForeShoulderThickness", SCALERS_NO_OFFSET);
 		addScaler(Transition.class, "ForeShoulderLength", SCALERS_NO_OFFSET);
-		addScaler(Transition.class, "AftShoulderRadius", SCALERS_NO_OFFSET);
 		addScaler(Transition.class, "AftShoulderThickness", SCALERS_NO_OFFSET);
 		addScaler(Transition.class, "AftShoulderLength", SCALERS_NO_OFFSET);
 
 		// Nose cone
-		addScaler(NoseCone.class, "BaseRadius", "isBaseRadiusAutomatic", SCALERS_NO_OFFSET);
+		list = new ArrayList<>(1);
-		addScaler(NoseCone.class, "ShoulderRadius", SCALERS_NO_OFFSET);
+		list.add(new NoseConeScaler());
+		SCALERS_NO_OFFSET.put(NoseCone.class, list);
 		addScaler(NoseCone.class, "ShoulderThickness", SCALERS_NO_OFFSET);
 		addScaler(NoseCone.class, "ShoulderLength", SCALERS_NO_OFFSET);
 		
@@ -138,10 +140,9 @@ public class ScaleDialog extends JDialog {
 		addScaler(LaunchLug.class, "Length", SCALERS_NO_OFFSET);
 		
 		// FinSet
-		addScaler(FinSet.class, "Thickness", SCALERS_NO_OFFSET);
+		list = new ArrayList<>(1);
-		addScaler(FinSet.class, "TabHeight", SCALERS_NO_OFFSET);
+		list.add(new FinSetScaler());
-		addScaler(FinSet.class, "TabLength", SCALERS_NO_OFFSET);
+		SCALERS_NO_OFFSET.put(FinSet.class, list);
-		addScaler(FinSet.class, "TabOffset", SCALERS_NO_OFFSET);
 		
 		// TrapezoidFinSet
 		addScaler(TrapezoidFinSet.class, "Sweep", SCALERS_NO_OFFSET);
@@ -198,14 +199,14 @@ public class ScaleDialog extends JDialog {
 	}
 	
 	private static void addScaler(Class<? extends RocketComponent> componentClass, String methodName,
-								  Map<Class<? extends RocketComponent>, List<Scaler>> scaler) {
+								  Map<Class<? extends RocketComponent>, List<Scaler>> scaler, Object... arguments) {
-		addScaler(componentClass, methodName, null, scaler);
+		addScaler(componentClass, methodName, null, scaler, arguments);
 	}
 	
 	private static void addScaler(Class<? extends RocketComponent> componentClass, String methodName, String autoMethodName,
-								  Map<Class<? extends RocketComponent>, List<Scaler>> scaler) {
+								  Map<Class<? extends RocketComponent>, List<Scaler>> scaler, Object... arguments) {
 		List<Scaler> list = scaler.computeIfAbsent(componentClass, k -> new ArrayList<>());
-		list.add(new GeneralScaler(componentClass, methodName, autoMethodName));
+		list.add(new GeneralScaler(componentClass, methodName, autoMethodName, arguments));
 	}
 	
 	
@@ -658,7 +659,7 @@ public class ScaleDialog extends JDialog {
 	 * Interface for scaling a specific component/value.
 	 */
 	private interface Scaler {
-		public void scale(RocketComponent c, double multiplier, boolean scaleMass);
+		void scale(RocketComponent c, double multiplier, boolean scaleMass);
 	}
 	
 	/**
@@ -669,34 +670,52 @@ public class ScaleDialog extends JDialog {
 		private final Method getter;
 		private final Method setter;
 		private final Method autoMethod;
+		private final Object[] arguments;
 		
-		public GeneralScaler(Class<? extends RocketComponent> componentClass, String methodName, String autoMethodName) {
+		public GeneralScaler(Class<? extends RocketComponent> componentClass, String methodName, String autoMethodName,
+							 Object... arguments) {
 
-			getter = Reflection.findMethod(componentClass, "get" + methodName);
+			this.getter = Reflection.findMethod(componentClass, "get" + methodName);
-			setter = Reflection.findMethod(componentClass, "set" + methodName, double.class);
+			if (arguments == null || arguments.length == 0) {
-			if (autoMethodName != null) {
+				this.setter = Reflection.findMethod(componentClass, "set" + methodName, double.class);
-				autoMethod = Reflection.findMethod(componentClass, autoMethodName);
 			} else {
-				autoMethod = null;
+				Class<?>[] argumentClasses = new Class<?>[arguments.length + 1];
+				argumentClasses[0] = double.class;
+				for (int i = 0; i < arguments.length; i++) {
+					argumentClasses[i+1] = ClassUtils.wrapperToPrimitive(arguments[i].getClass());
+				}
+				this.setter = Reflection.findMethod(componentClass, "set" + methodName, argumentClasses);
+			}
+			this.arguments = arguments;
+			if (autoMethodName != null) {
+				this.autoMethod = Reflection.findMethod(componentClass, autoMethodName);
+			} else {
+				this.autoMethod = null;
 			}
 			
 		}
 		
 		@Override
 		public void scale(RocketComponent c, double multiplier, boolean scaleMass) {
-			
 			// Do not scale if set to automatic
-			if (autoMethod != null) {
+			if (this.autoMethod != null) {
-				boolean auto = (Boolean) autoMethod.invoke(c);
+				boolean auto = (Boolean) this.autoMethod.invoke(c);
 				if (auto) {
 					return;
 				}
 			}
 			
 			// Scale value
-			double value = (Double) getter.invoke(c);
+			double value = (Double) this.getter.invoke(c);
 			value = value * multiplier;
-			setter.invoke(c, value);
+			if (this.arguments == null || this.arguments.length == 0) {
+				this.setter.invoke(c, value);
+			} else {
+				Object[] parameters = new Object[this.arguments.length + 1];
+				parameters[0] = value;
+				System.arraycopy(this.arguments, 0, parameters, 1, this.arguments.length);
+				this.setter.invoke(c, parameters);
+			}
 		}
 		
 	}
@@ -724,6 +743,56 @@ public class ScaleDialog extends JDialog {
 		
 	}
 
+	private static class TransitionScaler implements Scaler {
+
+		@Override
+		public void scale(RocketComponent component, double multiplier, boolean scaleMass) {
+			final Map<Class<? extends RocketComponent>, List<Scaler>> scalers = new HashMap<>();
+
+			// If the multiplier is larger than 1, the fore/aft radius is scaled first
+			// to prevent the fore/aft shoulder radius from becoming larger than the fore/aft radius
+			if (multiplier >= 1) {
+				addScaler(Transition.class, "ForeRadius", "isForeRadiusAutomatic", scalers);
+				addScaler(Transition.class, "AftRadius", "isForeRadiusAutomatic", scalers);
+				addScaler(Transition.class, "ForeShoulderRadius", scalers, false);
+				addScaler(Transition.class, "AftShoulderRadius", scalers, false);
+			}
+			// If the multiplier is smaller than 1, the fore/aft shoulder radius is scaled first
+			// to prevent the fore/aft radius from becoming larger than the fore/aft shoulder radius
+			else  {
+				addScaler(Transition.class, "ForeShoulderRadius", scalers, false);
+				addScaler(Transition.class, "AftShoulderRadius", scalers, false);
+				addScaler(Transition.class, "ForeRadius", "isForeRadiusAutomatic", scalers);
+				addScaler(Transition.class, "AftRadius", "isForeRadiusAutomatic", scalers);
+			}
+
+			performIterativeScaling(scalers, component, multiplier, scaleMass);
+		}
+	}
+
+	private static class NoseConeScaler implements Scaler {
+
+		@Override
+		public void scale(RocketComponent component, double multiplier, boolean scaleMass) {
+			final Map<Class<? extends RocketComponent>, List<Scaler>> scalers = new HashMap<>();
+
+			// If the multiplier is larger than 1, the base radius is scaled first
+			// to prevent the shoulder radius from becoming larger than the base radius
+			if (multiplier >= 1) {
+				addScaler(NoseCone.class, "BaseRadius", "isBaseRadiusAutomatic", scalers);
+				addScaler(NoseCone.class, "ShoulderRadius", scalers);
+			}
+			// If the multiplier is smaller than 1, the shoulder radius is scaled first
+			// to prevent the base radius from becoming larger than the shoulder radius
+			else  {
+				addScaler(NoseCone.class, "ShoulderRadius", scalers);
+				addScaler(NoseCone.class, "BaseRadius", "isBaseRadiusAutomatic", scalers);
+			}
+
+			performIterativeScaling(scalers, component, multiplier, scaleMass);
+		}
+	}
+	
 	private static class MassComponentScaler implements Scaler {
 		
 		@Override
@@ -738,8 +807,30 @@ public class ScaleDialog extends JDialog {
 		
 	}
 
-	private static class FreeformFinSetScaler implements Scaler {
+	private static class FinSetScaler implements Scaler {
+		@Override
+		public void scale(RocketComponent component, double multiplier, boolean scaleMass) {
+			final Map<Class<? extends RocketComponent>, List<Scaler>> scalers = new HashMap<>();
+			FinSet finset = (FinSet) component;
+			AxialMethod originalTabOffsetMethod = finset.getTabOffsetMethod();
+			finset.setTabOffsetMethod(AxialMethod.ABSOLUTE);
 
+			double tabOffset = finset.getTabOffset();
+			tabOffset = tabOffset * multiplier;
+
+
+			addScaler(FinSet.class, "Thickness", scalers);
+			addScaler(FinSet.class, "TabHeight", scalers);
+			addScaler(FinSet.class, "TabLength", scalers);
+
+			performIterativeScaling(scalers, component, multiplier, scaleMass);
+
+			finset.setTabOffset(tabOffset);
+			finset.setTabOffsetMethod(originalTabOffsetMethod);
+		}
+	}
+	
+	private static class FreeformFinSetScaler implements Scaler {
 		@Override
 		public void scale(RocketComponent component, double multiplier, boolean scaleMass) {
 			FreeformFinSet finset = (FreeformFinSet) component;
@@ -748,10 +839,8 @@ public class ScaleDialog extends JDialog {
 				points[i] = points[i].multiply(multiplier);
 			}
 			
-			finset.setPoints(points);
+			finset.setPoints(points, false);
-			
 		}
-		
 	}
 
 	private static class RadiusRingComponentScaler implements Scaler {
@@ -768,11 +857,7 @@ public class ScaleDialog extends JDialog {
 				addScaler(RadiusRingComponent.class, "OuterRadius", "isOuterRadiusAutomatic", scalers);
 			}
 
-			for (List<Scaler> foo : scalers.values()) {
+			performIterativeScaling(scalers, component, multiplier, scaleMass);
-				for (Scaler s : foo) {
-					s.scale(component, multiplier, scaleMass);
-				}
-			}
 		}
 
 	}
@@ -790,11 +875,7 @@ public class ScaleDialog extends JDialog {
 				addScaler(ThicknessRingComponent.class, "OuterRadius", "isOuterRadiusAutomatic", scalers);
 			}
 
-			for (List<Scaler> foo : scalers.values()) {
+			performIterativeScaling(scalers, component, multiplier, scaleMass);
-				for (Scaler s : foo) {
-					s.scale(component, multiplier, scaleMass);
-				}
-			}
 		}
 	}
 
@@ -817,10 +898,15 @@ public class ScaleDialog extends JDialog {
 				addScaler(RailButton.class, "TotalHeight", scalers);
 			}
 
-			for (List<Scaler> foo : scalers.values()) {
+			performIterativeScaling(scalers, component, multiplier, scaleMass);
-				for (Scaler s : foo) {
+		}
-					s.scale(component, multiplier, scaleMass);
+	}
-				}
+
+	private static void performIterativeScaling(Map<Class<? extends RocketComponent>, List<Scaler>> scalers,
+												RocketComponent component, double multiplier, boolean scaleMass) {
+		for (List<Scaler> foo : scalers.values()) {
+			for (Scaler s : foo) {
+				s.scale(component, multiplier, scaleMass);
 			}
 		}
 	}

swing/src/main/java/module-info.java

@@ -31,6 +31,7 @@ open module info.openrocket.swing {
 	requires com.formdev.flatlaf.extras;
 	requires com.formdev.flatlaf.intellijthemes;
 	requires org.checkerframework.checker.qual;
+	requires org.apache.commons.lang3;
 
 	// Service providers
 	// Also edit swing/src/main/resources/META-INF/services !! (until gradle-modules-plugin supports service