Bug r16311 - RecoveryManager.isRecoveryActive() returns true when disabled

[Fluxxdog]

When hpAutoRecovery and mpAutoRecovery are set to negative values, the auto-recovery mechanism is effectively disabled. However, the mentioned method is returning true, causing aborts such as:

Aborting implicit outfit change due to potential infinite loop in auto-recovery. Please buy the necessary lemon manually.

The only thing I can find that is setting RecoveryManager.recoveryActive=true is the class's own runBetweenBattleChecks(). Is an abort elsewhere causing it to be stuck at true? Having a script abort because of auto-recovery when auto-recovery is disabled is surely not intended.

 

[Veracity]

What is the context in which you are seeing this? Some script, called when, doing what?

RecoverCommand.java:

	public void run( final String cmd, final String parameters )
	{
		boolean wasRecoveryActive = RecoveryManager.isRecoveryActive();
		RecoveryManager.setRecoveryActive( true );

		SpecialOutfit.createImplicitCheckpoint();

		int target;

		if ( parameters.equalsIgnoreCase( "hp" ) || parameters.equalsIgnoreCase( "health" ) || parameters.equalsIgnoreCase( "both" ) )
		{
			target = (int) ( Preferences.getFloat( "hpAutoRecoveryTarget" ) * KoLCharacter.getMaximumHP() );
			RecoveryManager.recoverHP( Math.max( target, KoLCharacter.getCurrentHP() + 1 ) );
		}

		if ( parameters.equalsIgnoreCase( "mp" ) || parameters.equalsIgnoreCase( "mana" ) || parameters.equalsIgnoreCase( "both" ) )
		{
			target = (int) ( Preferences.getFloat( "mpAutoRecoveryTarget" ) * KoLCharacter.getMaximumMP() );
			RecoveryManager.recoverMP( Math.max( target, KoLCharacter.getCurrentMP() + 1 ) );
		}

		SpecialOutfit.restoreImplicitCheckpoint();

		RecoveryManager.setRecoveryActive( wasRecoveryActive );
	}

As you can see, isRecoveryActive() will be true executing that command. So, I would guess that you have some script which is calling "recover mp" or something.

"Having a script abort because of auto-recovery when auto-recovery is disabled is surely not intended." I suppose not - but you have not given evidence, yet, that that is happening. Tell us more about your scripts. In particular, which one is trying to buy a lemon, and how is it invoked?

 

[Fluxxdog]

I have a script that makea Superhuman cocktails. I have a `create(1,drink)` command that makes any possible SHC at the time. When I started noticing this, I was actually in the the Barrr, getting rum, which would trigger the command since I also had access to the fruit stand via outfit. It would abort when trying to buy lemons or grapefruits. That led me to NPCPurchaseRequest which in turn led me to RecoveryManager. I thought my preferences had changed, but they were both still set to -.05. I ran things again until it aborted again, and nothing was changing my settings.

 

[Darzil]

What is your script triggering from? It sounds like maybe the issue is around that script triggering whilst recovering is running (even if in this case it running is doing no hp or mp restores).

Edit - Actually, the NPCPurchaseRequest putting on the outfit in a Between Battle Script will cause this.

Recovery Active is set before calling the Between Battle Script in Recovery Manager, as it is considered recovery :

		RecoveryManager.recoveryActive = true;

		if ( isScriptCheck )
		{
			KoLmafia.executeScript( Preferences.getString( "betweenBattleScript" ) );
		}

If you need an outfit, NPCPurchaseRequest refuses to do it if recovery is active :

		if ( RecoveryManager.isRecoveryActive() )
		{
			if ( neededOutfit != OutfitPool.NONE )
			{
				KoLmafia.updateDisplay(
					MafiaState.ERROR,
					"Aborting implicit outfit change due to potential infinite loop in auto-recovery. Please buy the necessary " + getItemName() + " manually." );

				return false;
			}

			return true;
		}

 

[Veracity]

When I started noticing this, I was actually in the the Barrr, getting rum, which would trigger the command since I also had access to the fruit stand via outfit.

"Which would trigger the command". How? It is marked as a betweenBattleScript?

I thought my preferences had changed, but they were both still set to -.05. I ran things again until it aborted again, and nothing was changing my settings.

As I pointed out, your preferences have nothing to do with isRecoveryActive().

Darzil has it. A betweenBattleScript which makes an NPC purchase which requires an outfit change will trigger this since a betweenBattleScript is deemed to be "recovery".

Perhaps RecoveryManager.runBetweenBattleChecks() need not consider running a script to be "recovery". I.e., set the boolean after the script but before mood/hp/mp recovery.

Perhaps NPCPurchaseRequest.ensureProperAttire() needs to be looked at; it says: "If the recovery manager is running, do not change equipment as this has the potential for an infinite loop.". I'd like to understand what this "infinite loop" might be; perhaps there is a smaller hammer that could be used to avoid such.

 

[Darzil]

Not sure if it does, as I haven't looked at it, but the sort of thing I can imagine happening is that a change of outfit to buy something results in HP or MP falling below a trigger point and triggering a recovery.

 

[Veracity]

Yes, I recall something like that being an issue when you needed the KGE outfit in order to buy Knob Goblin seltzer from the Dispensary. That is not required any more. Are there any other examples, or is this a vestige from the past?

The "recover" command sets recoveryActive while it is doing its thing.

So does runBetweenBattleChecks - which includes HP and MP recovery. However, it also sets that while running the betweenBattleScript and executing a mood. I don't think I see the latter two as needing this kind of protection.

 

[lostcalpolydude]

Equipping travoltan trousers to buy stuff could reduce HP or MP and cause issues, maybe?

 

[Fluxxdog]

"Which would trigger the command". How? It is marked as a betweenBattleScript?

What is your script triggering from?

Yes, part of a BBS.

Actually, the NPCPurchaseRequest putting on the outfit in a Between Battle Script will cause this.

Darzil has it. A betweenBattleScript which makes an NPC purchase which requires an outfit change will trigger this since a betweenBattleScript is deemed to be "recovery".

Equipping travoltan trousers to buy stuff could reduce HP or MP and cause issues, maybe?

I remember reading that one... here.