Bug - Fixed Buffbot buff casting weirdness

starkid

New member
I made a recovery script for my clan's buff bot, and now the bot sometimes casts buffs on its self instead of on the intended target.

The scenario goes something like this:

Person A requests a buff
x out of y casts are made on Person A
bot runs out of mp and runs the recovery script
y-x casts are made on THE BOT

The script tries to heal MP with a jumbo Dr. Lucifer, and in the process makes some random skill casts to burn off the residual MP, and then heals HP with a cast of cannelloni cocoon. I think it is these intermediate casts that are screwing up the buff target.
 

Veracity

Developer
Staff member
Why would a buffbot want to "burn off residual MP"?
Why would a buffbot want or need to heal HP?
 

Terion

Member
Well, healing the HP would be so the next Lucifer restores the max MP. But it does seem that the mana-burning can be turned off...
 

starkid

New member
Okay, I changed the script so that its not using any skills. Its still unusual behavior though. I mean, why is the target name being changed but not the skill name or id?

also, stupid question: how do I turn off mana burning?

I probably should have mentioned that I'm using revision 8633.
 

slyz

Developer
Okay, I changed the script so that its not using any skills. Its still unusual behavior though. I mean, why is the target name being changed but not the skill name or id?
I have no idea of what you are talking about. Maybe you should post the script you are talking about?
Do you mean that when your recovery script casts Cocoon, the buffbot becomes the new target for casting the rest of the buffs that were requested?

also, stupid question: how do I turn off mana burning?
Adventure tab -> HP/MP Usage tab -> Mana Burning. the second drop-down to "Do not rebalance buffs".
 

starkid

New member
Do you mean that when your recovery script casts Cocoon, the buffbot becomes the new target for casting the rest of the buffs that were requested?

Yes, sort of. The bot becomes the target for the buff it is trying to cast when it runs out of mp. Any buff requests processed after that are correct. I'm sorry if I wasn't clear before.

Adventure tab -> HP/MP Usage tab -> Mana Burning. the second drop-down to "Do not rebalance buffs".

It was already set to that. And the breakfast preferences for the summonables are all un-checked.

You can recreate the bug using this:

Code:
boolean main(string type, int amount)
{
	print("Using recovery script");

	use( 3, $item[phonics down]);
	use_skill( 1, $skill[Patience of the Tortoise]);

	return true;
}

I think you can change the skill to anything you have available to you and it will still work. Just set this to your recovery script (cli command: set recoveryScript=script_name.ash ) and then try to buff someone else when you have little or no mp.

this is the output I got using my alt Lem:

Casting Empathy of the Newt on starkid 24 times...
Empathy of the Newt was successfully cast on starkid.
Using recovery script
Using 3 phonics down...
You acquire an effect: Tingly Biceps (duration: 6 Adventures)
You acquire an effect: Tingly Elbows (duration: 5 Adventures)
You gain 142 hit points
You gain 145 Muscularity Points
Finished using 3 phonics down.
Casting Patience of the Tortoise 1 times...
You acquire an effect: Patience of the Tortoise (duration: 5 Adventures)
Patience of the Tortoise was successfully cast.
Casting Empathy of the Newt on yourself 10 times...
You acquire an effect: Empathy (duration: 200 Adventures)
Empathy of the Newt was successfully cast on yourself.
 

slyz

Developer
It was already set to that.
I think Mafia is simply burning what little MP you have left before using the Dr Lucifer, since the MP would be lost otherwise. It does this even if the setting is set to "Do not rebalance buffs". I overlooked that possibility.


Casting Empathy of the Newt on starkid 24 times...
Empathy of the Newt was successfully cast on starkid.
Using recovery script
Using 3 phonics down...
You acquire an effect: Tingly Biceps (duration: 6 Adventures)
You acquire an effect: Tingly Elbows (duration: 5 Adventures)
You gain 142 hit points
You gain 145 Muscularity Points
Finished using 3 phonics down.
Casting Patience of the Tortoise 1 times...
You acquire an effect: Patience of the Tortoise (duration: 5 Adventures)
Patience of the Tortoise was successfully cast.
Casting Empathy of the Newt on yourself 10 times...
You acquire an effect: Empathy (duration: 200 Adventures)
Empathy of the Newt was successfully cast on yourself.
That does look like a bug, it's strange that no one has pointed this out before.
 

Grotfang

Developer
I'm in Bad Moon so really can't test this, but if you are confident of fiddling around with your source code, may I ask you to make a small edit?

From looking at the source code, I think the issue might be appearing somewhere from UseSkillRequest.java, possibly an issue with changing the target of the request. The diff attached will monitor the loop that mafia runs through when sent a buff request that involves restoring half way through. The reason I stick a loop counter in is so we can see whether the target is being changed in the same instance of the class. I haven't yet worked out why it would be changed if it is, but I would appreciate ruling it in or out before spending time on it.

If you could apply the patch, compile, and run this again, that would be fantastic. Alternatively, if you can stomach the larger download and don't wish to apply the patch yourself, I compiled the modified code and saved it to my dropbox. Obviously, there is no way for me to prove that my modified version is safe to use, but you have my assurance that the ONLY change in that jar file is the patch I have posted here.

http://dl.dropbox.com/u/2477024/KoLmafia-8638M.jar
 

Attachments

  • UseSkillRequest.diff
    828 bytes · Views: 47

starkid

New member
From looking at the source code, I think the issue might be appearing somewhere from UseSkillRequest.java, possibly an issue with changing the target of the request. The diff attached will monitor the loop that mafia runs through when sent a buff request that involves restoring half way through. The reason I stick a loop counter in is so we can see whether the target is being changed in the same instance of the class. I haven't yet worked out why it would be changed if it is, but I would appreciate ruling it in or out before spending time on it.

As requested (again, I'm using an alt):

Code:
[B]Loop: 0. Target: starkid[/B]
Casting Empathy of the Newt on starkid 7 times...
Empathy of the Newt was successfully cast on starkid.
[B]Loop: 1. Target: starkid[/B]
Using recovery script
Using 3 phonics down...
You acquire an effect: Tingly Wrists (duration: 9 Adventures)
You gain 142 hit points
You gain 141 Muscularity Points
Finished using 3 phonics down.
Loop: 0. Target: null
Casting Patience of the Tortoise 1 times...
You acquire an effect: Patience of the Tortoise (duration: 5 Adventures)
Patience of the Tortoise was successfully cast.
Casting Empathy of the Newt on yourself 10 times...
You acquire an effect: Empathy (duration: 200 Adventures)
Empathy of the Newt was successfully cast on yourself.
[B]Loop: 2. Target: yourself[/B]
Using recovery script
Using 3 phonics down...
You acquire an effect: Tingly Biceps (duration: 8 Adventures)
You acquire an effect: Tingly Elbows (duration: 5 Adventures)
You gain 144 hit points
You gain 148 Muscularity Points
Finished using 3 phonics down.
Loop: 0. Target: null
Casting Patience of the Tortoise 1 times...
You acquire an effect: Patience of the Tortoise (duration: 5 Adventures)
Patience of the Tortoise was successfully cast.
Casting Empathy of the Newt on yourself 3 times...
You acquire an effect: Empathy (duration: 60 Adventures)
Empathy of the Newt was successfully cast on yourself.

That looks like a positive test to me.
 

Theraze

Active member
Just curious... where's the second Loop: 1? It goes straight from Loop:0 to Loop:2, so either something's screwed up with the logging, or parts of the log are missing...
 

Grotfang

Developer
It's not missing. Bits of this are what I expected, bits are not. It'll take me time to work through the logic, but hopefully that should be enough.

Theraze: Loops 0 and 1 where the target is starkid is followed (in that loop) by loop 2 target yourself. I think that is where the problem is originating from -- the target should still be starkid. It looks odd because I print Loop: x BEFORE it casts, so we see Empathy cast on himself after it has said the target is starkid. This means target is being changed during that loop cycle, probably on invoking Patience of the Tortoise as a cast (our second source of loop statements, which cover the "Loop: 0. Target: null" bits).

In short, Loop: 0. Target: null is when he uses patience of the tortoise.

Loop: x. Target: starkid (and yourself) is when he should be consistently casting empathy on Starkid.

Thanks Starkid. I appreciate you taking the time to help bugfix this.
 

Grotfang

Developer
Sorry about the double post.

This code is confusing me. The only way I can see the output being the way it is (from my patch) is if target is changing in the instance of UseSkillRequest somewhere between my updateDisplay line and it actually casting the skill. Since it doesn't happen the first time round, the only change second time round is that currentCast [line 673] is 0 so we enter the conditional

Code:
if ( currentCast == 0 || needExtra )

[Line 683]. In this statement we have two calls to SpecialOutfit.java (I can't see anything that would interact here), a call to KoLCharacter and one to MoodManager. Again, nothing odd there.

Which leads me to the only part of the code I don't fully understand, which is that from RecoveryManager.recoverMP() [line 695], which has been leading me round in circles. Really, I have only one question -- is there any way that a recovery script (really, I suppose, a call to the CLI function "cast") could change the target string in that particular instance of UseSkillRequest?

What's really throwing me here is that the CLI cast call uses null to refer to the character whereas future casts within the original loop have target set to "yourself". This would normally suggest to me that it is NOT an interaction with the cast command from the recovery script, since they use different targets.

I'll keep on thinking this over, but if someone with a better understanding of that code could weigh in, it would be hugely appreciated.
 

jasonharper

Developer
There is a unique, cached instance of each UseSkillRequest; apparently, something invoked by recovery or mana burning is retrieving the same request that the buffbot is using, and resetting its target in the process. There's a getUnmodifiedInstance() method that doesn't reset anything, which should be used in cases where you're just looking up some detail about the skill rather than actually casting it, but that wouldn't fully solve the problem - consider the case where mana burning actually casts the same skill the buffbot is trying to cast, the change of target would be unavoidable in that case.

There are two possible solutions I can see:
* The main skill casting loop could reset the target on each iteration.
* Instances of UseSkillRequest could be created on demand instead of being cached (but that would have performance implications).
 

starkid

New member
The SkillDatabase class is using the UseSkillRequest class to look up skill names... Looks like the source of the problem. Check out getSkillsByType on line 799, and getSkillName on line 1003.

-------------
EDIT

Partial fix. I just added in a slightly more sensible solution for matching partial skill names. It doesn't fix the case where the recovery script casts the same buff as the one that caused the recovery script to be launched.

View attachment buffFix.diff
 
Last edited:

Grotfang

Developer
There are two possible solutions I can see:
* The main skill casting loop could reset the target on each iteration.
* Instances of UseSkillRequest could be created on demand instead of being cached (but that would have performance implications).

I don't like the idea of on demand instances -- I think resetting the target is simpler. Would this fix the problem? Again, starkid, if you would be happy testing that would be fantastic.

http://dl.dropbox.com/u/2477024/KoLmafia-8644M.jar
 

Attachments

  • UseSkillRequest.diff
    908 bytes · Views: 29

starkid

New member
Close, but not quite there yet:

Code:
Casting Empathy of the Newt on starkid 1 times...
Empathy of the Newt was successfully cast on starkid.
Using recovery script
Using 3 phonics down...
You acquire an effect: Tingly Biceps (duration: 4 Adventures)
You acquire an effect: Tingly Elbows (duration: 6 Adventures)
You gain 143 hit points
You gain 146 Muscularity Points
Finished using 3 phonics down.
Casting Patience of the Tortoise 1 times...
You acquire an effect: Patience of the Tortoise (duration: 5 Adventures)
Patience of the Tortoise was successfully cast.
Casting Empathy of the Newt on starkid 10 times...
[B]You acquire an effect: Empathy (duration: 200 Adventures)[/B]
Empathy of the Newt was successfully cast on starkid.

you might want to try this.setTarget( originalTarget);
 
Last edited:

starkid

New member
That works.

Quick question: I noticed that mafia is multi threaded, so I was wondering, is it possible that two different threads could try to cast a buff at the same time and recreate this problem all over again? I ask because I just put some chat features into the buff bot, and so its possible (and quite likely) that someone will ask for a buff by pm while the bot is processing k-mail requests...
 

Grotfang

Developer
Original bug should therefore be fixed as of r8646.

As for your additional queries (post 14, previous post), I'm afraid I'm not sure I can add much on either count. I have no way of testing interactions between kmail and chat easily. My gut instinct says that even if they do end up running simultaneously, targets should be handled well now. Try it and see.

Regarding your SkillDatabase change... if I read it rightly, the recent commit makes this unnecessary. Is this correct, or are there other interactions that make it worth implementing?

In short, can we count this bug report as closed, or is there something still missing?
 

starkid

New member
I tried it and I was right. Kind of fun to watch actually.

Code:
Casting Empathy of the Newt on starkid 15 times...
Casting Empathy of the Newt on Startree 19 times...
Empathy of the Newt was successfully cast on Startree.
Empathy of the Newt was successfully cast on Startree.

So, I think the only way to fully solve this problem will be to get rid of the cashed instances of UseSkillRequest. But the bot already uses so much memory, I don't think that would be such a great idea.

And about my change: SkillDatabase is using UseSkillRequest as a fancy string container, and it doesn't need to do that at all since it already has the information its looking for in a map. It also opens the door for more errors of this type in the future.
 
Top