Repository: runelite/runelite
Pull request #17168 (2023-11-06 20:33:46)

Fix HotColdClue running code before static initializer finishes. Also removes spurious log message.
clue scroll: in HotColdClue, fix issue where methods were called before the class's static initializer had run. Specifically, the static inititializer called `initializeSolver` which referenced the static variables BEGINNER_CLUE and MASTER_CLUE, which are not yet initialized at this time. This commit removes that call. This also removes a spurious warn message that was logged.

This commit also cleans up a lot of code not directly related to the above bug:
- It is also not necessary to null check the hotColdSolver as it is never null when it is used, so these null checks have been removed.
- This commit also adds a boolean `isBeginner` which allows the `HotColdClue` to know if it is a beginner or master clue without reference comparing to the `BEGINNER_CLUE` and `MASTER_CLUE` instances. This also simplifies a lot of logic as it is no longer possible to have a HotColdClue that is neither beginner or master.
- Also removes an unnecessary null check on the result of `WorldPoint.getMirrorPoint` which cannot return null.

n.b. I haven't tested this with a real hot cold clue.


Patch:
diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/cluescrolls/clues/HotColdClue.java b/runelite-client/src/main/java/net/runelite/client/plugins/cluescrolls/clues/HotColdClue.java
index 75c39d324..ef742c847 100644
--- a/runelite-client/src/main/java/net/runelite/client/plugins/cluescrolls/clues/HotColdClue.java
+++ b/runelite-client/src/main/java/net/runelite/client/plugins/cluescrolls/clues/HotColdClue.java
@@ -33,7 +33,6 @@ import java.util.EnumMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
-import javax.annotation.Nullable;
 import lombok.AccessLevel;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
@@ -62,18 +61,20 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 	private static final HotColdClue BEGINNER_CLUE = new HotColdClue("Buried beneath the ground, who knows where it's found. Lucky for you, A man called Reldo may have a clue.",
 		"Reldo",
 		"Speak to Reldo to receive a strange device.",
-		new WorldPoint(3211, 3494, 0));
+		new WorldPoint(3211, 3494, 0),
+		true);
 	private static final HotColdClue MASTER_CLUE = new HotColdClue("Buried beneath the ground, who knows where it's found. Lucky for you, A man called Jorral may have a clue.",
 		"Jorral",
 		"Speak to Jorral to receive a strange device.",
-		new WorldPoint(2436, 3347, 0));
+		new WorldPoint(2436, 3347, 0),
+		false);
 
 	private final String text;
 	private final String npc;
 	private final String solution;
 	@Getter(AccessLevel.PRIVATE)
 	private final WorldPoint npcLocation;
-	@Nullable
+	private final boolean isBeginner;
 	private HotColdSolver hotColdSolver;
 	private WorldPoint location;
 
@@ -93,14 +94,14 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 		return null;
 	}
 
-	private HotColdClue(String text, String npc, String solution, WorldPoint npcLocation)
+	private HotColdClue(String text, String npc, String solution, WorldPoint npcLocation, boolean isBeginner)
 	{
 		this.text = text;
 		this.npc = npc;
 		this.solution = solution;
 		this.npcLocation = npcLocation;
+		this.isBeginner = isBeginner;
 		setRequiresSpade(true);
-		initializeSolver();
 	}
 
 	@Override
@@ -112,11 +113,6 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 	@Override
 	public WorldPoint[] getLocations(ClueScrollPlugin plugin)
 	{
-		if (hotColdSolver == null)
-		{
-			return new WorldPoint[0];
-		}
-
 		if (hotColdSolver.getLastWorldPoint() == null)
 		{
 			return new WorldPoint[] {npcLocation};
@@ -130,11 +126,6 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 	@Override
 	public void makeOverlayHint(PanelComponent panelComponent, ClueScrollPlugin plugin)
 	{
-		if (hotColdSolver == null)
-		{
-			return;
-		}
-
 		panelComponent.getChildren().add(TitleComponent.builder()
 			.text("Hot/Cold Clue")
 			.build());
@@ -229,11 +220,6 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 	@Override
 	public void makeWorldOverlayHint(Graphics2D graphics, ClueScrollPlugin plugin)
 	{
-		if (hotColdSolver == null)
-		{
-			return;
-		}
-
 		// when final location has been found
 		if (location != null)
 		{
@@ -278,25 +264,11 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 
 	public boolean update(final String message, final ClueScrollPlugin plugin)
 	{
-		if (hotColdSolver == null)
-		{
-			return false;
-		}
-
 		final Set<HotColdTemperature> temperatureSet;
 
-		if (this == BEGINNER_CLUE)
-		{
-			temperatureSet = HotColdTemperature.BEGINNER_HOT_COLD_TEMPERATURES;
-		}
-		else if (this == MASTER_CLUE)
-		{
-			temperatureSet = HotColdTemperature.MASTER_HOT_COLD_TEMPERATURES;
-		}
-		else
-		{
-			temperatureSet = null;
-		}
+		temperatureSet = isBeginner() ?
+			HotColdTemperature.BEGINNER_HOT_COLD_TEMPERATURES :
+			HotColdTemperature.MASTER_HOT_COLD_TEMPERATURES;
 
 		final HotColdTemperature temperature = HotColdTemperature.getFromTemperatureSet(temperatureSet, message);
 
@@ -308,13 +280,8 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 		// Convert from real to overworld
 		final WorldPoint localWorld = WorldPoint.getMirrorPoint(plugin.getClient().getLocalPlayer().getWorldLocation(), true);
 
-		if (localWorld == null)
-		{
-			return false;
-		}
-
-		if ((this == BEGINNER_CLUE && temperature == HotColdTemperature.BEGINNER_VISIBLY_SHAKING)
-			|| (this == MASTER_CLUE && temperature == HotColdTemperature.MASTER_VISIBLY_SHAKING))
+		if ((isBeginner() && temperature == HotColdTemperature.BEGINNER_VISIBLY_SHAKING)
+			|| (!isBeginner() && temperature == HotColdTemperature.MASTER_VISIBLY_SHAKING))
 		{
 			markFinalSpot(localWorld);
 		}
@@ -338,26 +305,8 @@ public class HotColdClue extends ClueScroll implements LocationClueScroll, Locat
 
 	private void initializeSolver()
 	{
-		final boolean isBeginner;
-
-		if (this == BEGINNER_CLUE)
-		{
-			isBeginner = true;
-		}
-		else if (this == MASTER_CLUE)
-		{
-			isBeginner = false;
-		}
-		else
-		{
-			log.warn("Hot cold solver could not be initialized, clue type is unknown; text: {}, npc: {}, solution: {}",
-				text, npc, solution);
-			hotColdSolver = null;
-			return;
-		}
-
 		final Set<HotColdLocation> locations = Arrays.stream(HotColdLocation.values())
-			.filter(l -> l.isBeginnerClue() == isBeginner)
+			.filter(l -> l.isBeginnerClue() == isBeginner())
 			.collect(Collectors.toSet());
 		hotColdSolver = new HotColdSolver(locations);
 	}
diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPlugin.java b/runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPlugin.java
index 7f175111e..9f0edb36f 100644
--- a/runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPlugin.java
+++ b/runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPlugin.java
@@ -813,7 +813,7 @@ public class LootTrackerPlugin extends Plugin
 				container = client.getItemContainer(InventoryID.CHAMBERS_OF_XERIC_CHEST);
 				chestLooted = true;
 				break;
-			case InterfaceID.TOB:
+			case InterfaceID.TOB_REWARD:
 				if (chestLooted || !inTobChestRegion())
 				{
 					return;


Test patch:
diff --git a/runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerPluginTest.java b/runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerPluginTest.java
index db10c7608..c7289b3df 100644
--- a/runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerPluginTest.java
+++ b/runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerPluginTest.java
@@ -300,7 +300,7 @@ public class LootTrackerPluginTest
 		when(client.getLocalPlayer().getLocalLocation()).thenReturn(localPoint);
 
 		WidgetLoaded widgetLoaded = new WidgetLoaded();
-		widgetLoaded.setGroupId(InterfaceID.TOB);
+		widgetLoaded.setGroupId(InterfaceID.TOB_REWARD);
 		spyPlugin.onWidgetLoaded(widgetLoaded);
 
 		ArgumentCaptor<QueuedMessage> captor = ArgumentCaptor.forClass(QueuedMessage.class);
@@ -451,7 +451,7 @@ public class LootTrackerPluginTest
 		spyPlugin.onGameStateChanged(loading);
 
 		WidgetLoaded widgetLoaded = new WidgetLoaded();
-		widgetLoaded.setGroupId(InterfaceID.TOB);
+		widgetLoaded.setGroupId(InterfaceID.TOB_REWARD);
 		spyPlugin.onWidgetLoaded(widgetLoaded);
 
 		verify(spyPlugin).addLoot("Theatre of Blood", -1, LootRecordType.EVENT, null, Collections.singletonList(
@@ -480,7 +480,7 @@ public class LootTrackerPluginTest
 		spyPlugin.onGameStateChanged(loading);
 
 		WidgetLoaded widgetLoaded = new WidgetLoaded();
-		widgetLoaded.setGroupId(InterfaceID.TOB);
+		widgetLoaded.setGroupId(InterfaceID.TOB_REWARD);
 		spyPlugin.onWidgetLoaded(widgetLoaded);
 
 		verify(spyPlugin).addLoot("Theatre of Blood", -1, LootRecordType.EVENT, null, Collections.singletonList(
@@ -509,7 +509,7 @@ public class LootTrackerPluginTest
 		spyPlugin.onGameStateChanged(loading);
 
 		WidgetLoaded widgetLoaded = new WidgetLoaded();
-		widgetLoaded.setGroupId(InterfaceID.TOB);
+		widgetLoaded.setGroupId(InterfaceID.TOB_REWARD);
 		spyPlugin.onWidgetLoaded(widgetLoaded);
 
 		verify(spyPlugin).addLoot("Theatre of Blood", -1, LootRecordType.EVENT, null, Collections.singletonList(