Feature mallprices.txt override data, caching, & sharing considerations

Rinn

Developer
I wanted to get opinions on mallprices.txt since this was an unintended side effect to the git migration and the change I made when I removed support for the manual point releases in favor of the automated builds every git commit, the version string used to only change for point releases where now it's updated for every new build. Whenever the version string changes all override data files are deleted on app launch and since the version string changes more frequently now any overrides are also deleted more frequently. Normally this is preferred because we want to clear out stale data when those overrides are packaged in the jar but an unintended side effect that is the user's mallprices.txt override is also deleted.

If users have sharePriceData enabled this is probably less of a concern because recent prices will be retrieved from the kolmafia server on login, but otherwise the user's historical prices will get wiped on app update and fall back to the mallprices.txt in the jar which hasn't been updated for over 6 months. It's worth noting sharePriceData is disabled by default.

The open questions I have are:
  • Should mallprices.txt be treated as an override in this way the same as the other data files? Maybe it should just stop being deleted and that will resolve this.
  • Is there a good reason to keep mallprices.txt in the jar when it's so far out of date? I don't think updating it in git is a good idea either since that would be a lot of churn for data that becomes stale relatively quickly.
  • None of the prices loaded from mallprices.txt will prevent a mall search even if they are very up to date, but should they if they are recent? I've realized there's MallPriceManager.mallPrices which is the price cache for the current session and will prevent additional searches if an entry is found, whereas MallPriceDatabase is what contains the historical prices and that is only checked if you pass maxAge to a search. maxAge is a good option but it's not available for functions that do internal searches such as retrieve_item(). This is something I'd like to add a setting for if that's reasonable.
  • Any thoughts on having mafia retrieve and update shared mall price data more often than once per login? Obviously we wouldn't want to do this too frequently and hammer the mafia server.
 
Are there non-mallbot reasons someone might want Mafia to have updated mall price data more than once/login? (I realize this may just be a reworded version of your last question, but it seems like a relevant enough concern to be asked explicitly.)
 

fronobulax

Developer
Staff member
I always share and update price data.

The data became less useful to me when I realized I had scripts that assumed the existence of an historical price meant that the item was currently in the mall. That was an error on my part, and I have addressed it, but the result is that entries in mallprices.txt older than a certain age have no relevance. For my occasional excursions into trends I much prefer ColdFront as a data source.

My sense is that the option to opt out of price sharing is an artifact from when Kolmafia chose to support Java 5 users on dial up lines. Those days are gone so I would first make sharing unconditionally true and disable (or hide) the option to opt out.

I can't figure out a use case for a user maintaining their own override so I would focus on making mafia "more robust" rather than "more flexible".

So I would pull the file out of the jar, always update once a day and always share.

I seem to recall a couple of cases where mafia would try and update a file and would delete the local copy and then realize the update failed, leaving the user with a missing file. I think that has been addressed but it couldn't hurt to confirm.
 

Veracity

Developer
Staff member
Six weeks ago, I extracted mall price related stuff from StoreManager and created MallPriceManager.


I worked on it extensively at that time, but not recently.

We have two kinds of mall price data:

That which you have learned this session as a result of doing mall searches. We save the search results for each such search. If you search one item at a time, you seem to get 30 results at a time. If you do bulk searches by category, you seem to get five. Anyway, it is those searches that we examine to find the "5th-lowest" price. Since that is the only price we make available to scripts, the frequency of searching and/or sharing the data is irrelevant to mallbot quashing.

If you use mall_price(item), this is the data you get - and if no search results are cached, a new mall search is performed. Also, if they are "too old".

Timestamped mall prices - no search results - are stored in mallprices.txt. These are "historical" prices. They are used in a variety of places - acquire, maximizer - avoid having to do a mall search, as long as the price is not "too old". You can access this in ASH with mall_price(item, maxAge). (Note that if maxAge is 0, it will do a mall search.) Commonly, "too old" is 7 days.

So, given that, mallprices.txt will cut down on mall searches only if it is relatively up-to-date; if all the prices are more than 7 days old, everything will result in a mall search.

1) Distributing mallprices.txt with 60-day-old prices is pointless.
- We should stop doing that.

2) When you log in, we will read mallprices.txt from the KoLmafia server (this one - kolmafia.us - kindly provided by fewyn.)
That is only as up-to-date as the general community of kolmafia users who share price data keep it.
Note that is you share prices, ash mall_prices("allitems") will do a bulk search of every item (7536 currently) in the mall, using 254 (currently) requests to KoL and will share the results with the community when you log out.
- If you are using the largesse of the community, you should give back.
-- sharePriceData should default to true.
-- perhaps if you opt out of sharing your own price data, you shouldn't benefit from sharing other peoples' price data?
-- or perhaps simply remove the setting; always read on login and always write on logout.

3) mallprices.txt is what you read at login + what you have learned on your own - and if you share to the community, even if the local file is subsequently deleted, it will be refreshed when you log in again and it re-reads the public file. Still, if you can't connect to the server (transiently), a local file is better than nothing.
- We should not delete mallprices.txt on updata.

4) "None of the prices loaded from mallprices.txt will prevent a mall search even if they are very up to date, but should they if they are recent?"
I'd say no. If you do an actual mall purchase, we update the cached mall search results - which can be important, if you are buying from stores with limits - or which are ignoring you.

5) "maxAge is a good option but it's not available for functions that do internal searches such as retrieve_item(). This is something I'd like to add a setting for if that's reasonable." Well... retrieve_item() (and acquire) call InventoryManager.retrieveItem.
Code:
  // Number of days which is considered "too old" for a cached mall price.
  public static final float MALL_PRICE_AGE = 7.0f;
I believe I changed "magic numbers" to a configuration constant, at least, but it is not parameterized. It is used in cheaperToBuy() - and in item_value() and priceToAcquire() if you are not using asking for "exact" values, which will use the locally cached mall search results from MallPriceManager.

Compare:

MallPriceManager.getMallPrice(AdventureResult item) - which uses local cache
MallPriceManager.getMallPrice(AdventureResult item, float maxAge) - which will likely defer to the other, if maxAge is 0, but not necessarily.

I wondered why the former couldn't simply be the equivalent of the latter with maxAge = 0, but didn't get far enough in my incremental refactoring of this code to actually try it out.

I will also point out that I will probably be in this code, myself, very soon. I want "priceToAcquire", when given a count of 20, say, return something closer to what it will actually cost, rather than 20 times the 5th-cheapest price. What with store limits and other factors, prices 6 and up can end up surprising you. Among other changes, that will probably require requesting/saving more than 5 mall search results per item. See my currently open "draft" PR.
 

Rinn

Developer
Okay all that all makes sense, I would personally like to see MALL_PRICE_AGE is taken into account more often to use the cached price, rather than doing a mall search even for standard normal mall_price calls but I understand why it doesn't work that way. MALL_PRICE_AGE should probably at least be a setting so it's not hardcoded.

I'm coming at this from the standpoint of I just want to stop doing so many mall searches generally because a vast majority of the time I'm willing to get less exact out of date results in favor of running my automation scripts faster, but I also don't want to go update a bunch of scripts I didn't write to modify their search behavior.

Anyway I'll pull mallprices from the override list so it stops being deleted and remove it from the jar to start.
 
Last edited:

Veracity

Developer
Staff member
By the way - looking at the code again (while reviewing your PR), I noticed that "sharePriceData" already controls both reading and writing. Which is to say, you don't share your own data, you don't get the benefit of others' data. As it should be.
 
I was running KoLmafia-26333 for the past two weeks and noticed historical prices recently had been returning 0, including prices of food items when all food item prices had been searched with mall_prices("food") in earlier mafia sessions on the same and previous days.

after reading this I was not able to reproduce in 26333 neither after changing "sharePriceData" to true nor when changing it back to false for testing, I don't know how the historical mallprices could have been cleared?
 
Top