Bug - Not A Bug Skills page double summons

drakono82

New member
In the relay browser, I open the "skills" page from the top frame. I select a summoning skill like sugar, love songs, or stickers, and when I click "use skill", it uses it twice. The MP updates correctly, showing 2 casts' worth of MP disappearing, but the results showing what I summoned only show one summoning. (Whether it's the first or second summoning results, I don't know.) I have also observed the CLI later claiming to have updated the number of summoned items, but I think that was only for love songs getting updated in the next combat.

I have definitely seen the issue on builds 8497 and 8495, and I believe 8490. (I updated Monday morning, June 14th, so that would have been the newest at that time.)
 

slyz

Developer
The same thing is happening for me, with libram summons from the skills page, with r8497.
Code:
> get libramSummons

14

> ashq print(mp_cost($skill[summon BRICKOs]));

104

> inv bricko brick

BRICKO brick (84)
I then went to my skills page through the relay browser (with GM turned off), to cast 1 summon:
Code:
You acquire BRICKO brick (3)

> get libramSummons

15

> ashq print(mp_cost($skill[summon BRICKOs]));

119

> inv bricko brick

BRICKO brick (87)
The MP cost shown on the skills page is 135 MP: the skill was cast twice, and Mafia missed the second one.
Code:
> refresh inv

Refreshing closet...
Updating consumable items...
Updating miscellaneous items...
Refreshing stickers...
Requests complete.

> inv bricko brick

BRICKO brick (90)

Attached is a debug log resulting from visiting the skills page and casting one summon bricko from there.
 

Attachments

  • DEBUG_20100617.txt
    664.1 KB · Views: 40

jasonharper

Developer
drakono, do you have a relay override script for campground.php? slyz does, and that's the ultimate source of the problem in the provided debug log.

Details: bookshelf skill use from skills.php redirects to campground.php. Relay override fires, and calls visit_url() - which returns an empty string in this case, because it actually redirects back to skills.php to deliver the results. Override writes only an empty string, so the override is assumed to have failed - the page is then retrieved and parsed as normal, resulting in one valid set of summoning results. However, that visit_url() did in fact also result in summoning; we just didn't parse it, since nothing ever followed the redirect to pick up its results.

It's not at all clear to me how this should be fixed. Having visit_url() in a relay override follow redirects would avoid the double-casting, but would likely have undesirable side-effects. In particular, this page, which is essentially skills.php with a campground results section on top, would have been processed only by a campground.php override, not a skills.php override if one existed - the results would be inconsistent with other skills.php page loads. Perhaps a redirect should kill the override script, and launch a new one on the redirected URL.
 

slyz

Developer
Good to know, I'll simply remove Bale's telescope helper (that was the campground.php relay override).
Just another question, since I can't test this right now - does this also affect summons when they are made from the gCLI or from the Skill Casting tab?
 

drakono82

New member
Yeah, I also had installed Bale's telescope script. I got it recently, which explains why I only saw it recently. I also recently was told mafia implements a very similar feature via the "telescope" command, so I'll remove the relay override.
 

Veracity

Developer
Staff member
Revision 7970 on Jab 12, 2010 says: "When casting a libram skill from skills.php, do not also log its casting from the subsequent redirect to campground.php". Note that that deals with fixing the logging, not with actually casting the skill twice. I can assure you: when I was testing this back in January, the skill was NOT actually cast twice.

The OP does not include a debug log, but slyz's debug log shows a relay override script for campground.php...

- You visit the skill page, select a bookshelf skill, and cast it.
- KoLmafia submits a call to skills.php?pwd&action=Skillz&whichskill=8103&quantity=1
- KoL sends a redirect to campground.php?quantity=1&preaction=summonbrickos&pwd&skilluse=1
- KoLmafia passes the redirect down to the browser, which then submits it.
- KoLmafia intercepts call to campground.php, notices that you have campground.ash, and KoLMafiaASH.getClientHTML invokes your script.
-----Summoning #1:
- Your script calls visit_url(), which submits the URL to do the summoning.
- The request completes and KoL redirects to skills.php?message=1 with content length = 0
- visit_url returns with an empty string
- Your script returns an empty string to KoLMafiaASH.getClientHTML.
- Since your script did not return a page to send to the browser ...
-----Summoning #2
- KoLmafia submits the campground request to KoL.
- KoL responds with a redirect, which is passed down to the browser, and submited
- You see the result of the second summoning request.

You get where I'm going with this? The relay override script cast the summoning and then, when it failed to return text to KoLmafia, KoLmafia did it again.

- Perhaps the relay override script should not kick in when it is being invoked to cast a skill. You have access to individual form fields via form_field( string) or to all of them via form_fields().
- Perhaps we need to provide a way for a relay override script to recognize when visit_url resulted in a redirect and allow it to follow the redirect.
- Perhaps visit_url should automatically follow redirects and only return the final result text to the script.
- Perhaps if a relay override script returns an empty string, KoLmafia should notice that there is an unfollowed redirect and carry on from there, rather than submitting the original URL again.

Those last three "perhaps" all require changes to relay override script support. They may be desirable, but I suggest that the first is relevant regardless: if campground.php is trying to show the state of the telescope, for example, it should only take control on a page that actually shows the telescope, rather than submitting the URL for ALL campground requests - including bookshelf summons.

Edit: well, I see a lot of activity happened here while I was composing my reply. :)

I also had installed Bale's telescope script.
I think Bale should fix his script, as described in "perhaps" #1...

Does this also affect summons when they are made from the gCLI or from the Skill Casting tab?
No. Relay override scripts are only invoked when you submit something via the relay browser.
 
Last edited:

drakono82

New member
I apologize that I didn't submit more details, but I'm thankful to slyz for his logs. As you can see, I'm not really an active participant in these forums, and I really don't know all the much about KoLmafia. I just use it for some of the more shallow (for lack of a better word) purposes. I wouldn't know how to capture that log info slyz posted.
 

slyz

Developer
Thanks Jason and Veracity.

Adding this to campground.ash solved the problem:
PHP:
void main() {
	if ( form_field("skilluse") != 1 ) override();
}

EDIT: to capture a log, Help -> Start Debug Log, then do whatever you want to capture a log off, and Help -> Stop Debug Log once you're done.
 

heeheehee

Developer
Staff member
Wouldn't it make more sense for that to be
PHP:
void main() {
    if(form_field("action")=="telescopelow") override();
}
?

Edit: Also note that you'd probably have to rename the void main() to void override().
 

Camber

Member
Would this change be necessary for ALL relay ash scripts that tweak the top menu? Without a telescope yet, i am not using Bale's script, but i am using topmenu.ash.
 
Top