Cool, those functions are a lot more robust than what I had. Essentially same outcome, but better implementation.
When comparing the price to make to the user's autoBuyPriceLimit, I think we should value inventory as 0 meat.
Agreed, and I like the solution that you ended up at.
Is this behavior satisfying?
The only remaining issue I can see with this implementation is maybe some fringe cases where using the average meatSpend causes confusing behavior. Like, if I ask mafia to get two pumpkin pies and I only have one pumpkin on hand, it would see that the priceToMake of the first one is low, and the priceToMake of the second is high. (assuming pumpkin price of 10k) There's a window of aBPLs from 5k to 10k where it would reject buying a single pumpkin to make my pie, but be okay with acquiring one if I ask it to get two and already have one on hand. That's a little funky.
..Actually wait, I think the extant code in KoLMafia.java will still stop any single purchases that exceed the users aBPL. So that should be okay, I think. It's a little different behavior than when we stop the WHOLE acquisition process, as you can see:
Code:
> set autoBuyPriceLimit=6000
autoBuyPriceLimit => 6000
> closet put pumpkin
Placing items into closet...
Requests complete.
> acquire pumpkin pie
☯ Gnollish pie tin mall=100 make=2147483647
☯ flat dough mall=100 make=2147483647
Searching for "wad of dough"...
Search complete.
Searching for "flat dough"...
Search complete.
☯ pie crust mall=100 make=200
☯ pumpkin mall=9100 make=2147483647
Searching for "pumpkin pie"...
Search complete.
Searching for "pie crust"...
Search complete.
Searching for "Gnollish pie tin"...
Search complete.
☯ flat dough mall=100 make=100
☯ wad of dough mall=100 make=100
☯ pie crust mall=100 make=200
☯ pumpkin mall=9100 make=2147483647
☯ pumpkin pie mall=12770 make=9200
☯ Gnollish pie tin mall=100 make=2147483647
☯ flat dough mall=100 make=100
☯ wad of dough mall=100 make=100
☯ pie crust mall=100 make=200
☯ pumpkin mall=9100 make=2147483647
The average amount of meat spend on components (9,200) for one pumpkin pie exceeds autoBuyPriceLimit (6,000)
You need 1 more pumpkin pie to continue.
> closet take pumpkin
Removing items from closet...
You acquire an item: pumpkin
Requests complete.
> acquire 2 pumpkin pie
☯ Gnollish pie tin (2) mall=200 make=2147483647
☯ flat dough (2) mall=200 make=2147483647
☯ wad of dough (2) mall=200 make=200
☯ pie crust (2) mall=200 make=400
☯ pumpkin mall=16379 make=2147483647
☯ Gnollish pie tin (2) mall=200 make=2147483647
☯ flat dough (2) mall=200 make=2147483647
☯ wad of dough (2) mall=200 make=200
☯ flat dough (2) mall=200 make=200
☯ wad of dough (2) mall=200 make=200
☯ pie crust (2) mall=200 make=400
☯ pumpkin mall=16379 make=2147483647
☯ pumpkin pie (2) mall=25540 make=16579
☯ Gnollish pie tin (2) mall=200 make=2147483647
☯ flat dough (2) mall=200 make=200
☯ wad of dough (2) mall=200 make=200
☯ pie crust (2) mall=200 make=400
☯ pumpkin mall=9100 make=2147483647
Verifying ingredients for pumpkin pie (2)...
☯ Gnollish pie tin (2) mall=200 make=2147483647
☯ wad of dough (2) mall=200 make=2147483647
☯ flat dough (2) mall=200 make=200
☯ wad of dough (2) mall=200 make=200
☯ pie crust (2) mall=200 make=400
Searching for "pie crust"...
Search complete.
Purchasing pie crust (2 @ 100)...
You acquire pie crust (2)
Purchases complete.
Searching for "pumpkin"...
Search complete.
Stopped purchasing pumpkin @ 9,100.
Using cached search results for pumpkin...
Stopped purchasing pumpkin @ 9,100.
You need 1 more pumpkin to continue.
> inv pumpkin
pumpkin
Looks fine to me. Note that I sanitized the above by removing umpteen flat dough, wad of dough recursive checks, so if the debugBuy looks a little goofy, that's why.
Could the error message be more explicit?
I think it's good. Though I think proper grammar for "The average amount of meat spend on components" should either be "spent" or "to spend."
EDIT: I added a check for isAutomated too, so that the autoBuyPriceLimit check is bypassed for non-automated retrieveItem() calls. Although I don't think Mafia uses isAutomated = false anywhere at the moment.
BuyCommand uses the isAutomated=false version of makePurchases, but that's one step downstream of retrieveItem(). I'm not aware of any calls to retrieveItem itself with isAutomated=false. Still, good coding practices, and all that.
