Bug - Fixed Divide by zero error eating quantum taco/ consuming Astral Energy Drink

[Aventuristo]

My problem seems related to the bug fixed Feb. 23, but it's happening in the Mafia I downloaded today (r27403). If I try to consume an astral energy drink from the Item Manager, or do << ash chew(1, $item[[10883]astral energy drink]); >> in the gCLI, I get an unexpected error, debug log printed. I've attached the debug log with the errors; the first is from the Item Manager, the second is from the gCLI.

 

[MCroft]

It looks like quantum taco is fullness 0 in fullness.txt

When we call EatItemRequest's maximumUses(final int itemId, final String itemName, final int fullness)
, it ends with return fullnessLeft / fullness;

I'm not sure if this should be special cased in EatItemRequest or if we should fudge the data so it has a fullness.

According to the wiki

• You must have 2 fullness left to eat the taco, but your stomach is filled by the amount of the randomly-chosen food, even if this would cause you to overeat.

> eat quantum taco
Unexpected error, debug log printed.
class java.lang.ArithmeticException: / by zero
java.lang.ArithmeticException: / by zero
    at net.sourceforge.kolmafia.request.EatItemRequest.maximumUses(EatItemRequest.java:206)
    at net.sourceforge.kolmafia.request.UseItemRequest.maximumUses(UseItemRequest.java:443)
    at net.sourceforge.kolmafia.request.UseItemRequest.maximumUses(UseItemRequest.java:352)
    at net.sourceforge.kolmafia.request.EatItemRequest.run(EatItemRequest.java:246)
    at net.sourceforge.kolmafia.RequestThread.postRequest(RequestThread.java:242)
    at net.sourceforge.kolmafia.RequestThread.postRequest(RequestThread.java:207)
    at net.sourceforge.kolmafia.textui.command.UseItemCommand.use(UseItemCommand.java:221)
    at net.sourceforge.kolmafia.textui.command.UseItemCommand.use(UseItemCommand.java:47)
    at net.sourceforge.kolmafia.textui.command.UseItemCommand.run(UseItemCommand.java:42)
    at net.sourceforge.kolmafia.KoLmafiaCLI.doExecuteCommand(KoLmafiaCLI.java:467)

 

[fronobulax]

If you get there before I do (coming for to carry me home) consider looking at https://kolmafia.us/threads/unable-to-chew-new-astral-energy-drink-part-2-electric-boogaloo.28939/ which seems to be another divide by zero caused by a zero fullness.

 

[MCroft]

That makes me think we should have a failsafe to prevent divide by zero in any case, regardless of special casing the code or giving it a fake fullness.

 

[MCroft]

I'm not sure what I'm seeing. In practical terms, we have a switch statement which is failing on case EAT and SPLEEN (and potentially DRINK) because it doesn't have the protection from zero statement that's below it.

So, it's easy to pull those if wrapper up into the case statement, but why do we have both bits of code?

UseItemRequest.java
// Check binge requests before checking fullness or inebriety
switch (consumptionType) {
case SPIRIT_HOBO:
case GLUTTONOUS_GHOST:
case SLIMELING:
return Integer.MAX_VALUE;
  case ROBORTENDER:
return 1;
  case PASTA_GUARDIAN:
UseItemRequest.limiter = "character class";
    return KoLCharacter.isPastamancer() ? 1 : 0;
  case EAT:
return EatItemRequest.maximumUses(
itemId, itemName, ConsumablesDatabase.getFullness(itemName));
  case DRINK:
return DrinkItemRequest.maximumUses(
itemId, itemName, ConsumablesDatabase.getInebriety(itemName), allowOverDrink);
  case SPLEEN:
return SpleenItemRequest.maximumUses(
itemId, itemName, ConsumablesDatabase.getSpleenHit(itemName));
}

// Delegate to specialized classes as appropriate

int inebriety = ConsumablesDatabase.getInebriety(itemName);
if (inebriety > 0) {
return DrinkItemRequest.maximumUses(itemId, itemName, inebriety, allowOverDrink);
}

int fullness = ConsumablesDatabase.getFullness(itemName);
if (fullness > 0 || itemId == ItemPool.MAGICAL_SAUSAGE) {
return EatItemRequest.maximumUses(itemId, itemName, fullness);
}

int spleenHit = ConsumablesDatabase.getSpleenHit(itemName);
if (spleenHit > 0) {
return SpleenItemRequest.maximumUses(itemId, itemName, spleenHit);
}

 

[fronobulax]

This is in the realm of "it cannot happen" for me at the moment. I think I am going to try and write a failing test. If I succeed I'll make a PR and we can go from there.

 

[fronobulax]

Fix Divide by zero consumption errors. by jaadams5 · Pull Request #1759 · kolmafia/kolmafia

Failing tests to find and fix divide by zero for some consumption. See https://kolmafia.us/threads/divide-by-zero-error-eating-quantum-taco-consuming-astral-energy-drink.28941/

github.com

 

[fronobulax]

If you just comment out the switch the code returns MAX_INT for the test cases.

I wonder of the thing to do is just remove EAT, DRINK and SPLEEN from the consumptionType switch, since if the fullness isn't zero they will return the same thing as the case did. Pondering.

 

[fronobulax]

Moving makes the new tests work but breaks a glitch season reward test. I think it is the difference between a UseItem vs EatItem request but I don't have it sorted out yet.

 

[MCroft]

I debugged it as well and the second code was hit on startup with a consumptionType of "NONE". This came up when building the panes for the Inventory panels, (X possible, Y current)

If it falls through everything, it will return Integer.MAX_VALUE;

However, GLITCH_ITEM doesn't fall through, it bails out early on consumptionType Eat but not consumptionType NONE:

case ItemPool.GLITCH_ITEM -> {
UseItemRequest.limiter = "daily limit";
return 1 - Preferences.getInteger("_glitchMonsterFights");
}

So do a bunch of other things. We may need to check at the end of EatItemRequest.maximumUses.

What's the right way to figure out the max uses of an item that has variable fullness? The taco, per the wiki, has a minimum fullness to use of 2, but applies the fullness of whatever it morphs into, which isn't known when you eat it.

 

[MCroft]

progress, for you to consider. It passes the new tests, but I don't like the output of eating a quantum taco:

> eat quantum taco

Eating 1 quantum taco...
Consumption limit reached.
Finished eating 1 quantum taco.

> eat pr0n taco

(usable quantity of pr0n taco is limited to 0 by fullness)

 

[fronobulax]

I'll look more closely later today.

I am thinking we may have to bite the bullet and have three special cases to avoid dividing by zero when fullness is zero. Logically if something has fullness of zero then an infinite number can be consumed. Maybe a very large number won't break things.

Since there is a suggestion that this worked differently before and after a fix circa Feb. 23 2023 I also have some motivation to try and figure out what changed and why.

 

[MCroft]

some data to experiment with:

fullness.txt:8: [glitch season reward name]    0    0        0    0    0    0    Inedible, starts a fight with %monster%
fullness.txt:170: Children's Meal of the Damned    0    1    crappy    0    0    0    0
fullness.txt:359: flask of peppermint oil    0    1    crappy    0    0    0    0
fullness.txt:413: gloomy black mushroom    0    1    crappy    0    0    0    0    Inedible
fullness.txt:583: magical sausage    0    1    EPIC    1    0    0    0    999 MP
fullness.txt:637: oily golden mushroom    0    1    crappy    0    0    0    0    Inedible
fullness.txt:729: quantum taco    0    1    ???    0    0    0    0    random food
fullness.txt:865: spooky frank    0    1    crappy    0    0    0    0
inebriety.txt:77: bottle of Chateau de Vinegar    0    0    crappy    0    0    0    0    Undrinkable
inebriety.txt:474: N. O. Beer    0    0    crappy    0    0    0    0    Undrinkable
inebriety.txt:490: Otorian Battle Scar    0    0    crappy    0    0    0    0    Undrinkable
inebriety.txt:580: Schrödinger's thermos    0    1    ???    0    0    0    0    random booze
spleenhit.txt:31: [10883]astral energy drink    5    1        0    0    0    0    Lucky!

Astral energy drink may be a different special case. It does provide a spleen hit cost, but provides "Lucky!" instead of adventures, apparently.

 

[fronobulax]

I have changes that pass when I run the tests in the IDE but fail when run as part of the entire test suite. My hypothesis is that because requests can be reused there is leakage. If there is leakage in tests then it is possible that reusing requests is not always a good thing but I think the way forward is to identify what leaky data is causing the issue and then explicitly set that as part of the cleanups in testing. I will do that.

The zero items in https://kolmafia.us/threads/divide-...suming-astral-energy-drink.28941/#post-172296 should be added to tests but I'm going to hold off momentarily. I can imagine a New and Improved test that is drive by the data file and not just a list of hard coded items.

 

[fronobulax]

ConsumablesDatabase.getRawSpleenHit fails to disambiguate "astral energy drink" and returns 0 rather than 8 (obsolete) or 5. Am thinking about a separate PR to deal with that and suspend this one until that is resolved.

Also, I looked at the instance code and I do not believe sharing instances across tests is possible.

 

[fronobulax]

astral energy drinks are special in that when looking at consumption related things the results are different depending upon which item number is included in the string or whether it is just "astral energy drink". Tracking that down exposed TCRS tests were leaking consumable data which caused trouble fixing the divide by zero. When https://github.com/kolmafia/kolmafia/pull/1766 is resolved that will make my life easier fixing teh divide by zero. Stay tuned.

 

[Ryo_Sangnoir]

ConsumablesDatabase.getRawSpleenHit fails to disambiguate "astral energy drink" and returns 0 rather than 8 (obsolete) or 5. Am thinking about a separate PR to deal with that and suspend this one until that is resolved.

Also, I looked at the instance code and I do not believe sharing instances across tests is possible.

Why is "astral energy drink" getting passed anyway? If we know what you want to consume, we know what the ID is, so we should be able to pass the correct item.

 

[fronobulax]

Might be a a self-created rabbit hole I went down because a function was not returning the data that was in the datafile.

 

[MCroft]

It's weird.

We have an itemId when we call maximumUses.

We call getSpleenHit(itemName), which looks up the map ConsumablesDatabase.consumableByName.get(name);

But it would probably be more precise to create ConsumablesDatabase.getSpleenHitById(itemId) and have that function call ConsumablesDatabase.consumableByItemId(ItemId)

case SPLEEN:
return SpleenItemRequest.maximumUses(
itemId, itemName, ConsumablesDatabase.getSpleenHit(itemName));
}

 

[Veracity]

Or overload getSpleenHit to have a version that takes a name and a version that takes an id.
The thing about Consumables in general is that they don't necessarily have an itemId.
Consider sushi and hot dogs.