Bug - Fixed Casting Smile of Mr. A on players with spaces in their names doesn't work

[heeheehee]

The CLI spits this at me:

> cast smile of mr. a on crimbo grotto

Selected target is not a valid target.

A debug log spat this at me:

Requesting: http://www.kingdomofloathing.com/skills.php?action=Skillz.&whichskill=3&specificplayer=crimbo%2Bgrotto&bufftimes=1

The problem seems to be that it's turning the space into an underscore, but just for this skill? I'm not entirely sure what skills are affected, as not everything seems to be affected.

> cast empathy on crimbo grotto

Casting Empathy of the Newt on crimbo grotto 1 times...
Empathy of the Newt was successfully cast on crimbo grotto.

 

[heeheehee]

I also see

> get_player_id crimbo grotto

Returned: crimbo grotto

For whatever reason, casting empathy encoded crimbo grotto as crimbo+grotto, which explains why that request worked.

 

[Darzil]

I haven't got a golden Mr A to play with, can you try casting it on such a person using the normal interface with debug on so we can see the URL submitted?

 

[heeheehee]

I mean, the URL should be

Requesting: http://www.kingdomofloathing.com/skills.php?action=Skillz.&whichskill=3&specificplayer=crimbo+grotto&bufftimes=1

Note + instead of %2B for "Crimbo Grotto" instead of "Crimbo_Grotto".

 

[heeheehee]

For what it's worth, if I do a /who on the player, then get_player_id returns the right ID, and casting Smile on said target also works. So I guess there's also a bug in ContactManager or whatever the file was called.

 

[Veracity]

Bump, since a friend just gave me a Golden Mr. A and I can now test.

 

[Bale]

That's a really good friend! Have fun!

 

[Veracity]

The issue was that we built the URL string correctly, including URL encoding the " " to a "+", but before we ran it, we called UseSkillRequest.reconstructFields which basically took the URL and reencoded it, resulting in the "+" turning into "%2B", as you saw.

Aside from the fact that that was an incorrect method of regenerating the URL - GenericRequest.cloneURL does it the right way - it was completely unnecessary.

Revision 14553 gets rid of that method. It also fixes all the subClasses of CreateItemRequest (which DO need to override that method) to similarly not pointlessly (and potentially incorrectly) replace the URL string with something which is intended to be identical to the one which was already there...

 

[Veracity]

By the way, in the course of verifying my fix to this, I sent two boxes of sunshine to the character "crimbo grotto".

Although I have a major issue with one member of that clan (who, therefore, is on my ignore list), I am very fond of other members of the clan, have a whitelist to the clan, and have no issues whatsoever with the clan itself. I hope there will be no ill-will from my having "blessed" the clan "bot" (or whatever it is) with a little sunshine.