Bug - Fixed steal() in a consult script results in an attack (when I'm not a moxie class?)

r15893.

steal() in my combat script is causing me to attack if the Pickpocket action isn't available for some reason (for instance, if I'm a non-moxie class or when I lose initiative). I expected that it would do nothing in these cases... I can handle this by explicitly checking if(contains_text(page, "value=\"steal")). But that seems unnecessarily complicated.

Is the fallback-to-attack intentional or necessary in some way?

Two updates:

I've confirmed that this isn't an issue as an AT, in r15893. So it does appear to be related to class, or it got silently fixed. (Of course, the class change involved an ascension, so maybe something else got changed along the way.)

Also, have_skill($skill[pickpocket]) won't work as a check, since Mafia apparently doesn't treat Pickpocket as a skill? (I just get 'Changing "Pickpocket" to "Gnefarious Pickpocketing" would get rid of this message'.) Checking class appears to be the only possibility. Why isn't Pickpocket recognized?

Because "Pickpocket" is not a skill, any more than "Run Away" is; both are combat actions.

(Are you aware that having a tiny black hole equipped lets you pickpocket, even if you are not a moxie class? Ditto for being in Bird form.)

Yes, I knew there were other sources of Pickpocket. But I didn't realize that there was any distinction between a skill and a combat action. I thought that some items simply granted the skill Pickpocket, in the same way that other skills are granted by items. (Pocket Crumbs, etc.) I guess Run Away is another example of a non-skill combat action.

So, scratch that not-a-bug. I still wonder why steal() would cause an attack.

PaladinWhite said:

I'm a sauceror. steal() in my combat script is causing me to attack. I expected that it would do nothing, since I'm not a moxie class... Of course, I can handle this by explicitly checking have_skill($skill[pickpocket]) or (my_class() == $class[accordion thief] || my_class() == $class[disco bandit]). But that seems unnecessarily complicated.

Click to expand...

I do this by checking if(contains_text(page, "value=\"steal")) because that produces the correct answer regardless of initiative, class and even black hole.

Thanks, Bale. I'll try that.

I just found that steal() causes an attack even when I'm an AT, if I lose initiative. So it appears to fall back to an attack at all times, regardless of class.

PaladinWhite said:

Is the fallback-to-attack intentional or necessary in some way?

Click to expand...

Considering the code, it is obvious that it is "intentional". That's how it's always worked. You are the first person to complain about it.

Note that all ASH combat actions return a buffer value, which contains the response text from submitting the URL to perform the specified action. Were this to become a no-op if you cannot currently steal, what return value would you expect?

My initial expectation was that steal() would either steal or not steal; which is to say, steal or do nothing. In the "do nothing" case, it would simply return an unchanged buffer of the current page.

I just can't wrap my head around why we'd want it to fall back to attacking, which is a completely different action, with no logical connection to pickpocketing. It seems like a totally arbitrary choice. Why not fall back to running away, instead? Or casting the first skill in the skill list? Those seem like equally (il)logical responses to not being able to pickpocket.

Other combat actions aren't treated that way.
Trying to use a skill that isn't available submits the request to KoL anyway, and gets the response "You don't have that skill."
Trying to use an item that isn't available submits the request to KoL anyway, and gets the response "You don't have that item."
Neither one falls back to attacking when the requested action isn't valid.

I'm not sure how KoL would respond if Mafia submitted an illegal pickpocket request, but if there's a parallel response in that case, it would seem to make sense to allow KoL to respond, as in other cases. Alternatively, Mafia could handle the issue itself and detect the inability to steal, and refrain from submitting the request at all.

PaladinWhite said:

In the "do nothing" case, it would simply return an unchanged buffer of the current page.

Click to expand...

I wonder where it would get that text.

Beats me. But I'm not intimately familiar with most of Mafia's code.

If I'm in a fight, and I close the relay browser, then re-open it, the fight is still displayed. So that content must be stored somewhere, right?

Or does re-opening the browser always attempt to visit main.php, regardless of game state, causing KoL to return a fresh copy of the fight page? In that case, let's have steal() visit main.php if nothing can be stolen, which will have the same result. It's hacky, but I still argue it would be preferable to falling back to a logically-unconnected action.

(This is all a moot point if KoL will return failure text for an invalid pickpocket attempt, in the same way that it does for an invalid item or skill use.)

PaladinWhite said:

If I'm in a fight, and I close the relay browser, then re-open it, the fight is still displayed. So that content must be stored somewhere, right?

Click to expand...

Not in KoLmafia.

Or does re-opening the browser always attempt to visit main.php, regardless of game state, causing KoL to return a fresh copy of the fight page?

Click to expand...

This.

(This is all a moot point if KoL will return failure text for an invalid pickpocket attempt, in the same way that it does for an invalid item or skill use.)

Click to expand...

I think we should just submit the "steal" action. If you can't steal, because you are the wrong class or have already done so this fight or have done something else already, it will be a wasted server hit - but that will be the fault of your consult script, not KoLmafia.

I am sure the resulting text will have some error or other. For all I know, it might even result in you getting smashed by the monster - but, as I said, that is your script's fault.

Revision 15903 does that.