Add support for sorting ground items by price (#34)

[Infinitay]

New Feature

• Adds the ability to sort ground items by the max value between the GE and High Alch prices before sorting the ground items by the user's specified sorting list

New Additions

• Create a new method HotkeyableMenuSwapsPlugin#sortGroundItemsGE using HotkeyableMenuSwapsPlugin#sortGroundItems
  • The reason I am sorting the prices in the new method is to easily and cleanly sort the ground item menu entries before the custom sort from the user's list takes place
  • Also more readable code instead of doing the MenuEntry sorting before the highlightedItemValue != null || hiddenItemValue != null check and after
• Created the class GroundItemPriceSortMode to provide user's with a choice on how they want to sort the prices if they choose to enable the price sorting
• Added the new config item with the key "groundItemsPriceSortMethod" that stores the user's price sorting option

Changes

• Use Slf4j for debugging
  • Modified some old statements to use Sl4j and created new debug statements
• Use the new static coin id of 995 for coin ground items in GroundItemsStuff#buildGroundItem
  • Manually testing lists coins with the id of 995 instead of the previous id of 617
• Move re-used code for retrieving the GroundItem from the Scene to a new method HotkeyableMenuSwapsPlugin#getGroundItemFromScene
• Modified GroundItemsStuff#reloadGroundItemPluginLists with a new parameter shouldSortByPrice to account for price sorting

https://github.com/geheur/More-menu-entry-swaps/assets/6964154/813e1295-ac81-42a4-a4dc-029f66c83b71

When watching the video, the list under "Custom hides" is the proper ordering from most expensive to least. Please also keep in mind of the custom ground item order that I pass in later to show that it still works.

[geheur]

I think it would be confusing to have the list of ground items take priority - you can set values in it so it's weird that a value of "2" would go over an expensive item on the ground. Maybe these should be sorted together, with the value in the list overriding the ge price? Not sure which is better.

Also, how does it handle untradeable items? Lots of untradeable items show up at the top of the list for good reason. Would this sort them to the bottom? I feel like this might make this feature very annoying to use.

HA price max is weird because many items have significantly higher HA price than GE price, e.g. steel longsword at 110 vs 300. I would avoid doing that. Items worth alching are high enough value that this won't matter but for these low value items it will be confusing.

[Infinitay]

you can set values in it so it's weird that a value of "2" would go over an expensive item on the ground

That is my fault. I completely forgot to test it with positive values

Maybe these should be sorted together, with the value in the list overriding the ge price?

That was my intent. It crossed my mind to test the changes with a positive value and I did not see that wasn't the case

how does it handle untradeable items?

Lots of untradeable items show up at the top of the list for good reason

When I was making this feature I had the intent in mind that items with value > untradeables.

• In the wildy, aren't only your untradeables dropped? Doesn't all your other loot go to a lootkey?
• In PVM you will have a gravestone anyways

Would this sort them to the bottom?

Yes. I suppose I can add an option to always have it at the top if they choose to and make it sort after the custom list sort

HA price max is weird because many items have significantly higher HA price than GE price

The reason I did this is for two reasons

• I had a rare case where the GE price was not loading for ground items and it was showing it as 0 for everything I was dropping
• Another are those untradeable items on the GE. For example, you can't sell burnt food on the GE and so it will have a GE price of 0. However, if someone drops a stack of mills worth, it would never consider it because we wouldn't be checking the HA price too
  • Initially, I was going to only use HA price if GE price was 0, but I figured it would just be easier to take the max (again would prevent the first rare issue I faced of GE price not loading)

[geheur]

For the HA thing - maybe the dropdown can be disabled, ge, or highest ge/ha I don't like the idea of descending anyways because I don't know what anyone would ever use it for.

I think the untradeable thing is important because if you get a drop like a dragon defender it would be odd for it to show at the bottom. Maybe you can use the "price" (shop value)? That is what it sorts by normally, somewhat. Or maybe just HA value since people are way more familiar with that value.

[Infinitay]

Initially I wanted to make everything optional, but I feel like I have a bad habit of offering too much customization. I will go ahead and continue with it and make the changes since you approve of them. I will:

• Add an option for the user to sort pricing by GE price, High Alch price, or both (default)
• Add an option to always show untradeables on top (enabled by default because that is what you said the osrs behavior is)
  • I am considering on whether or not to allow a user's custom sort list to override this. What are your thoughts? For now, I am opting to allow the user's list to take precedence, but I am worried because I remember reading that you can associate max value to some entries. But I digress, the option does specify "on top" so I would want it to be at the very top no matter what. However, again, I feel like the user should be allowed to change which untradeable if they ever need to via the custom list sort. I would appreciate some input on this.

I don't like the idea of descending anyways because I don't know what anyone would ever use it for.

I think you mean ascending? I think everyone would want descending so it's from most expensive to least expensive. Back before there was loot keys in PVP, you would want to snag up the most expensive loot first and then get out as fast as you can. Although, since I'm already adding options for price sorting I will keep the sorting order option unless you really object it.

I think the untradeable thing is important

What if I were to make it so when the user selects to show untradeables on top, it will sort those untradeables by max(Store Price, High Alch Price)? Because I don't think some untradeables could be bought at stores just as some untradeables can't be alched.

[geheur]

Untradeables are not always on top. I believe it uses the "store price" (1.5x HA). I think using the HA price for these is fine, and just including them in the sort normally. FYI this store price thing exists for all items iirc.

The custom list should always override everything else.

I mean descending because players think of the list from top to bottom. Ascending has the cheapest stuff at the top. I see you did the opposite in the code but I don't think that is intuitive.

so tl;dr for changes as far as I can tell: make untradables worth ha value in sorting. options should be off, ge, or max(ha, ge) Make the custom list's values be used as values for the sorting, as overrides for the ge/ha price. The comments I made on the code.

[Infinitay]

I discussed it with you on GitHub briefly but I'll go ahead and write it here too

make untradables worth ha value in sorting.

I think RL already does this because the OSRS wiki for the Dragon Defender say's it is unalchable, but there is still a price return of 40804 by ItemComposition#getHaPrice. I also tested it with a firecape and it returned a value of around 39K.

make untradables worth ha value in sorting.

For the reason above, this is already done assuming `ItemComposition#getHaPrice does return a price for all untradeable items

options should be off, ge, or max(ha, ge)

I will go ahead and do this now as you suggested due to issues with items such as the steel sword. For items with a GE value of 0 I'll make it sort by the HA price instead.

Make the custom list's values be used as values for the sorting, as overrides for the ge/ha price.

This is already the behavior (first it will sort by GE price, then it will sort again using custom values (overrides ge sort), then untradeables always on top option (overrides ge price and custom value)

Although as mentioned previously, I want to add an option to always include untradeables at the top because of items like desert ammy having no price at all and being at the bottom

[Infinitay]

With the latest few commits, I replaced ground item price sorting order to a price sorting method instead such as Grand Exchange pricing or the maximum between the Grand Exchange and High Alchemy prices.

Config

• Rename key and method from "groundItemsPriceSortMethod" to "groundItemsPriceSortMode"
• Updated description for new options

Plugin

• Remove redundant and unreachable sorting order assignment
• Rename to "sortGroundItemsGE" to "sortGroundItemsByPrice"
• Refactored to use the new GroundItemPriceSortMode
• Remove price ordering sorting

GroundItemPriceSortMode

• Refactored enum for pricing mode instead of ordering mode (disabled, ge price, max(ge, ha))
• Added abstract item price method getItemPrice for each pricing mode

GroundItemsStuff

• Set the GroundItem tradeability equal to
  • The original item comp if the item is unnoted
  • The linked unnoted item
• Expand checks to set price and force tradable state for coins and plat tokens

Note, ended up leaving a debug statement so fixup'ed the commit and rebased

[Infinitay]

Refactored ground items price sorting by removing the separate sorting method and instead handling it within HotkeyableMenuSwapsPlugin#sortGroundItems instead

Previous behavior:

After:

[Infinitay]

I realized now there is some odd issue where changing the sort mode starts incrementing the count by * however many times you changed the sort mode but resets it back to 0 if you disable it. I guess that's why logging in GroundItemsStuff is being printed out multiple times and essentially isn't clearing the state. I'll work on this after I push the missing price sort mode check within #sortGroundItems

[Infinitay]

Fixed registering multiple instances of GroundItemsStuff to EventBus

GroundItemsStuff

• Add more logging
• Add didPricingModeChange parameter to #reloadGroundItemPluginLists
  • Fixes issue where the class would be registered to the EventBus multiple times when the method is called

HotkeyableMenuSwapsPlugin

• Add more logging
• Fixed issue where the class would be registered to the EventBus multiple times when the method is called
  • Add didPricingModeChange parameter to #reloadGroundItemSort
  • Add checks for the ground items key groundItemsPriceSortMode to make sure the option was changed either on/off
    • In the comments I write "register" or "don't register", but what I mean is whether or not to recognize (un)registering the event.

Preview

GE Value of items

https://github.com/geheur/More-menu-entry-swaps/assets/6964154/2c0de0e7-676a-4ac6-8cfe-14459fd82943

Final thoughts

This should be everything. If you could test out the plugin for yourself in case you can find any issues I would appreciate it if you find the time to do so. If you have any questions or issues with the code let me know. Thanks.