Bug - Fixed Undesirable re-purchase of etched hourglass when not found in inventory

[katyarn]

Many players report that KoLMafia will buy grains of sand to make a new etched hourglass when attempting to use one as part of breakfast. The most common cause of this seems to be when the hourglass is left in Hangk's. KoLMafia will attempt to pull it, but if it doesn't register a success then it seems that it falls back to creating a new one. This can also occur if Hangk's is emptied but KoLMafia inventory gets desynchronized from KOL's. Perhaps there are other cases as well (closet?).

Right now inside of the BreakfastManager's useToys function, it attempts to pull items from storage and then creates a UseItemRequest. Later, it looks like the UseItemRequest attempts to call InventoryManager's retriveItem function. Maybe it is worth a special case to skip the creation of etched hourglasses in retrieveItem?

autoBuyPriceLimit also does not prevent this. Individually, grains of sand tend to cost much less than a typical player's price limits are set to. In aggregate, however, they will far exceed it. Perhaps functions calling the retrieveItem command should consider the total cost to create an item and check that against the autoBuyPriceLimit? Right now it looks like it only checks the cost of multiples of the end products.

 

[Veracity]

It will attempt to pull from Hagnk's if you "can interact". I.e., not Hardcore, and out of Ronin.
Also, assuming you have not disabled pulling from storage, for some reason.
You say "it doesn't register a success". Why not? Works every time for me.

If it has to buy sand, then, obviously, it can interact. Why not simply pull the finished item?
I suppose one explanation is that it thinks it can create a new item for less than the cost you can sell one.
In which case, your "valueOf Inventory" seems to be mis-set.

"This can also occur if Hangk's is emptied but KoLMafia inventory gets desynchronized from KOL's."
Wow. That would be a big bug. I have, literally, never seen that happen.
Can you give me a reproducible method that will demonstrate this? I will fix that bug, rather than put in a kludge to work around an issue supported only by anecdata.

"Perhaps functions calling retrieveItem should consider total cost to create and check that against autoBuyPriceLimit".
That is how it operates for me. It compares "costToCreate" vs. the preference.

I am astonished that so many things work differently for you than from how they are coded to act and how they behave for me.

Perhaps you can try these things:

set debugBuy=true
acquire? etched hourglass

I am loath to put in special handling for one item unless there is really a bug which is disobeying expected behavior.
I'd prefer to fix the bug rather than installing a kludge.

 

[Veracity]

Proposal:

Make default for "valueOfInventory" be 0.
Which is to say, pay no attention whatsoever to "autosell price" or "mall price" of an item you own.

We will automatically "retrieve" an item you already own, rather than considering that maybe you can autosell it for X Meat or the current mall price for it is XXXX Meat.

You want to pay attention to "how much can I sell this in the mall"? Then tweak that constant to reflect your personal behavior.

I have, literally, zero interest in "playing the mall". So "The minimum mall price of item X" is of zero interest to me. If I want to use item X, and own one, I have zero interest in "you can sell your item X for XXXX Meat and create a new one for less than that."

Because I am well aware that "somebody is offering item X for XXXX Meat" really means that "XXXX is the current price that somebody is NOT successfully selling item X for".

 

[Veracity]

I still like that - but since etched hourglass is not tradable, you either have one in a place you have configured as a retrievable place, or you have to create one.

You can disable "retrieve from storage", but if not, other than anecdata about "KoLmafia's concept of storage is desynched from KoL", that can't fail.
Similarly for closet.

With the talk of "retrieving from storage resulting in a failure", which I don't believe happens, I'm assuming user error.

Anecdata about "many players report this" reinforces that impression. The question is what, exactly, the user error is.

 

[katyarn]

I haven't personally encountered this issue. I'm just reporting it after seeing it occur to many players over the past few years. The above was my speculations about what causes it.

User A — 08/04/2020
Just lost 5 million meat because mafia decided I needed a new etched hourglass while running breakfast
I ran it before I pulled my stuff from hagnks after breaking prism, no idea why it wouldn't just pull it from hagnks

User B — 01/21/2022
My mafia bugged out and couldn't resolve storage properly. So when I ran breakfast, it tried to pull the etched hourglass. Failed, then decided I meant to make an etched hourglass
So now I'm broke

User C — 02/12/2022
kolmafia just made me a second hourglass; I finished my CS ascension, restarted mafia, and breakfast built me a new hourglass.
not sure this is the right channel to post. Do I potentially have a misconfiguration somewhere?
User B
I had that before, in my case its because I had an etched hourglass and it couldn't be pulled from storage. Because I messed with mafia while it was pulling from storage.
User C
that could be it. I have breakfast set to always run, and I pulled from storage as that was happening

User D — 03/31/2022
Ah, ran breakfast in the middle of pulling stuff from Hagnk. Mafia didn't find etched hourglass so it made a new one

User E — 05/07/2022
Oh lol, mafia tried to pull the etched hourglass (i forgot to empty hagnk's before starting garbo).
Instead of pulling it, for some reason, mafia created a new hourglass by buying up 10k grains of sand.
The "maximum mall price" protection didn't protected because sand is cheap-ish.

Now I have a new hourglass, freshly crafted for only... 8M meat :/
And the etched hourglass was pulled and used afterwards.

User F — 06/09/2022
well updated mafia decided to eat 7 million meat today, anyone want to buy some bacon and/or an empty hourglass?
it keeps doing it
trying to make etched hourglass
it was in hagnks the first time
it's not now
User G
refresh all?
User F
refresh fixed it

Above are relevant quotes I can find from users who encountered this issue.

Unfortunately I don't have any reproduction steps, but the simplest case here seems to be emptying Hangk's while breakfast is running. However that does not seem to be the only cause.

I don't think using KoLMafia while breakfast is running is a user error. KoLMafia doesn't do anything to prevent the user from interacting while that is running currently. Perhaps that should be changed?

If it has to buy sand, then, obviously, it can interact. Why not simply pull the finished item?
I suppose one explanation is that it thinks it can create a new item for less than the cost you can sell one.
In which case, your "valueOf Inventory" seems to be mis-set.

I don't think this property is the cause. Otherwise users would be seeing the issue on multiple days when calling breakfast whenever the hourglass was in Hangk's. So far it's been isolated to one day for each user.

"Perhaps functions calling retrieveItem should consider total cost to create and check that against autoBuyPriceLimit".
That is how it operates for me. It compares "costToCreate" vs. the preference.

I don't believe these users have set their autoBuyPriceLimits in the millions. I can try contacting them and confirming.

 

[Veracity]

OK. How about this?

> acquire? etched hourglass

Searching for "pile of sand"...
Search complete.
Searching for "grain of sand"...
Search complete.
Searching for "hourglass"...
Search complete.
Searching for "small pile of sand"...
Search complete.
Using cached search results for grain of sand...
Searching for "empty hourglass"...
Search complete.
Searching for "large pile of sand"...
Search complete.
Using cached search results for grain of sand...
Searching for "fire"...
Search complete.
The average amount of meat spent on components (6,496,048) for one etched hourglass exceeds autoBuyPriceLimit (20,000)
etched hourglass: fail

Turns out, it wasn't checking component costs unless you actually had enough components...

 

[Veracity]

I don't think using KoLMafia while breakfast is running is a user error. KoLMafia doesn't do anything to prevent the user from interacting while that is running currently. Perhaps that should be changed?

I think that doing something that depends on knowing what is in storage and what is in inventory, while something else that is modifying both of those places is running, is a user error. It would never occur to me to do that, since it seems obviously non-deterministic, to me.

 

[lostcalpolydude]

I know that pulling everything from storage (using KoL's built-in button equivalent) takes a while. Long enough that the request times out, in my experience. So internally, there's built-in detection that the request has timed out, and then a delay of 20(?) more seconds (I arbitrarily chose this, because it seemed long enough after testing a few times), and then inventory and such are refreshed to figure out what ultimately happened.

I always use "pull all" in the gCLI to empty storage. I think I tested once with the relay browser button at Hagnk's, because someone said it was working differently; I found that it was a different URL that did exactly the same thing, and updated code to handle that also.

All this to say, KoL makes emptying storage not the same clean request as other operations. Having not looked at any of the code in some time (or even played KoL really, the main thing keeping me logging in is giving out BAFH whitelists), it would not surprise me if something occasionally goes wrong with handling that request.

 

[katyarn]

User H — 04/14/2021
Strange interaction happened today: after finishing a HCCS, I ran:
cli_execute("pull all; breakfast");
and it proceeded to

pull: 1 etched hourglass

Create 3 meat paste
You acquire meat paste (3)
You lose 30 Meat
pull: 1156 grain of sand

buy 5 grain of sand for 100 each from shop #806711 on 20210414
buy 1 grain of sand for 400 each from shop #3186943 on 20210414
buy 84 grain of sand for 554 each from shop #3390526 on 20210414
buy 30 grain of sand for 555 each from shop #21315 on 20210414
buy 5487 grain of sand for 949 each from shop #177122 on 20210414
buy 273 grain of sand for 949 each from shop #412289 on 20210414
buy 4120 grain of sand for 950 each from shop #2582373 on 20210414

Use 10000 grain of sand

... and so on, until it created a second hourglass in my inventory and used it.

Here's another case someone encountered. This one is important because it highlights that KoLMafia can end up in a strange state after emptying Hangk's.

 

[Veracity]

Read lost’s explanation.

I stopped doing “pull all” when I started occasionally doing PVP years ago. It’s been years - and I will never do it again.

Perhaps those who want to do that should do “refresh storage; refresh inventory; refresh closet” after doing do.

Actually, I thought KoLmafia did that itself, since only KoL knows what is destined for inventory and what for the closet.

 

[phreddrickk]

Perhaps those who want to do that should do “refresh storage; refresh inventory; refresh closet” after doing do.

Would it be helpful for me to throw in a PR to do just that after a "pull all"?

 

[fronobulax]

Would it be helpful for me to throw in a PR to do just that after a "pull all"?

My experience with synch problems after pull all seems to be rooted in the fact that something else was being done while the pull all was executing. Thus my hypothesis is that the root cause was that one or more inventory operations was not thread safe. A refresh whatever probably won't hurt but if there are several thousand unique items in at least one choice of whatever the total time to complete will be noticeably longer.

 

[Veracity]

Would it be helpful for me to throw in a PR to do just that after a "pull all"?

Well....

StorageRequest.parseTransfer will call StorageRequest.emptyStorage(urlString) if it sees Hagnk mentioning that "all your stuff" is being retrieved.

Looks to me like StorageRequest.emptyStorage does refresh Inventory, Closet - and if you only wanted to pull your favs, also Storage.

If we believe that KoL is lying by saying it pulled "all your stuff" when it might have errored out and only pulled some, how could we detect this?
Or, perhaps, ALWAYS refresh Storage, Just In Case?

We refresh inventory, closet, and storage via calls to api.php - JSON - not by parsing ginormous HTML pages.

I have tons of items in storage. The JSON result string is 86904 characters with 7368 different items.
It takes a fraction of a second to refresh storage for me.

 

[fronobulax]

Using the relay browser initiation of pull all it takes anywhere from 2 to 5 minutes of wall clock time before the relay browser and gCLI indicate to me that they are done. I presume my laptop and/or internet connection are slower than other people's or it may be my speed is dependent upon having thousands of unique items in Hagnk's. Upon several occasions, long, long ago I observed synch issues. I decided that they were caused by doing "other things" while Hagnk's was emptying, hypothesized that something related to addition, deletion or iteration was not thread safe for inventory and chose "don't do anything else while pulling all" as an acceptable response.

 

[katyarn]

Spending a few more seconds after a few minutes of waiting once (or twice) per day is a good tradeoff for data integrity and preventing other issues like this. Maybe KoLMafia could prevent user interaction with other pages until it has finished the empty storage request as another safeguard?

 

[phreddrickk]

I started getting this error a little bit after this fix was rolled out:

> js maximize("sleaze dmg, sleaze spell dmg", false)

Maximizer: sleaze dmg, sleaze spell dmg
Maximizing...
96 combinations checked, best score 411.00
Searching for "lewd playing card"...
Search complete.
The average amount of meat spent on components (1,976,000) for one deck of lewd playing cards exceeds autoBuyPriceLimit (50,000)
Returned: false

I own two decks of lewd playing cards. My Left-Hand Man is out, but is not otherwise occupied. Interestingly, running this string through the maximizer directly seems to work fine, but when I look at the maximizer after running the maximize function, I see

• fail & equip familiar deck of lewd playing cards (+138)

Given that this talks about the autoBuyPriceLimit and the average meat spent, and also given that it started happening very recently, I feel like this belongs here. But I can make a new thread if that's better.

When I manually equip my second deck, the maximizer function runs just fine.

 

[Veracity]

The maximizer uses simRetrieveItem to figure out how & where to get an item is is interested in.
It looks like InventoryManager.retrieveItem should not log that error message if it is doing a "sim" retrieve.

          makeFromComponents = false;

         KoLmafia.updateDisplay(
              MafiaState.ERROR,
              "The average amount of meat spent on components ("
                  + KoLConstants.COMMA_FORMAT.format(meatSpend)
                  + ") for one "
                  + item.getName()
                  + " exceeds autoBuyPriceLimit ("
                  + KoLConstants.COMMA_FORMAT.format(autoBuyPriceLimit)
                  + ")");

          // If making it from components was cheaper than buying the final product, and we
          // couldn't afford to make it, don't bother trying to buy the final product.
          shouldUseMall = false;

In fact, the make from components message shouldn't be an ERROR. It doesn't return immediately when it detects the issue; it continues on looking for other methods to get the item, having eliminated "makeFromComponents and shouldUseMall".

Which is funny, because it makes exactly two further checks:

      if (makeFromComponents) {
        if (sim) {
          return "create";
        }

and

    if (shouldUseMall) {
      if (sim) {
        return "buy";
      }

ending with:

    if (sim) {
      return "fail";
    }

    KoLmafia.updateDisplay(
        MafiaState.ERROR, "You need " + missingCount + " more " + item.getName() + " to continue.");

    return null;

- the message should not be an error; it is informative, and a real ERROR will be generated later.
- It need not be displayed if "sim". probably.