Bug - Not A Bug Issue With Mood Not Buying Items From NPC Store

[bombar]

Issue: When running mood execute, the mood is not properly buying and using Knob Goblin eyedrops from NPC shops. If a previous item in the mood failed.

Steps to reproduce:
0) Make sure preference for "Buy items from NPC stores whenever needed" is checked
1) Create a mood with:
"When I run low on Merry Smithiness, use 1 Flaskfull of Hallow"
"When I run low on Peeled Eyeballs, use 3 Knob Goblin eyedrops"
2) Place all Flaskfull of Hallow and handful of smithness into your closet
3) Use the cli command "mood execute"
4) Notice that this will come up (edit: Note the bug that for some reason Knob Goblin eyedrops can't be bought and consumed):

> mood execute

Verifying ingredients for Flaskfull of Hollow (1)...
You need 2 more handful of Smithereens to continue.
Desired purchase quantity not reached (wanted 3, got 0)
You need 3 more Knob Goblin eyedrops to continue.
Mood swing complete.

5) Remove Merry Smithiness line from the mood
6) Run "mood execute"
7) Notice now it will work: (edit: Note that now without changing any preferences, just removing a buff that couldn't be applied, knob goblin eyedrops can be bought and used)

> mood execute

Purchasing Knob Goblin eyedrops (3 @ 200)...
You acquire Knob Goblin eyedrops (3)
You spent 600 Meat
Purchases complete.
Using 3 Knob Goblin eyedrops...
You acquire an effect: Peeled Eyeballs (duration: 30 Adventures)
Finished using 3 Knob Goblin eyedrops.
Mood swing complete.

Build Verified: r15072

 

[Bale]

I presume that your preference to buy from the mall whenever needed is disabled. If that is disabled, then this is expected behavior since Flaskfulls of Hollow are not purchasable from any NPC stores.

Are you expecting mafia's mood to ignore a failure and just continue without letting the user know that his mood failed? That does not sound like desireable behavior to me. I know I'd be very upset if my Light was rendered 25% less useful because mafia failed to use a Flaskfull and mafia did not abort.

Please let me know if I misunderstood anything or why you disagree that this should be expected behavior.

 

[Veracity]

Assuming that you have "buy from mall" disabled, I see no bug in your "steps to reproduce".
Bale nailed it.

 

[bombar]

Please let me know if I misunderstood anything or why you disagree that this should be expected behavio

Sorry you are right, I think there was a misunderstanding, I wasn't talking about the bug being about the flaskfulls since those can't be bought in an NPC, but rather the items in the mood that could be bought in the store, but aren't being bought. I'm concerned that something in the mood that could have been applied wasn't applied and the error message is wrong, because the only reason it couldnt' buy the item was that it didn't attempt to buy the item.

The mood says "Desired purchase quantity not reached (wanted 3, got 0)" in reference to knob goblin eyedrops, this is where I have the issue with a bug existing, why did it fail to purchase knob goblin eyedrops, all the settings were correct, as can be seen by the next example, it just failed to attempt to buy them

If in the same mood you had "Heavy Petting" in it, it would still buy and consume the Knob Goblin Pet Buffing Sprays and you would get the buffs associated with that, but not the Knob Goblin Eyedrops.

So some buffs get applied in the mood, and some don't, but out of the ones that don't some are producing the wrong error message, and aren't even being attempted to be applied because another failure in the mood occurred. It should be consistent, either all buffs that could be applied from the mood are applied when executing the mood, or none. Not apply some that could be applied, and don't apply some that could have been applied, because then you introduce inconsistencies.

(From what I can tell it all has to do with the alphabetically order of the buffs, once it fails to "aquire" items for one buff, any buff later then that it doesn't even bother to try to "aquire" the item, but I'm not 100% sure)

Are you expecting mafia's mood to ignore a failure and just continue without letting the user know that his mood failed?

Actually this currently happens in a few cases with mood, especially if the buff in question comes from a limited consumption resource like spleen. If you are out of spleen room, it will produce an error message that says can't reapply because spleen is full, but not abort the script. I actually had to code around that in one of my moods because I did want to abort if it couldn't.

 

[Bale]

If I recall correctly, when there is a failure in the mood, it will enter an error state and every successive element in that mood will fail so that the user can decide what to do. I think you agree that is a good behavior?

If I understand your point, the error message to the user is not helpful.

So this bug report is for KoLmafia to have a more helpful error message?

 

[bombar]

when there is a failure in the mood, it will enter an error state and every successive element in that mood will fail so that the user can decide what to do. I think you agree that is a good behavior?

I think this might be the point I'm struggling with, why is it good not to attempt to apply other buffs in the mood because one failed? Then at least you would know really which ones could be applied, or couldn't be applied.

I could understand if you didn't apply any buffs if there was going to be a failure, but the fact that some buffs were applied that could, and others that could be applied, but weren't is wierd to me. Why does ABC order determine whether or not a bug should be applied or not?

I mean really what is the difference between already applying "Heavy Petting" and not applying "Peeled Eyeballs"?

This all being said, an error message fix would definitely be useful and most likely I could even see the logic in that being the only fix since not a ton would be gained by being consistent for what could be a bit of work. I know the first reaction I had to this was something got broken with the auto npc store buying code, and I manually went and bought 100 eyedrops just to make sure I had them and the mood didn't have to rebuy them. It wasn't until I was trying to get to the root cause to log a bug report that I figured out what the actual problem was.

Note I think I should explain the usecase, and why this its an issue for me. Currently I have setup a mood that handles the buffs I want for the 150 turns I'm in the crimbo mining camp. It has for example use 1 bottle of bubbles, use 4 gene tonics, 1 frost flower, 15 eyedrops, 15 pet buffing sprays, etc. When I start my mining camp run, I switch into the mood, execute the mood, and then switch out of the mood because I don't want it reapplying a frost flower at turn 50 because its not longer cost effective. I use the error messages generated to know which items I need to restock on, so having error message that aren't really accurate isn't good, also not knowing whether or not something really could have been applied is not good. Now I understand that this isn't the usual usecase for a mood, but its the best way to make sure I don't forget buffs, and that they only get applied the number of times I want them to be.

Also, with the way it currently works, and the way I use it, I run into an issue when I get to the store to restock and the item is so expensive I don't want to restock it, now I have to edit the mood or create a new sub mood without that item just to make it so I could apply the other items, now on another day if I don't remember to readd that, and the price drops, now I'm forgetting to apply a buff that I would have wanted. This happend with Gene Tonic: Dude, at the start of crimbo they were like 4k, then they spiked to 9k, so I had to remove them from the mood so I could use the mood, but because they were removed I kept foregetting to recheck mall prices for them on when to readd them. If it would have just failed to apply that one buff, but still applied all the others, I could have made a decision each day which is what I think you guys were trying to do when you mention "so that the user can decide what to do" i could have restocked or not, but I wouldn't have had to change the mood just because I decided on this day I wasn't going to use that buff.

 

[fractalnavel]

... It should be consistent, either all buffs that could be applied from the mood are applied when executing the mood, or none. Not apply some that could be applied, and don't apply some that could have been applied, because then you introduce inconsistencies. ...

... I could understand if you didn't apply any buffs if there was going to be a failure, but the fact that some buffs were applied that could, and others that could be applied, but weren't is wierd to me.

Sounds like a two-phase commit thing. This would be cool, but doesn't seem practical, as it would have to track "what-if" for every action in the mood. Pretty much ends up being a simulation sort of thing.

As for stopping at first failure vs trying all anyway, maybe that's a simple option? The current behavior as default seems best.

For error messages, it seems the current approach indicates what the action would have been if there was no failure. Maybe the final message shouldn't say "Mood swing complete" in that case? Because it wasn't.

 

[bombar]

Sounds like a two-phase commit thing. This would be cool, but doesn't seem practical, as it would have to track "what-if" for every action in the mood.

Actually the I way I was thinking would that it first acquires any item it needs to acquire for all buffs, and then if all items it needed were acquired then it would use those items, that way you wouldn't waste items when the entire mood couldn't be full-filled. So instead of a two-phase commit thing, you have a buy phase and an apply phase, and the apply phase only gets run if the buy phase was successful. During the "Acquire" phase it would also cheek to make sure you have the skill if its a casted skill, and run any unconditional scripts to see if those failed.

As for stopping at first failure vs trying all anyway, maybe that's a simple option? The current behavior as default seems best.

this would be useful.

As far as the error message goes, it should say something like Previous buffs couldn't be applied, or acquired so didn't attempt with this one. To make it clear to the user that no attempt was made, not that it failed to acquire the item.

 

[lostcalpolydude]

Actually the I way I was thinking would that it first acquires any item it needs to acquire for all buffs, and then if all items it needed were acquired then it would use those items, that way you wouldn't waste items when the entire mood couldn't be full-filled.

It's really just a list of use/cast actions, and acquiring is a byproduct of that, so this would be much more complicated than it sounds.

I think what would work best for you is a script that is not a mood that you run at the start of your adventuring. I don't see anything being changed, including the message, which seems like it should be clear enough unless people don't know it's coming from their mood to begin with. Mafia did attempt to acquire the item, with all methods you allowed it to use.

 

[bombar]

Mafia did attempt to acquire the item, with all methods you allowed it to use.

What do you mean it did try to acquire the Knob Goblin Eyedrops?

That was my entire issue in the first place, it didn't go to the npc store and try to acquire them as far as I can tell.