FRU improvements and general cleanup

trigger-support/src/main/java/gg/xp/xivsupport/events/triggers/seq/SequentialTrigger.java

@@ -98,7 +98,7 @@ public void forceExpire() {
 			inst.forceExpire();
 			instance = null;
 		}
-		instances.forEach(SequentialTriggerController::forceExpire);
+		instances.forEach(SequentialTriggerController::stopSilently);
 		instances.clear();
 	}
 

trigger-support/src/main/java/gg/xp/xivsupport/events/triggers/seq/SequentialTriggerController.java

@@ -27,6 +27,7 @@
 import java.io.Serial;
 import java.time.Duration;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -35,6 +36,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.BooleanSupplier;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 public class SequentialTriggerController<X extends BaseEvent> {
@@ -43,7 +45,7 @@ public class SequentialTriggerController<X extends BaseEvent> {
 	private static final AtomicInteger threadIdCounter = new AtomicInteger();
 	//	private final Instant expiresAt;
 	private final BooleanSupplier expired;
-	private final Thread thread;
+	private final Thread triggerThread;
 	private final Object lock = new Object();
 	private final X initialEvent;
 	private final int timeout;
@@ -63,7 +65,7 @@ public SequentialTriggerController(EventContext initialEventContext, X initialEv
 		this.timeout = timeout;
 //		expiresAt = initialEvent.getHappenedAt().plusMillis(timeout);
 		context = initialEventContext;
-		thread = new Thread(() -> {
+		triggerThread = new Thread(() -> {
 			try {
 				triggerCode.accept(initialEvent, this);
 			}
@@ -83,9 +85,9 @@ public SequentialTriggerController(EventContext initialEventContext, X initialEv
 			}
 		}, "SequentialTrigger-" + threadIdCounter.getAndIncrement());
 		this.initialEvent = initialEvent;
-		thread.setDaemon(true);
-		thread.setPriority(Thread.MAX_PRIORITY);
-		thread.start();
+		triggerThread.setDaemon(true);
+		triggerThread.setPriority(Thread.MAX_PRIORITY);
+		triggerThread.start();
 		synchronized (lock) {
 			waitProcessingDone();
 		}
@@ -640,7 +642,7 @@ public void provideEvent(EventContext ctx, X event) {
 		}
 	}
 
-	private static final int defaultCycleProcessingTime = 250;
+	private static final int defaultCycleProcessingTime = 500;
 	private static final int cycleProcessingTime;
 
 	// Workaround for integration tests exceeding cycle time
@@ -672,7 +674,10 @@ private void waitProcessingDone() {
 			try {
 				long timeLeft = failAt - System.currentTimeMillis();
 				if (timeLeft <= 0) {
-					log.error("Cycle processing time max ({}ms) exceeded", timeoutMs);
+					String formattedStackTrace = Arrays.stream(triggerThread.getStackTrace())
+							.map(element -> "\t at %s".formatted(element.toString()))
+							.collect(Collectors.joining("\n"));
+					log.error("Cycle processing time max ({}ms) exceeded. Trigger thread stack:\n{}", timeoutMs, formattedStackTrace);
 					cycleProcessingTimeExceeded = true;
 					return;
 				}

triggers/titan-jails/src/test/java/gg/xp/xivsupport/events/JailExampleTest.java

@@ -24,6 +24,7 @@
 import gg.xp.xivsupport.models.XivEntity;
 import gg.xp.xivsupport.models.XivZone;
 import gg.xp.xivsupport.persistence.PersistenceProvider;
+import gg.xp.xivsupport.persistence.settings.JobSortValidationException;
 import gg.xp.xivsupport.speech.BaseCalloutEvent;
 import gg.xp.xivsupport.speech.CalloutEvent;
 import gg.xp.xivsupport.speech.ProcessedCalloutEvent;
@@ -631,18 +632,18 @@ public void orderValidation() {
 		MutablePicoContainer container = setup();
 		JailSolver jail = container.getComponent(JailSolver.class);
 		// Insufficient size
-		Assert.assertThrows(IllegalArgumentException.class, () -> jail.getSort().setJobOrder(List.of(Job.WHM, Job.SCH)));
+		Assert.assertThrows(JobSortValidationException.class, () -> jail.getSort().setJobOrder(List.of(Job.WHM, Job.SCH)));
 		{
 			List<Job> current = new ArrayList<>(jail.getSort().getJobOrder());
 			// Make a duplicate
 			current.set(0, current.get(1));
-			Assert.assertThrows(IllegalArgumentException.class, () -> jail.getSort().setJobOrder(current));
+			Assert.assertThrows(JobSortValidationException.class, () -> jail.getSort().setJobOrder(current));
 		}
 		{
 			List<Job> current = new ArrayList<>(jail.getSort().getJobOrder());
 			// Too many
 			current.add(current.get(0));
-			Assert.assertThrows(IllegalArgumentException.class, () -> jail.getSort().setJobOrder(current));
+			Assert.assertThrows(JobSortValidationException.class, () -> jail.getSort().setJobOrder(current));
 		}
 	}
 

triggers/triggers-dt/src/main/java/gg/xp/xivsupport/triggers/ultimate/FRU.java

@@ -704,6 +704,7 @@ else if (playerHasMarker) {
 	@NpcCastCallout(0x9D20)
 	private final ModifiableCallout<AbilityCastStart> p2enrage = ModifiableCallout.durationBasedCall("P2 Enrage", "Enrage, Knockback");
 
+	// TODO: add callout for when main crystal becomes vulnerable
 	@NpcCastCallout(value = 0x9D43, cancellable = true)
 	private final ModifiableCallout<AbilityCastStart> intermissionEnrage = ModifiableCallout.durationBasedCall("Endless Ice Age (Intermission)", "Kill Crystals, Bait AoEs");
 
@@ -755,11 +756,11 @@ private static Predicate<HasDuration> initDurBetween(int secondsMin, int seconds
 
 	private final ModifiableCallout<?> relLongRewind = new ModifiableCallout<>("Relativity: Long Rewind", "Bait Spinny")
 			.extendedDescription("This call happens after the first fire/stack pop, if you have long rewind.");
-	private final ModifiableCallout<BuffApplied> relShortRewindEruption = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Short Rewind w/ Eruption", "Stand on Light").autoIcon()
+	private final ModifiableCallout<BuffApplied> relShortRewindEruption = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Short Rewind w/ Eruption", "Drop Rewind on Light").autoIcon()
 			.extendedDescription("This call happens after the first fire/stack pop, if you have short rewind and have eruption (no water).");
-	private final ModifiableCallout<BuffApplied> relShortRewindEruptionMedFire = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Short Rewind w/ Eruption and Med Fire", "Stand inside Light").autoIcon()
+	private final ModifiableCallout<BuffApplied> relShortRewindEruptionMedFire = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Short Rewind w/ Eruption and Med Fire", "Drop Rewind inside Light").autoIcon()
 			.extendedDescription("This call happens after the first fire/stack pop, if you have short rewind and have eruption (no water) as well as medium fire.");
-	private final ModifiableCallout<BuffApplied> relShortRewindWater = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Short Rewind w/ Water", "Stand In").autoIcon()
+	private final ModifiableCallout<BuffApplied> relShortRewindWater = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Short Rewind w/ Water", "Drop Rewind Inside").autoIcon()
 			.extendedDescription("This call happens after the first fire/stack pop, if you have short rewind and have water (no eruption).");
 
 	private final ModifiableCallout<BuffApplied> relMedFirePop = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Medium Fire Popping", "Move Out").autoIcon()
@@ -768,9 +769,11 @@ private static Predicate<HasDuration> initDurBetween(int secondsMin, int seconds
 
 	private final ModifiableCallout<?> relShortRewindBait = new ModifiableCallout<>("Relativity: Short Rewind Part 2", "Bait Spinny")
 			.extendedDescription("This call happens after the second fire/stack pop, if you have short rewind and do not have medium fire.");
+	// TODO icons
 	private final ModifiableCallout<?> relShortRewindMedFire = new ModifiableCallout<>("Relativity: Short Rewind Part 2 (Med Fire)", "AFK")
 			.extendedDescription("This call happens after the second fire/stack pop, if you have short rewind and had medium fire.");
-	private final ModifiableCallout<?> relLongRewind2 = new ModifiableCallout<>("Relativity: Long Rewind Part 2", "Stand Middle")
+	private final ModifiableCallout<?> relLongRewind2 = new ModifiableCallout<>("Relativity: Long Rewind Part 2", "Drop Rewind Middle")
+			.statusIcon(0x9A0)
 			.extendedDescription("This call happens after the first fire/stack pop, if you have long rewind.");
 
 	private final ModifiableCallout<BuffApplied> relLongFirePop = ModifiableCallout.<BuffApplied>durationBasedCall("Relativity: Long Fire Popping", "Move Out").autoIcon()
@@ -901,7 +904,6 @@ private static Predicate<HasDuration> initDurBetween(int secondsMin, int seconds
 								s.updateCall(relShortRewindWater, e);
 							}
 							else {
-								// TODO: some people need to bait inside for this?
 								if (medFireC.anyMatch(isPlayer)) {
 									s.updateCall(relShortRewindEruptionMedFire, e);
 								}

xivsupport/src/main/java/gg/xp/xivsupport/gui/GuiEarlyComponents.java

@@ -18,6 +18,7 @@ public class GuiEarlyComponents {
 
 	private final MutablePicoContainer pico;
 	private Container gp;
+	private JLabel loadingLabel;
 
 	public GuiEarlyComponents(MutablePicoContainer pico) {
 		this.pico = pico;
@@ -75,7 +76,7 @@ public void windowStateChanged(WindowEvent e) {
 			mainFrame.add(new ReplayControllerGui(pico, replay).getPanel(), BorderLayout.PAGE_START);
 		}
 		gp = (Container) mainFrame.getGlassPane();
-		JLabel loadingLabel = new JLabel("Loading...");
+		loadingLabel = new JLabel("Loading...");
 		loadingLabel.setFont(loadingLabel.getFont().deriveFont(Font.PLAIN, 48));
 		gp.setLayout(new GridBagLayout());
 		gp.add(loadingLabel);
@@ -118,7 +119,7 @@ private void setUpTrayIcon() {
 	}
 
 	void hideLoading() {
-		this.gp.setVisible(false);
+		gp.remove(loadingLabel);
 	}
 
 }

xivsupport/src/main/java/gg/xp/xivsupport/persistence/settings/JobSortSetting.java

@@ -42,9 +42,16 @@ public JobSortSetting(PersistenceProvider pers, String settingKey, XivState stat
 				validateJobSortOrder(listFromSettings);
 				jobSort = listFromSettings;
 			}
+			catch (JobSortValidationException je) {
+				if (je.isSilent()) {
+					log.warn(je.getMessage());
+				}
+				else {
+					log.error(je.getMessage(), je);
+				}
+			}
 			catch (Throwable t) {
-				// TODO: this is annoying because it will crop up every time a new job is added
-				log.error("Saved jail order did not pass validation", t);
+				log.error("Saved jail order did not pass validation ({})", settingKey, t);
 			}
 		}
 		// Fall back to default
@@ -147,24 +154,45 @@ public void resetJailSort() {
 	}
 
 	/**
-	 * @return Whether or not this setting has actually been set by the user, or if it is using default values.
+	 * @return True if this setting has actually been set by the user, false if it is using default values.
 	 */
 	public boolean isSet() {
 		return sortSetting.isSet();
 	}
 
 	public void validateJobSortOrder(List<Job> newSort) {
-		int expectedNumberOfJobs = allValidJobs.size();
-		int actualNumberOfJobs = newSort.size();
-		if (expectedNumberOfJobs != actualNumberOfJobs) {
-			// TODO: this spams log on startup
-			// Make it more intelligent - if saved sort is a subset of the available jobs, don't throw this
-			throw new IllegalArgumentException(String.format("New jail sort order was not the same size! %s -> %s", expectedNumberOfJobs, actualNumberOfJobs));
+		// First, check for duplicates
+		// Convert the list to a set and check that the size is still the same.
+		if (newSort.isEmpty()) {
+			throw new JobSortValidationException("New sort order was empty!", false, allValidJobs, newSort);
 		}
-		EnumSet<Job> newSortAsSet = EnumSet.copyOf(newSort);
+		Set<Job> newSortAsSet = EnumSet.copyOf(newSort);
 		int newUniqueSize = newSortAsSet.size();
+		int actualNumberOfJobs = newSort.size();
 		if (newUniqueSize != actualNumberOfJobs) {
-			throw new IllegalArgumentException("New jail sort had duplicates!");
+			List<Job> tmpJobs = new ArrayList<>(newSort);
+			allValidJobs.forEach(tmpJobs::remove);
+			throw new JobSortValidationException("New jail sort had duplicates! Extras: %s".formatted(tmpJobs), false, allValidJobs, newSort);
+		}
+		if (!newSortAsSet.equals(allValidJobs)) {
+			// Jobs present in base list but not in the new order
+			Set<Job> jobsMissingFromNewSort = EnumSet.copyOf(allValidJobs);
+			jobsMissingFromNewSort.removeAll(newSortAsSet);
+			// Jobs present in new order but not in base list
+			Set<Job> extraneousJobs = EnumSet.copyOf(newSort);
+			extraneousJobs.removeAll(allValidJobs);
+
+			boolean silentFail = true;
+			StringBuilder sb = new StringBuilder("Job sort did not pass validation!");
+
+			if (!jobsMissingFromNewSort.isEmpty()) {
+				sb.append("\nJobs missing from new sort: ").append(jobsMissingFromNewSort);
+			}
+			if (!extraneousJobs.isEmpty()) {
+				sb.append("\nInvalid jobs found: ").append(extraneousJobs);
+				silentFail = false;
+			}
+			throw new JobSortValidationException(sb.toString(), silentFail, allValidJobs, newSort);
 		}
 	}
 

xivsupport/src/main/java/gg/xp/xivsupport/persistence/settings/JobSortValidationException.java

@@ -0,0 +1,38 @@
+package gg.xp.xivsupport.persistence.settings;
+
+import gg.xp.xivdata.data.*;
+
+import java.io.Serial;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Set;
+
+public class JobSortValidationException extends RuntimeException {
+
+	@Serial
+	private static final long serialVersionUID = 6391762663338571558L;
+	private final boolean silent;
+	private final Set<Job> expected;
+	private final List<Job> actual;
+
+	JobSortValidationException(final String message, final boolean silent, Set<Job> expected, List<Job> actual) {
+		super(message);
+		this.silent = silent;
+		this.expected = expected.isEmpty() ? Collections.emptySet() : EnumSet.copyOf(expected);
+		this.actual = new ArrayList<>(actual);
+	}
+
+	public boolean isSilent() {
+		return silent;
+	}
+
+	public Set<Job> getExpected() {
+		return Collections.unmodifiableSet(expected);
+	}
+
+	public List<Job> getActual() {
+		return Collections.unmodifiableList(actual);
+	}
+}