Bug - Fixed fullness not calculated correctly when "too full" message encountered

[stannius]

Repro Steps
1) Assumption: character without stomach of steel
2) Outside of mafia, eat anything with 1 <= fullness <= 5
3) Launch mafia. currentFullness=0
4) Eat 2 rat appendix chow mein. currentFullness=10
5) Attempt to eat 1 rat appendix chow mein.

Result
currentFullness=15

Expected
currentFullness=11

 

[StDoodle]

Mafia only knows that your fullness is >10... I'm not sure I follow on why an assumption of 15 is any better or worse than an assumption of 11. Actually, I can kind of see why, from a programmatic standpoint, it might be easier to simply assume you're completely full.

 

[xKiv]

If you started with a 1-fullness food, so that your real fullness is 11, you should be able to eat 4 more - but if mafia thinks you are completely full (at 15), it won't let you eat anything.
At least that's how I remember how it was when it happened to me. Had to eat from relay.

 

[Winterbay]

I think it would be better to increase the current fullness with the least amount that would stop me from eating that food. In this case 1. Had it been a 3-fullness food it should've increased my fullness to 13, since as stated above you cannot eat via Mafia's UI if your fullness is too high.

 

[slyz]

The same thing is happening for me. It happens when eating through the realy browser or from the gCLI.

> set currentFullness = 10

currentFullness => 10

> eat 1 Knob sausage chow mein

Since you are not in a Mysticality sign, you may not visit the restaurant.
Eating 1 Knob sausage chow mein...
Consumption limit reached.

> get currentFullness

15

This is the code that handles the "too full" message in UseItemRequest.java (lines 1744-1765):

if ( responseText.indexOf( "too full" ) != -1 )
{
	UseItemRequest.lastUpdate = "Consumption limit reached.";
	KoLmafia.updateDisplay( KoLConstants.ERROR_STATE, UseItemRequest.lastUpdate );

	int fullness = ItemDatabase.getFullness( item.getName() );
	int couldEat = fullness == 0 ? 0 : 
		Math.max( 0, Math.min( item.getCount() - 1, (KoLCharacter.getFullnessLimit() - KoLCharacter.getFullness()) / fullness ) );
	Preferences.increment( "currentFullness", couldEat * fullness );
	
	int estimatedFullness = KoLCharacter.getFullnessLimit() - fullness + 1;

	if ( fullness > 0 && estimatedFullness > KoLCharacter.getFullness() )
	{
		Preferences.setInteger( "currentFullness", estimatedFullness );
	}

	ResultProcessor.processResult( item.getInstance( item.getCount() - couldEat ) );
	KoLCharacter.updateStatus();

	return;
}

In the example, currentFullness starts at 10, fullness = 5, couldEat should be 0, estimatedFullness = 11, and currentFullness should be set to 11.

I don't really see why it is set to 15. The message hasn't changed and this can be reproduced consistently, lag has nothing to do with it.

 

[stannius]

Mafia only knows that your fullness is >10... I'm not sure I follow on why an assumption of 15 is any better or worse than an assumption of 11. Actually, I can kind of see why, from a programmatic standpoint, it might be easier to simply assume you're completely full.

Mafia ignores a consumption request that would take you over the limit. So if currentFullness = 11, I can attempt to eat a 4 fullness food, then a 3, etc. until I find one that fits in my remaining stomach space. If currentFullness = 15 I have to work around mafia to eat or set currentFullness myself.

I am 99% sure mafia used to set fullness to 11 in this situation. And thus it was broken at some point (perhaps revision 7409, but that's a wild guess.)

I also walked through the section of code as slyz did and it looked correct to me. I don't have a java debugger so I can't step through the code line-by-line.

 

[Theraze]

I believe this line:

Preferences.increment( "currentFullness", couldEat * fullness );

sets the fullness to what it would be if the eating had succeeded. This:

    if ( fullness > 0 && estimatedFullness > KoLCharacter.getFullness() )
    {
        Preferences.setInteger( "currentFullness", estimatedFullness );
    }

means that if the estimated would be MORE than if the eating had succeeded, it will overwrite it... so if you eat the 15 outside mafia, then try to eat... let's take your chow mein. Mafia would increment to 5 naturally, since that's how much the main incrementing would be. Then it estimates and comes up with 15-5+1=11, as you said. Because 11>5, it would set your fullness to 11, guessing that you failed to eat because chow mein takes 5. The next time you try, it would fail to eat, because it only has 4 appetite available. If you tried to eat an item that took 4 though, or tried to eat caviar to begin with (for example), it would set you to 15/15.

Bottom line, code is working exactly as it's supposed to.

 

[Winterbay]

But it isn't setting the fullness to 11, it seems to set it to 15 directly which is not what it should do. At least not if I understand your arguing correctly...

 

[Theraze]

Nope... 10+5=15. 11 is not > 15, so it doesn't set fullness to 11.

 

[stannius]

I believe this line:

Preferences.increment( "currentFullness", couldEat * fullness );

sets the fullness to what it would be if the eating had succeeded.

Actually this line right above it

int couldEat = fullness == 0 ? 0 : 
Math.max( 0, Math.min( item.getCount() - 1, 
(KoLCharacter.getFullnessLimit() - KoLCharacter.getFullness()) / fullness ) );

sets couldEat to 0 in the given repro steps (either mine or slyz's). So the line you quoted increments by 0.

It is line 4059

			Preferences.increment( "currentFullness", fullness * count );

that sets currentFullness to 15. Then the "you're already too full" code doesn't roll it back.

I installed subversion, ant, and the jdk so that I could build the jar myself. Then I rolled the code as far back as r6060 and the behavior was the same. Apparently I was misremembering what happened in this case - I was thinking of the case of currentFullness=0 and you eat something.

In fact, I don't think there is any way that couldEat could get a value greater than 0. If that is true, then the code is not working exactly as it's supposed to.

 

[Theraze]

Nope... because if fullness is down where it thinks that you can eat, then couldEat should be >0. If not, then it should reject your eating as it does when you already have (according to mafia) 15 fullness. The way that math goes is like this:
1) If the item desired to be eaten causes 0 fullness, set couldEat to 0.
2) Set to the max of the following:
2a) 0.
2b) Whatever is lowest of the following:
2bi) Total item count -1.
2bii) (Current fullness max - current fullness) divided by how much fullness is caused. In the case of a 15 max, 10 current, and a 5 fullness item, this would be (15-10)/5=1 since 15-10=5, and 5/5=1.

Your math arguments still fail to convince me...

But on a math sidenote, I was pondering Hotstuff's 12 days of Christmas total present tweet of:

N(N+1)(N+2)/6

being total presents, and came up with that

((N+1)^3-(N+1))/6

gives the same result.

 

[Veracity]

It is line 4059

			Preferences.increment( "currentFullness", fullness * count );

that sets currentFullness to 15. Then the "you're already too full" code doesn't roll it back.

I think this is the issue. Revision 8874 resets the fullness added by registerRequest in the "too full" processing before calculating what being too full says about our real fullness. I also commented and refactored the "too full" code to, hopefully, be clearer.

 

[slyz]

I managed to pinpoint the part of the code that was working, but not the part that caused the issue at hand. Thanks Veracity =)

1) If the item desired to be eaten causes 0 fullness, set couldEat to 0.
2) Set to the max of the following:
2a) 0.
2b) Whatever is lowest of the following:
2bi) Total item count -1.
2bii) (Current fullness max - current fullness) divided by how much fullness is caused. In the case of a 15 max, 10 current, and a 5 fullness item, this would be (15-10)/5=1 since 15-10=5, and 5/5=1.

Your math arguments still fail to convince me...

if you are trying to eat 1 food, 2bi = 0 => 2b = 0 => 2 = 0 => couldEat = 0.

couldEat is not the number of food you would have eaten if eating had succeeded, it's the number you were actually able to eat. And 2bii is just a higher limit, I think it's there as a safeguard.

 

[Theraze]

Well, those happened before the incrementing gets calculated. So 2bii actually had the capability of equalling something above 0, but... Yeah, you're right. It's a sort of weird failsafe. Veracity's fix should work better than this code was able to.

 

[stannius]

I think this is the issue. Revision 8874 resets the fullness added by registerRequest in the "too full" processing before calculating what being too full says about our real fullness. I also commented and refactored the "too full" code to, hopefully, be clearer.

I tried the latest build and it now works as I expected. Much grass!