Bug - Fixed Mana burning tries to cast Thick-Skinned infinitely

[nicnak]

I have Thick Skinned, both the skill permed and the hatter buff active. When trying to open the relay browser, the char pane never loads and I see Mafia using 100% of a core. When I refresh the page, Mafia jumps up to 200% usage. Using jstack and eclipse, I narrowed it down to the ManaBurnManager looping forever incremented the amount of times it will cast Think Skinned.
Looping code is:

		while ( i.hasNext() )
		{
			ManaBurn b = (ManaBurn) i.next();
			
			if ( !b.isCastable( allowedMP ) )
			{
				i.remove();
				continue;
			}
			
			allowedMP -= b.simulateCast();
			Collections.sort( burns );
			i = burns.iterator();
		}

In a paused failing state here are some local variables.
burns: [cast 1160947058 Thick-Skinned, cast 3 Jingle Bells, cast 1 Fat Leon's Phat Loot Lyric]
b: cast 1160947058 Thick-Skinned

isCastable( allowedMP ) returns true because of SkillDatabase line 475 where its typed as a PASSIVE skill. Skill ID is 80 here.

Changing the setting to "Do not rebalance buffs" is a temp fix and lets me keep playing. Running at version r11484.

 

[heeheehee]

Attached patch changes ManaBurnManager to check if skills are usable in addition to being known. Reproduced below so that you can read it without having to download the attached file.

Index: src/net/sourceforge/kolmafia/moods/ManaBurnManager.java
===================================================================
--- src/net/sourceforge/kolmafia/moods/ManaBurnManager.java     (revision 11484)
+++ src/net/sourceforge/kolmafia/moods/ManaBurnManager.java     (working copy)
@@ -157,7 +157,7 @@
 
                        // Only cast if the player knows the skill
 
-                       if ( !SkillDatabase.contains( skillName ) || !KoLCharacter.hasSkill( skillName ) )
+                       if ( SkillDatabase.getUsableKnownSkillName( skillName ) == null )
                        {
                                continue;
                        }

 

[nicnak]

Many thanks. Patched it in and works great. Thanks for all your work on this, us people who are often silent really appreciate it.

 

[roippi]

I feel like there's a few simultaneous bugs that should be addressed here. hee^3's patch definitely addresses the major one. I'd also add some sanity checking in there for future-proofing.

I'm away from my dev box for a few more days, if someone else wants to take care of this.

 

[heeheehee]

Right, a safer solution would have been to check that the skill in question was the source of the effect, but I don't know if that's possible without too much extra work. This seemed to be the next viable option, as it at least stops the infinite looping.

 

[holatuwol]

Checking for mp cost of zero in 11488, which is functionally equivalent but with slightly less overhead since it would have happened anyway later on.