Bug You are too drunk to continue

[ckb]

I have been getting an error of "You are too drunk to continue." in aftercore, even though I am not. This may be related to this bug about drunkness..
Mafia is reporting my inebriety at 17/14, when it should be 17/20 I think.

> modtrace liver

type	source	Liver Capacity
Skill	Liver of Steel	+5.00	= +5.00
Skill	Drinking to Drink	+1.00	= +6.00

I'll try to get more info on why this is happening, or what else might cause this to happen.

 

[gausie]

I can't work this out, I have seen the error reported by others. I don't understand how passive skills are showing in modtrace but not being counted by getInebrietyLimit.

 

[ckb]

Some other things I noticed:
This problem does not happen with older builds (r27596)
In my adventures today, I did not have the relay browser running. Loading the relay browser fixed the problem (I assume because it loaded and parsed the charpane)

 

[ckb]

I logged out and logged back in again after adventuring and overdrinking. Mafia now reports inebriety_limit() = 14, still with +6 Liver Capacity.
Opening and refreshing the Relay browser (and the charpane) correct this to inebriety_limit() = 20

 

[heeheehee]

Huh. This is fascinating:

> inebriety_limit

Returned: 9

> modtrace liver

type	source	Liver Capacity
Skill	Liver of Steel	+5.00	= +5.00
Skill	Hard Drinker	+5.00	= +10.00
Skill	Hollow Leg	+1.00	= +11.00
Skill	Drinking to Drink	+1.00	= +12.00

> inebriety_limit

Returned: 21

That said, I can reproduce pretty reliably.

 

[heeheehee]

modtrace calls recalculateAdjustments, but that happens at the end of the invocation...?

> inebriety_limit

Returned: 9

> numeric_modifier Liver Capacity

Returned: 0.0

> modtrace liver capacity

type	source	Liver Capacity
Skill	Liver of Steel	+5.00	= +5.00
Skill	Hard Drinker	+5.00	= +10.00
Skill	Hollow Leg	+1.00	= +11.00
Skill	Drinking to Drink	+1.00	= +12.00

> numeric_modifier Liver Capacity

Returned: 12.0

Well, at least we're consistent. And it looks like that call to modtrace recomputes the modifiers.

 

[heeheehee]

It's not just organ capacities, it turns out -- other skill modifiers are affected, too.

> numeric_modifier weapon damage

Returned: 30.0

> modtrace weapon damage

type	source	Weapon Damage	Weapon Damage Percent
Equipment Power	15% weapon power	+30.00	= +30.00
Skill	Claws of the Walrus	+7.00	= +37.00

> numeric_modifier weapon damage

Returned: 37.0

 

[heeheehee]

Yeah, this isn't a new bug per se, but rather one that became much, much more visible with the organ tracking change. I checked out an earlier revision (r27601 contained the organ tracking change), and saw the same results for Weapon Damage:

> get_revision

Returned: 27596

> numeric_modifier weapon damage

Returned: 30.0

 

[Ryo_Sangnoir]

I posted some investigation in https://kolmafia.us/threads/missing-1-drunk-capacity-starting-with-27601.29257/ -- there's a race condition to do with the passive skill cache. I don't know whether this is the root cause, but it might be.

Okay, so somebody else reported this (from after freeing the king), so I did some investigation and found the following, copied here for ease of reference:

yeah, I can see a race
`Modifiers.availablePassiveSkillModifiersByVariable` is cleared by `resetModifiers` which is called by `liberateKing`, but changing the `kingLiberated` preference sets `availableSkillsChanged` to `true`, which repopulates it in `applyPassiveModifiers`

I think this also applies whenever a skill provided by a piece of equipment is added or removed.

 

[Ryo_Sangnoir]

A side issue is that, after modtrace recalculates your adjustments, the sidepane still shows the previous value -- it needs a KoLCharacter.updateStatus() call to update.

 

[heeheehee]

Yeah. I think this was broken by r26962.

(and by "think" I mean I checked the output of `numeric_modifier weapon damage` on login with both r26961 and r26962, and confirmed that r26962 introduced the regression I've been using as my test case.)

 

[ckb]

As a hacky fix, I added this to my afterAdventureScript

if (my_inebriety()>inebriety_limit()) { visit_url("charpane.php"); }

That will reset Mafia's inebriety_limit

 

[heeheehee]

@Ryo_Sangnoir A simpler explanation: during LoginManager.initialize(), we call recalculateAdjustments() on login (in KoLCharacter.reset()) well before we populate our skills (via the CharSheetRequest in KoLmafia.refreshSession()).

diff --git a/src/net/sourceforge/kolmafia/KoLmafia.java b/src/net/sourceforge/kolmafia/KoLmafia.java
index 109340ccbd..562156635c 100644
--- a/src/net/sourceforge/kolmafia/KoLmafia.java
+++ b/src/net/sourceforge/kolmafia/KoLmafia.java
@@ -695,6 +695,8 @@ public abstract class KoLmafia {
     // It would be nice to not have to do this
     IslandManager.ensureUpdatedBigIsland();
 
+    KoLCharacter.recalculateAdjustments();
+
     KoLmafia.setIsRefreshing(false);
   }

seems to fix the issue for me.

 

[Ryo_Sangnoir]

I tried jamming in some recalculateAdjustments while looking at https://kolmafia.us/threads/mafia-setting-timesrested-to-0-on-login.29268/, but I never managed to make it work. If it works in the place you've got it, that's good to go, I think.

 

[heeheehee]

I was playing around with trying to get it to work in tests, but there are some quirks with how we manage passives -- both campground details as well as familiar modifiers (e.g. Underwater Familiar) were blocking applyPassiveModifiers from correctly detecting that skills had been reset.

 

[heeheehee]

I looked at it some more, and that's not actually quite right. applyPassiveModifiers was always populating availablePassiveSkillModifiersByVariable, even if there were no skills loaded.

Anyways. #2011.

 

[heeheehee]

Hm. r27646 doesn't seem to have fixed the problem fully. I'll tinker some more.

 

[heeheehee]

Oh, I understand the race you described now. We fire a listener for (skill) whenever we add a new skill, which in turn sets availableSkillsChanged.

I suppose we want a mechanism to wait until any pending listeners have finished executing.

 

[heeheehee]

#2012 sidesteps that by just updating availableSkillsChanged in a synchronous fashion, and adding an explicit call to recalculateAdjustments() in CharSheetRequest. I could probably revert the change in #2011, since I'm pretty sure this is a more comprehensive fix, but, that commit doesn't really hurt anything (other than result in a small performance hit on session refresh).

 

[Ryo_Sangnoir]

Oh, I understand the race you described now. We fire a listener for (skill) whenever we add a new skill, which in turn sets availableSkillsChanged.

The other part of this is that (when it's a combat skill) sometimes we fire "addAvailableSkill" and sometimes "addAvailableCombatSkill". The former recalculates modifiers and the latter doesn't. I am curious as to whether we could just always use the latter.