From 27b5d7308f23411092d5d1f5e2bb0ca214ab5ace Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 12 Sep 2019 21:08:04 -0400 Subject: [PATCH] menu entry swapper: fix menu searching optimization --- .../MenuEntrySwapperPlugin.java | 230 ++++++++++-------- .../MenuEntrySwapperPluginTest.java | 212 ++++++++++++++++ 2 files changed, 347 insertions(+), 95 deletions(-) create mode 100644 runelite-client/src/test/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPluginTest.java diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPlugin.java b/runelite-client/src/main/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPlugin.java index 8c0e99df93..74241613f2 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPlugin.java +++ b/runelite-client/src/main/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPlugin.java @@ -39,9 +39,9 @@ import net.runelite.api.ItemComposition; import net.runelite.api.MenuAction; import net.runelite.api.MenuEntry; import net.runelite.api.NPC; +import net.runelite.api.events.ClientTick; import net.runelite.api.events.ConfigChanged; import net.runelite.api.events.FocusChanged; -import net.runelite.api.events.MenuEntryAdded; import net.runelite.api.events.MenuOpened; import net.runelite.api.events.MenuOptionClicked; import net.runelite.api.events.PostItemComposition; @@ -355,31 +355,16 @@ public class MenuEntrySwapperPlugin extends Plugin } } - @Subscribe - public void onMenuEntryAdded(MenuEntryAdded event) + private void swapMenuEntry(int index, MenuEntry menuEntry) { - final String option = Text.removeTags(event.getOption()).toLowerCase(); - - if (event.getType() == MenuAction.CANCEL.getId()) - { - optionIndexes.clear(); - } - - int size = optionIndexes.size(); - optionIndexes.put(option, size); - - if (client.getGameState() != GameState.LOGGED_IN) - { - return; - } - - final int eventId = event.getIdentifier(); - final String target = Text.removeTags(event.getTarget()).toLowerCase(); + final int eventId = menuEntry.getIdentifier(); + final String option = Text.removeTags(menuEntry.getOption()).toLowerCase(); + final String target = Text.removeTags(menuEntry.getTarget()).toLowerCase(); final NPC hintArrowNpc = client.getHintArrowNpc(); if (hintArrowNpc != null && hintArrowNpc.getIndex() == eventId - && NPC_MENU_TYPES.contains(MenuAction.of(event.getType()))) + && NPC_MENU_TYPES.contains(MenuAction.of(menuEntry.getType()))) { return; } @@ -388,128 +373,128 @@ public class MenuEntrySwapperPlugin extends Plugin { if (config.swapPickpocket() && shouldSwapPickpocket(target)) { - swap("pickpocket", option, target, true); + swap("pickpocket", option, target, index); } if (config.swapAbyssTeleport() && target.contains("mage of zamorak")) { - swap("teleport", option, target, true); + swap("teleport", option, target, index); } if (config.swapHardWoodGrove() && target.contains("rionasta")) { - swap("send-parcel", option, target, true); + swap("send-parcel", option, target, index); } if (config.swapBank()) { - swap("bank", option, target, true); + swap("bank", option, target, index); } if (config.swapContract()) { - swap("contract", option, target, true); + swap("contract", option, target, index); } if (config.swapExchange()) { - swap("exchange", option, target, true); + swap("exchange", option, target, index); } if (config.swapDarkMage()) { - swap("repairs", option, target, true); + swap("repairs", option, target, index); } // make sure assignment swap is higher priority than trade swap for slayer masters if (config.swapAssignment()) { - swap("assignment", option, target, true); + swap("assignment", option, target, index); } if (config.swapTrade()) { - swap("trade", option, target, true); - swap("trade-with", option, target, true); - swap("shop", option, target, true); + swap("trade", option, target, index); + swap("trade-with", option, target, index); + swap("shop", option, target, index); } if (config.claimSlime() && target.equals("robin")) { - swap("claim-slime", option, target, true); + swap("claim-slime", option, target, index); } if (config.swapTravel()) { - swap("travel", option, target, true); - swap("pay-fare", option, target, true); - swap("charter", option, target, true); - swap("take-boat", option, target, true); - swap("fly", option, target, true); - swap("jatizso", option, target, true); - swap("neitiznot", option, target, true); - swap("rellekka", option, target, true); - swap("follow", option, target, true); - swap("transport", option, target, true); + swap("travel", option, target, index); + swap("pay-fare", option, target, index); + swap("charter", option, target, index); + swap("take-boat", option, target, index); + swap("fly", option, target, index); + swap("jatizso", option, target, index); + swap("neitiznot", option, target, index); + swap("rellekka", option, target, index); + swap("follow", option, target, index); + swap("transport", option, target, index); } if (config.swapPay()) { - swap("pay", option, target, true); - swap("pay (", option, target, false); + swap("pay", option, target, index); + swapContains("pay (", option, target, index); } if (config.swapDecant()) { - swap("decant", option, target, true); + swap("decant", option, target, index); } if (config.swapQuick()) { - swap("quick-travel", option, target, true); + swap("quick-travel", option, target, index); } if (config.swapEnchant()) { - swap("enchant", option, target, true); + swap("enchant", option, target, index); } } else if (config.swapTravel() && option.equals("pass") && target.equals("energy barrier")) { - swap("pay-toll(2-ecto)", option, target, true); + swap("pay-toll(2-ecto)", option, target, index); } else if (config.swapTravel() && option.equals("open") && target.equals("gate")) { - swap("pay-toll(10gp)", option, target, true); + swap("pay-toll(10gp)", option, target, index); } else if (config.swapHardWoodGrove() && option.equals("open") && target.equals("hardwood grove doors")) { - swap("quick-pay(100)", option, target, true); + swap("quick-pay(100)", option, target, index); } else if (config.swapTravel() && option.equals("inspect") && target.equals("trapdoor")) { - swap("travel", option, target, true); + swap("travel", option, target, index); } else if (config.swapHarpoon() && option.equals("cage")) { - swap("harpoon", option, target, true); + swap("harpoon", option, target, index); } else if (config.swapHarpoon() && (option.equals("big net") || option.equals("net"))) { - swap("harpoon", option, target, true); + swap("harpoon", option, target, index); } else if (config.swapHomePortal() != HouseMode.ENTER && option.equals("enter")) { switch (config.swapHomePortal()) { case HOME: - swap("home", option, target, true); + swap("home", option, target, index); break; case BUILD_MODE: - swap("build mode", option, target, true); + swap("build mode", option, target, index); break; case FRIENDS_HOUSE: - swap("friend's house", option, target, true); + swap("friend's house", option, target, index); break; } } @@ -518,68 +503,68 @@ public class MenuEntrySwapperPlugin extends Plugin { if (config.swapFairyRing() == FairyRingMode.LAST_DESTINATION) { - swap("last-destination", option, target, false); + swapContains("last-destination", option, target, index); } else if (config.swapFairyRing() == FairyRingMode.CONFIGURE) { - swap("configure", option, target, false); + swapContains("configure", option, target, index); } } else if (config.swapFairyRing() == FairyRingMode.ZANARIS && option.equals("tree")) { - swap("zanaris", option, target, false); + swapContains("zanaris", option, target, index); } else if (config.swapBoxTrap() && (option.equals("check") || option.equals("dismantle"))) { - swap("reset", option, target, true); + swap("reset", option, target, index); } else if (config.swapBoxTrap() && option.equals("take")) { - swap("lay", option, target, true); + swap("lay", option, target, index); } else if (config.swapChase() && option.equals("pick-up")) { - swap("chase", option, target, true); + swap("chase", option, target, index); } else if (config.swapBirdhouseEmpty() && option.equals("interact") && target.contains("birdhouse")) { - swap("empty", option, target, true); + swap("empty", option, target, index); } else if (config.swapQuick() && option.equals("enter")) { - swap("quick-enter", option, target, true); + swap("quick-enter", option, target, index); } else if (config.swapQuick() && option.equals("ring")) { - swap("quick-start", option, target, true); + swap("quick-start", option, target, index); } else if (config.swapQuick() && option.equals("pass")) { - swap("quick-pass", option, target, true); - swap("quick pass", option, target, true); + swap("quick-pass", option, target, index); + swap("quick pass", option, target, index); } else if (config.swapQuick() && option.equals("open")) { - swap("quick-open", option, target, true); + swap("quick-open", option, target, index); } else if (config.swapQuick() && option.equals("climb-down")) { - swap("quick-start", option, target, true); - swap("pay", option, target, true); + swap("quick-start", option, target, index); + swap("pay", option, target, index); } else if (config.swapAdmire() && option.equals("admire")) { - swap("teleport", option, target, true); - swap("spellbook", option, target, true); - swap("perks", option, target, true); + swap("teleport", option, target, index); + swap("spellbook", option, target, index); + swap("perks", option, target, index); } else if (config.swapPrivate() && option.equals("shared")) { - swap("private", option, target, true); + swap("private", option, target, index); } else if (config.swapPick() && option.equals("pick")) { - swap("pick-lots", option, target, true); + swap("pick-lots", option, target, index); } else if (config.shiftClickCustomization() && shiftModifier && !option.equals("use")) { @@ -587,25 +572,25 @@ public class MenuEntrySwapperPlugin extends Plugin if (customOption != null && customOption == -1) { - swap("use", option, target, true); + swap("use", option, target, index); } } // Put all item-related swapping after shift-click else if (config.swapTeleportItem() && option.equals("wear")) { - swap("rub", option, target, true); - swap("teleport", option, target, true); + swap("rub", option, target, index); + swap("teleport", option, target, index); } else if (option.equals("wield")) { if (config.swapTeleportItem()) { - swap("teleport", option, target, true); + swap("teleport", option, target, index); } } else if (config.swapBones() && option.equals("bury")) { - swap("use", option, target, true); + swap("use", option, target, index); } } @@ -614,6 +599,35 @@ public class MenuEntrySwapperPlugin extends Plugin return !target.startsWith("villager") && !target.startsWith("bandit") && !target.startsWith("menaphite thug"); } + @Subscribe + public void onClientTick(ClientTick clientTick) + { + // The menu is not rebuilt when it is open, so don't swap or else it will + // repeatedly swap entries + if (client.getGameState() != GameState.LOGGED_IN || client.isMenuOpen()) + { + return; + } + + MenuEntry[] menuEntries = client.getMenuEntries(); + + // Build option map for quick lookup in findIndex + int idx = 0; + optionIndexes.clear(); + for (MenuEntry entry : menuEntries) + { + String option = Text.removeTags(entry.getOption()).toLowerCase(); + optionIndexes.put(option, idx++); + } + + // Perform swaps + idx = 0; + for (MenuEntry entry : menuEntries) + { + swapMenuEntry(idx++, entry); + } + } + @Subscribe public void onPostItemComposition(PostItemComposition event) { @@ -635,7 +649,30 @@ public class MenuEntrySwapperPlugin extends Plugin } } - private int searchIndex(MenuEntry[] entries, String option, String target, boolean strict) + private void swap(String optionA, String optionB, String target, int index) + { + swap(optionA, optionB, target, index, true); + } + + private void swapContains(String optionA, String optionB, String target, int index) + { + swap(optionA, optionB, target, index, false); + } + + private void swap(String optionA, String optionB, String target, int index, boolean strict) + { + MenuEntry[] menuEntries = client.getMenuEntries(); + + int thisIndex = findIndex(menuEntries, index, optionB, target, strict); + int optionIdx = findIndex(menuEntries, thisIndex, optionA, target, strict); + + if (thisIndex >= 0 && optionIdx >= 0) + { + swap(optionIndexes, menuEntries, optionIdx, thisIndex); + } + } + + private int findIndex(MenuEntry[] entries, int limit, String option, String target, boolean strict) { if (strict) { @@ -649,7 +686,8 @@ public class MenuEntrySwapperPlugin extends Plugin MenuEntry entry = entries[idx]; String entryTarget = Text.removeTags(entry.getTarget()).toLowerCase(); - if (entryTarget.equals(target)) + // Limit to the last index which is prior to the current entry + if (idx <= limit && entryTarget.equals(target)) { return idx; } @@ -657,8 +695,8 @@ public class MenuEntrySwapperPlugin extends Plugin } else { - // Without strict matching we have to iterate all entries... - for (int i = entries.length - 1; i >= 0; i--) + // Without strict matching we have to iterate all entries up to the current limit... + for (int i = limit; i >= 0; i--) { MenuEntry entry = entries[i]; String entryOption = Text.removeTags(entry.getOption()).toLowerCase(); @@ -669,25 +707,27 @@ public class MenuEntrySwapperPlugin extends Plugin return i; } } + } return -1; } - private void swap(String optionA, String optionB, String target, boolean strict) + private void swap(ArrayListMultimap optionIndexes, MenuEntry[] entries, int index1, int index2) { - MenuEntry[] entries = client.getMenuEntries(); + MenuEntry entry = entries[index1]; + entries[index1] = entries[index2]; + entries[index2] = entry; - int idxA = searchIndex(entries, optionA, target, strict); - int idxB = searchIndex(entries, optionB, target, strict); + client.setMenuEntries(entries); - if (idxA >= 0 && idxB >= 0) + // Rebuild option indexes + optionIndexes.clear(); + int idx = 0; + for (MenuEntry menuEntry : entries) { - MenuEntry entry = entries[idxA]; - entries[idxA] = entries[idxB]; - entries[idxB] = entry; - - client.setMenuEntries(entries); + String option = Text.removeTags(menuEntry.getOption()).toLowerCase(); + optionIndexes.put(option, idx++); } } diff --git a/runelite-client/src/test/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPluginTest.java b/runelite-client/src/test/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPluginTest.java new file mode 100644 index 0000000000..fd0fe86f91 --- /dev/null +++ b/runelite-client/src/test/java/net/runelite/client/plugins/menuentryswapper/MenuEntrySwapperPluginTest.java @@ -0,0 +1,212 @@ +/* + * Copyright (c) 2019, Adam + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package net.runelite.client.plugins.menuentryswapper; + +import com.google.inject.Guice; +import com.google.inject.Inject; +import com.google.inject.testing.fieldbinder.Bind; +import com.google.inject.testing.fieldbinder.BoundFieldModule; +import net.runelite.api.Client; +import net.runelite.api.GameState; +import net.runelite.api.MenuAction; +import net.runelite.api.MenuEntry; +import net.runelite.api.events.ClientTick; +import net.runelite.client.config.ConfigManager; +import net.runelite.client.game.ItemManager; +import static org.junit.Assert.assertArrayEquals; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import static org.mockito.Matchers.any; +import org.mockito.Mock; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; + +@RunWith(MockitoJUnitRunner.class) +public class MenuEntrySwapperPluginTest +{ + @Mock + @Bind + Client client; + + @Mock + @Bind + ConfigManager configManager; + + @Mock + @Bind + ItemManager itemManager; + + @Mock + @Bind + MenuEntrySwapperConfig config; + + @Inject + MenuEntrySwapperPlugin menuEntrySwapperPlugin; + + private MenuEntry[] entries; + + @Before + public void before() + { + Guice.createInjector(BoundFieldModule.of(this)).injectMembers(this); + + when(client.getGameState()).thenReturn(GameState.LOGGED_IN); + + when(client.getMenuEntries()).thenAnswer((Answer) invocationOnMock -> + { + // The menu implementation returns a copy of the array, which causes swap() to not + // modify the same array being iterated in onClientTick + MenuEntry[] copy = new MenuEntry[entries.length]; + System.arraycopy(entries, 0, copy, 0, entries.length); + return copy; + }); + doAnswer((Answer) invocationOnMock -> + { + Object argument = invocationOnMock.getArguments()[0]; + entries = (MenuEntry[]) argument; + return null; + }).when(client).setMenuEntries(any(MenuEntry[].class)); + } + + private static MenuEntry menu(String option, String target, MenuAction menuAction) + { + MenuEntry menuEntry = new MenuEntry(); + menuEntry.setOption(option); + menuEntry.setTarget(target); + menuEntry.setType(menuAction.getId()); + return menuEntry; + } + + @Test + public void testSlayerMaster() + { + when(config.swapTrade()).thenReturn(true); + when(config.swapAssignment()).thenReturn(true); + + entries = new MenuEntry[]{ + menu("Cancel", "", MenuAction.CANCEL), + menu("Rewards", "Duradel", MenuAction.NPC_FIFTH_OPTION), + menu("Trade", "Duradel", MenuAction.NPC_FOURTH_OPTION), + menu("Assignment", "Duradel", MenuAction.NPC_THIRD_OPTION), + menu("Talk-to", "Duradel", MenuAction.NPC_FIRST_OPTION), + }; + menuEntrySwapperPlugin.onClientTick(new ClientTick()); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(MenuEntry[].class); + // Once for assignment<->talk-to and once for trade<->talk-to + verify(client, times(2)).setMenuEntries(argumentCaptor.capture()); + + MenuEntry[] value = argumentCaptor.getValue(); + assertArrayEquals(new MenuEntry[]{ + menu("Cancel", "", MenuAction.CANCEL), + menu("Rewards", "Duradel", MenuAction.NPC_FIFTH_OPTION), + menu("Talk-to", "Duradel", MenuAction.NPC_FIRST_OPTION), + menu("Trade", "Duradel", MenuAction.NPC_FOURTH_OPTION), + menu("Assignment", "Duradel", MenuAction.NPC_THIRD_OPTION), + }, argumentCaptor.getValue()); + } + + @Test + public void testBankers() + { + when(config.swapBank()).thenReturn(true); + + entries = new MenuEntry[]{ + menu("Cancel", "", MenuAction.CANCEL), + menu("Examine", "Gnome banker", MenuAction.EXAMINE_NPC), + menu("Examine", "Gnome banker", MenuAction.EXAMINE_NPC), + menu("Walk here", "", MenuAction.WALK), + + // Banker 2 + menu("Collect", "Gnome banker", MenuAction.NPC_FOURTH_OPTION), + menu("Bank", "Gnome banker", MenuAction.NPC_THIRD_OPTION), + menu("Talk-to", "Gnome banker", MenuAction.NPC_FIRST_OPTION), + + // Banker 1 + menu("Collect", "Gnome banker", MenuAction.NPC_FOURTH_OPTION), + menu("Bank", "Gnome banker", MenuAction.NPC_THIRD_OPTION), + menu("Talk-to", "Gnome banker", MenuAction.NPC_FIRST_OPTION), + }; + + menuEntrySwapperPlugin.onClientTick(new ClientTick()); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(MenuEntry[].class); + verify(client, times(2)).setMenuEntries(argumentCaptor.capture()); + + assertArrayEquals(new MenuEntry[]{ + menu("Cancel", "", MenuAction.CANCEL), + menu("Examine", "Gnome banker", MenuAction.EXAMINE_NPC), + menu("Examine", "Gnome banker", MenuAction.EXAMINE_NPC), + menu("Walk here", "", MenuAction.WALK), + + // Banker 2 + menu("Collect", "Gnome banker", MenuAction.NPC_FOURTH_OPTION), + menu("Talk-to", "Gnome banker", MenuAction.NPC_FIRST_OPTION), + menu("Bank", "Gnome banker", MenuAction.NPC_THIRD_OPTION), + + // Banker 1 + menu("Collect", "Gnome banker", MenuAction.NPC_FOURTH_OPTION), + menu("Talk-to", "Gnome banker", MenuAction.NPC_FIRST_OPTION), + menu("Bank", "Gnome banker", MenuAction.NPC_THIRD_OPTION), + }, argumentCaptor.getValue()); + } + + @Test + public void testContains() + { + when(config.swapPay()).thenReturn(true); + + entries = new MenuEntry[]{ + menu("Cancel", "", MenuAction.CANCEL), + menu("Examine", "Kragen", MenuAction.EXAMINE_NPC), + menu("Walk here", "", MenuAction.WALK), + + menu("Pay (south)", "Kragen", MenuAction.NPC_FOURTH_OPTION), + menu("Pay (north)", "Kragen", MenuAction.NPC_THIRD_OPTION), + menu("Talk-to", "Kragen", MenuAction.NPC_FIRST_OPTION), + }; + + menuEntrySwapperPlugin.onClientTick(new ClientTick()); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(MenuEntry[].class); + verify(client).setMenuEntries(argumentCaptor.capture()); + + assertArrayEquals(new MenuEntry[]{ + menu("Cancel", "", MenuAction.CANCEL), + menu("Examine", "Kragen", MenuAction.EXAMINE_NPC), + menu("Walk here", "", MenuAction.WALK), + + menu("Pay (south)", "Kragen", MenuAction.NPC_FOURTH_OPTION), + menu("Talk-to", "Kragen", MenuAction.NPC_FIRST_OPTION), + menu("Pay (north)", "Kragen", MenuAction.NPC_THIRD_OPTION), + }, argumentCaptor.getValue()); + } +} \ No newline at end of file