diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestHelperPlugin.java b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestHelperPlugin.java index 0b3e55d1c6..5741e48c7f 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestHelperPlugin.java +++ b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestHelperPlugin.java @@ -79,7 +79,7 @@ @PluginDescriptor( name = "Quest Helper", - version = "1.0.13", + version = "1.0.14", description = "Helps you with questing", tags = { "quest", "helper", "overlay" } ) diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestScript.java b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestScript.java index fc750f8952..2d5a2c52a9 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestScript.java +++ b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/QuestScript.java @@ -8,6 +8,7 @@ import net.runelite.api.widgets.Widget; import net.runelite.client.plugins.microbot.Microbot; import net.runelite.client.plugins.microbot.Script; +import net.runelite.client.plugins.microbot.questhelper.logic.IQuest; import net.runelite.client.plugins.microbot.questhelper.logic.PiratesTreasure; import net.runelite.client.plugins.microbot.questhelper.logic.QuestRegistry; import net.runelite.client.plugins.microbot.questhelper.questinfo.QuestHelperQuest; @@ -104,6 +105,10 @@ public class QuestScript extends Script { private volatile long nextCustomAttemptAt; private boolean customActionPending; + static QuestHelper selectedQuestSnapshot(QuestHelperPlugin plugin) { + return plugin == null ? null : plugin.getSelectedQuest(); + } + private static WorldPoint scenePlayerLocation() { Player player = Microbot.getClient().getLocalPlayer(); return player == null ? null : player.getWorldLocation(); @@ -140,7 +145,9 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { clearInteractionState(); return; } - if (getQuestHelperPlugin().getSelectedQuest() == null) { + QuestHelper selectedQuest = selectedQuestSnapshot(getQuestHelperPlugin()); + QuestStep currentQuestStep = selectedQuest == null ? null : selectedQuest.getCurrentStep(); + if (currentQuestStep == null) { clearInteractionState(); return; } @@ -150,11 +157,13 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { observedResetGeneration = interactionResetGeneration.get(); } - QuestStep questStep = getQuestHelperPlugin().getSelectedQuest().getCurrentStep().getActiveStep(); + QuestStep questStep = currentQuestStep.getActiveStep(); if (questStep == null) { clearInteractionState(); return; } + int selectedQuestId = selectedQuest.getQuest() == null ? -1 : selectedQuest.getQuest().getId(); + IQuest questLogic = questLogicFor(selectedQuest); observePendingInteraction(questStep); if (QuestInteractionFlow.handleCutscene(Microbot.getVarbitValue(4606) > 0, @@ -187,7 +196,7 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { if (shouldPauseBeforeCustomLogic( Rs2Dialogue.isInDialogue(), pendingInteraction != null, Rs2Player.isAnimating(), - customLogicRunsWhileAnimating())) return; + customLogicRunsWhileAnimating(questLogic))) return; if (questStep != null && !questStep.getWidgetsToHighlight().isEmpty()) { var visibleWidgetHighlights = questStep.getWidgetsToHighlight().stream() @@ -250,7 +259,7 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { Rs2Widget.clickWidget(widgetHighlight.getNameToCheckFor()); } else { Rs2Widget.clickWidget(widget.getId()); - if (Rs2Shop.isOpen() && getQuestHelperPlugin().getSelectedQuest().getQuest().getId() == Quest.PIRATES_TREASURE.getId()) { + if (Rs2Shop.isOpen() && selectedQuestId == Quest.PIRATES_TREASURE.getId()) { Rs2Shop.buyItemOptimally("karamjan rum", 1); } } @@ -262,11 +271,10 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { boolean dialogueAdvanceReserved = Rs2Dialogue.isInDialogue(); if (dialogueAdvanceReserved && !QuestInteractionFlow.allowGenericDialogue( - this::allowDialogueAdvance, this::executeQuestCustomLogic)) return; + this::allowDialogueAdvance, () -> executeQuestCustomLogic(questLogic))) return; - if (getQuestHelperPlugin().getSelectedQuest() != null && !Microbot.getClientThread().runOnClientThreadOptional(() -> - getQuestHelperPlugin().getSelectedQuest().isCompleted()).orElse(null)) { - if (Rs2Widget.isWidgetVisible(ComponentID.DIALOG_OPTION_OPTIONS) && getQuestHelperPlugin().getSelectedQuest().getQuest().getId() != Quest.COOKS_ASSISTANT.getId() && !Rs2Bank.isOpen()) { + if (!Microbot.getClientThread().runOnClientThreadOptional(selectedQuest::isCompleted).orElse(true)) { + if (Rs2Widget.isWidgetVisible(ComponentID.DIALOG_OPTION_OPTIONS) && selectedQuestId != Quest.COOKS_ASSISTANT.getId() && !Rs2Bank.isOpen()) { if (!dialogueAdvanceReserved && !allowDialogueAdvance()) return; boolean hasOption = Rs2Dialogue.handleQuestOptionDialogueSelection(); //if there is no quest option in the dialogue, just click player location to remove @@ -275,8 +283,7 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { if (Rs2Dialogue.acceptQuestStartDialogue()) { return; } - if (getQuestHelperPlugin().getSelectedQuest() != null && - getQuestHelperPlugin().getSelectedQuest().getQuest().getId() == Quest.IMP_CATCHER.getId() + if (selectedQuestId == Quest.IMP_CATCHER.getId() && Microbot.getClient().getTopLevelWorldView().getPlane() == 1) { Rs2Dialogue.keyPressForDialogueOption(1); // presses option 1 sleep(1200,1800); @@ -286,14 +293,12 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { return; } - if (getQuestHelperPlugin().getSelectedQuest() != null && - getQuestHelperPlugin().getSelectedQuest().getQuest().getId() == Quest.COOKS_ASSISTANT.getId() && + if (selectedQuestId == Quest.COOKS_ASSISTANT.getId() && Rs2Dialogue.isInDialogue()) { dialogueStartedStep = questStep; // Force this to be true for Cook's Assistant } - if (getQuestHelperPlugin().getSelectedQuest() != null && - getQuestHelperPlugin().getSelectedQuest().getQuest().getId() == Quest.PIRATES_TREASURE.getId() && + if (selectedQuestId == Quest.PIRATES_TREASURE.getId() && Rs2Dialogue.isInDialogue()) { dialogueStartedStep = questStep; } @@ -314,9 +319,9 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { if (pendingInteraction != null) return; boolean playerAnimating = Rs2Player.isAnimating(); - if (playerAnimating && !customLogicRunsWhileAnimating()) return; + if (playerAnimating && !customLogicRunsWhileAnimating(questLogic)) return; - if (!runIdleCustomLogic(questStep)) return; + if (!runIdleCustomLogic(questStep, questLogic)) return; if (playerAnimating) return; @@ -341,7 +346,7 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { * If we do not prioritize this, the script will think we are missing items */ if (questStep instanceof DetailedQuestStep && !(questStep instanceof NpcStep || questStep instanceof ObjectStep || questStep instanceof DigStep)) { - boolean result = applyDetailedQuestStep((DetailedQuestStep) getQuestHelperPlugin().getSelectedQuest().getCurrentStep().getActiveStep()); + boolean result = applyDetailedQuestStep((DetailedQuestStep) questStep); if (result) { sleepUntil(() -> Rs2Player.isInteracting() || Rs2Player.isMoving() || Rs2Player.isAnimating() || Rs2Dialogue.isInDialogue(), 500); sleepUntil(() -> !Rs2Player.isInteracting() && !Rs2Player.isMoving() && !Rs2Player.isAnimating()); @@ -349,17 +354,16 @@ public boolean run(QuestHelperConfig config, QuestHelperPlugin mQuestPlugin) { } } - if (getQuestHelperPlugin().getSelectedQuest().getCurrentStep() instanceof ConditionalStep) { - QuestStep conditionalStep = getQuestHelperPlugin().getSelectedQuest().getCurrentStep().getActiveStep(); - applyStep(conditionalStep); - } else if (getQuestHelperPlugin().getSelectedQuest().getCurrentStep() instanceof NpcStep) { - applyNpcStep((NpcStep) getQuestHelperPlugin().getSelectedQuest().getCurrentStep()); - } else if (getQuestHelperPlugin().getSelectedQuest().getCurrentStep() instanceof ObjectStep) { - applyObjectStep((ObjectStep) getQuestHelperPlugin().getSelectedQuest().getCurrentStep()); - } else if (getQuestHelperPlugin().getSelectedQuest().getCurrentStep() instanceof DigStep) { - applyDigStep((DigStep) getQuestHelperPlugin().getSelectedQuest().getCurrentStep()); - } else if (getQuestHelperPlugin().getSelectedQuest().getCurrentStep() instanceof PuzzleStep) { - applyPuzzleStep((PuzzleStep) getQuestHelperPlugin().getSelectedQuest().getCurrentStep()); + if (currentQuestStep instanceof ConditionalStep) { + applyStep(questStep); + } else if (currentQuestStep instanceof NpcStep) { + applyNpcStep((NpcStep) currentQuestStep); + } else if (currentQuestStep instanceof ObjectStep) { + applyObjectStep((ObjectStep) currentQuestStep); + } else if (currentQuestStep instanceof DigStep) { + applyDigStep((DigStep) currentQuestStep); + } else if (currentQuestStep instanceof PuzzleStep) { + applyPuzzleStep((PuzzleStep) currentQuestStep); } if (!(questStep instanceof NpcStep) && !(questStep instanceof ObjectStep)) { @@ -927,7 +931,7 @@ private List collectAllItemRequirements(DetailedQuestStep questStep } } - QuestHelper selectedQuest = getQuestHelperPlugin().getSelectedQuest(); + QuestHelper selectedQuest = selectedQuestSnapshot(getQuestHelperPlugin()); if (selectedQuest != null) { updateEverHeldItemTracking(selectedQuest); @@ -1392,46 +1396,44 @@ public boolean applyStep(QuestStep step) { return true; } - private boolean executeQuestCustomLogic() { - var questLogic = QuestRegistry.getQuest(getQuestHelperPlugin().getSelectedQuest().getQuest().getId()); + private boolean executeQuestCustomLogic(IQuest questLogic) { if (questLogic instanceof PiratesTreasure) ((PiratesTreasure) questLogic).setMQuestPlugin(mQuestPlugin); return questLogic == null || questLogic.executeCustomLogic(); } public void onGraphicsObjectCreated(GraphicsObject graphicsObject) { - if (graphicsObject == null || getQuestHelperPlugin() == null - || getQuestHelperPlugin().getSelectedQuest() == null) { + if (graphicsObject == null) { return; } - var questLogic = QuestRegistry.getQuest( - getQuestHelperPlugin().getSelectedQuest().getQuest().getId()); + IQuest questLogic = questLogicFor(selectedQuestSnapshot(getQuestHelperPlugin())); if (questLogic != null && questLogic.onGraphicsObjectCreated(graphicsObject)) { nextCustomAttemptAt = 0; } } - private boolean runIdleCustomLogic(QuestStep step) { + private boolean runIdleCustomLogic(QuestStep step, IQuest questLogic) { long now = System.nanoTime(); if (lastCustomStep == step && now - nextCustomAttemptAt < 0) return !customActionPending; lastCustomStep = step; - nextCustomAttemptAt = now + customLogicIntervalNanos(); - customActionPending = !executeQuestCustomLogic(); + nextCustomAttemptAt = now + customLogicIntervalNanos(questLogic); + customActionPending = !executeQuestCustomLogic(questLogic); return !customActionPending; } - private long customLogicIntervalNanos() { - var questLogic = QuestRegistry.getQuest( - getQuestHelperPlugin().getSelectedQuest().getQuest().getId()); + private long customLogicIntervalNanos(IQuest questLogic) { return questLogic == null ? 600_000_000L : Math.max(0, questLogic.customLogicIntervalNanos()); } - private boolean customLogicRunsWhileAnimating() { - var questLogic = QuestRegistry.getQuest( - getQuestHelperPlugin().getSelectedQuest().getQuest().getId()); + private boolean customLogicRunsWhileAnimating(IQuest questLogic) { return questLogic != null && questLogic.customLogicRunsWhileAnimating(); } + private static IQuest questLogicFor(QuestHelper selectedQuest) { + QuestHelperQuest quest = selectedQuest == null ? null : selectedQuest.getQuest(); + return quest == null ? null : QuestRegistry.getQuest(quest.getId()); + } + static boolean shouldPauseBeforeCustomLogic(boolean inDialogue, boolean pending, boolean animating, boolean allowWhileAnimating) { return !inDialogue && (pending || (animating && !allowWhileAnimating)); @@ -1461,9 +1463,9 @@ private boolean isCurrentQuestStep(QuestStep step) { || mainScheduledFuture == null || mainScheduledFuture.isCancelled()) return false; return Microbot.getClientThread().runOnClientThreadOptional(() -> { QuestHelperPlugin plugin = getQuestHelperPlugin(); - return plugin != null && plugin.getSelectedQuest() != null - && plugin.getSelectedQuest().getCurrentStep() != null - && plugin.getSelectedQuest().getCurrentStep().getActiveStep() == step + QuestHelper selectedQuest = selectedQuestSnapshot(plugin); + return selectedQuest != null && selectedQuest.getCurrentStep() != null + && selectedQuest.getCurrentStep().getActiveStep() == step && Microbot.getVarbitValue(4606) == 0; }).orElse(false); } diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/logic/MisthalinMystery.java b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/logic/MisthalinMystery.java index 25ace23a81..9589afe10c 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/logic/MisthalinMystery.java +++ b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/logic/MisthalinMystery.java @@ -17,6 +17,7 @@ import net.runelite.client.plugins.microbot.Microbot; import net.runelite.client.plugins.microbot.api.npc.models.Rs2NpcModel; import net.runelite.client.plugins.microbot.questhelper.QuestHelperPlugin; +import net.runelite.client.plugins.microbot.questhelper.questhelpers.QuestHelper; import net.runelite.client.plugins.microbot.questhelper.steps.DetailedQuestStep; import net.runelite.client.plugins.microbot.questhelper.steps.ObjectStep; import net.runelite.client.plugins.microbot.questhelper.steps.QuestStep; @@ -83,8 +84,9 @@ public class MisthalinMystery extends BaseQuest public boolean executeCustomLogic() { QuestHelperPlugin plugin = getQuestHelperPlugin(); - if (plugin == null || plugin.getSelectedQuest() == null - || plugin.getSelectedQuest().getCurrentStep() == null) + QuestHelper selectedQuest = plugin == null ? null : plugin.getSelectedQuest(); + QuestStep currentStep = selectedQuest == null ? null : selectedQuest.getCurrentStep(); + if (currentStep == null) { approachSequence.reset(); resetDamagedWallApproach(); @@ -93,7 +95,7 @@ public boolean executeCustomLogic() return true; } - QuestStep step = plugin.getSelectedQuest().getCurrentStep().getActiveStep(); + QuestStep step = currentStep.getActiveStep(); if (!handleLaceyInterrupt( Rs2Dialogue.getQuestion(), Rs2Dialogue.hasDialogueOption(LACEY_INTERRUPT_ANSWER, true), @@ -212,12 +214,13 @@ public long customLogicIntervalNanos() public boolean customLogicRunsWhileAnimating() { QuestHelperPlugin plugin = getQuestHelperPlugin(); - if (plugin == null || plugin.getSelectedQuest() == null - || plugin.getSelectedQuest().getCurrentStep() == null) + QuestHelper selectedQuest = plugin == null ? null : plugin.getSelectedQuest(); + QuestStep currentStep = selectedQuest == null ? null : selectedQuest.getCurrentStep(); + if (currentStep == null) { return false; } - QuestStep step = plugin.getSelectedQuest().getCurrentStep().getActiveStep(); + QuestStep step = currentStep.getActiveStep(); return step instanceof DetailedQuestStep && isMirrorShowdownText(((DetailedQuestStep) step).getText()); } @@ -610,9 +613,9 @@ static void dispatchWaypoint(WorldPoint waypoint, Runnable canvasMove, Runnable private static boolean isActiveStep(QuestHelperPlugin plugin, QuestStep expected) { - return plugin.getSelectedQuest() != null - && plugin.getSelectedQuest().getCurrentStep() != null - && plugin.getSelectedQuest().getCurrentStep().getActiveStep() == expected; + QuestHelper selectedQuest = plugin == null ? null : plugin.getSelectedQuest(); + QuestStep currentStep = selectedQuest == null ? null : selectedQuest.getCurrentStep(); + return currentStep != null && currentStep.getActiveStep() == expected; } static boolean shouldStopRoute(boolean interrupted, boolean human, boolean paused, diff --git a/runelite-client/src/test/java/net/runelite/client/plugins/microbot/questhelper/QuestCustomLogicLifecycleTest.java b/runelite-client/src/test/java/net/runelite/client/plugins/microbot/questhelper/QuestCustomLogicLifecycleTest.java index e67ef6483e..c8a91e07d5 100644 --- a/runelite-client/src/test/java/net/runelite/client/plugins/microbot/questhelper/QuestCustomLogicLifecycleTest.java +++ b/runelite-client/src/test/java/net/runelite/client/plugins/microbot/questhelper/QuestCustomLogicLifecycleTest.java @@ -1,6 +1,7 @@ package net.runelite.client.plugins.microbot.questhelper; import java.io.InputStream; +import net.runelite.client.plugins.microbot.questhelper.questhelpers.QuestHelper; import org.junit.Test; import org.objectweb.asm.ClassReader; import org.objectweb.asm.tree.AbstractInsnNode; @@ -11,10 +12,104 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class QuestCustomLogicLifecycleTest { + @Test + public void selectedQuestIsCapturedWithOneNullablePluginRead() throws Exception + { + QuestHelperPlugin plugin = mock(QuestHelperPlugin.class); + QuestHelper selected = mock(QuestHelper.class); + when(plugin.getSelectedQuest()).thenReturn(selected, (QuestHelper) null); + + assertSame(selected, QuestScript.selectedQuestSnapshot(plugin)); + verify(plugin, times(1)).getSelectedQuest(); + assertNull(QuestScript.selectedQuestSnapshot(null)); + } + + @Test + public void selectedQuestReadsAreCentralized() throws Exception + { + ClassNode script = new ClassNode(); + try (InputStream input = QuestScript.class.getResourceAsStream("QuestScript.class")) + { + assertNotNull(input); + new ClassReader(input).accept(script, ClassReader.SKIP_FRAMES); + } + + int selectedQuestReads = 0; + for (MethodNode method : script.methods) + { + for (AbstractInsnNode instruction : method.instructions) + { + if (instruction instanceof MethodInsnNode) + { + MethodInsnNode call = (MethodInsnNode) instruction; + if (call.owner.endsWith("/QuestHelperPlugin") && call.name.equals("getSelectedQuest")) + { + selectedQuestReads++; + assertEquals("Only the null-safe snapshot helper may read the mutable selection", + "selectedQuestSnapshot", method.name); + } + } + } + } + assertEquals(1, selectedQuestReads); + } + + @Test + public void schedulerTickUsesOneQuestAndCurrentStepSnapshot() throws Exception + { + ClassNode script = new ClassNode(); + try (InputStream input = QuestScript.class.getResourceAsStream("QuestScript.class")) + { + assertNotNull(input); + new ClassReader(input).accept(script, ClassReader.SKIP_FRAMES); + } + + int schedulerTickMethods = 0; + for (MethodNode method : script.methods) + { + int selectedQuestSnapshots = 0; + int currentStepReads = 0; + boolean schedulerTick = false; + for (AbstractInsnNode instruction : method.instructions) + { + if (!(instruction instanceof MethodInsnNode)) + { + continue; + } + MethodInsnNode call = (MethodInsnNode) instruction; + if (call.owner.endsWith("/QuestScript") && call.name.equals("observePendingInteraction")) + { + schedulerTick = true; + } + if (call.owner.endsWith("/QuestScript") && call.name.equals("selectedQuestSnapshot")) + { + selectedQuestSnapshots++; + } + if (call.owner.endsWith("/QuestHelper") && call.name.equals("getCurrentStep")) + { + currentStepReads++; + } + } + if (schedulerTick) + { + schedulerTickMethods++; + assertEquals("A scheduler tick must use one selected-quest snapshot", 1, selectedQuestSnapshots); + assertEquals("A scheduler tick must use one current-step snapshot", 1, currentStepReads); + } + } + assertEquals(1, schedulerTickMethods); + } + @Test public void onlyOptedInCustomLogicRunsDuringAnimation() {