Bug - Fixed HP > 2^31 goes negative or zero; errors

Veracity

Developer
Staff member
Brief code browsing:

AdventureResult.HP and AdventureResult.MP have their values in the "count" field of an AdventureResult. So does AdventureResult.MEAT.
"count" is an int. An Adventure result also includes a priority (int), id (int), and name (String). Making count into a long will not double the size of an AdventureResult.
KoLCharacter has Meat, HP, and MP variables as ints and the methods to return them (getMaximumHP(), etc) return ints.
Numeric Modifiers are doubles. IEEE doubles have 53 bits of precision, which is fewer than 64 for a long - but more than 32 for an int.

ASH ints are Java longs. ASH floats are Java doubles. Runtime functions that return Meat, HP, MP, etc. call KoLCharacter methods and pass the returned value ro the Value(long) constructor to make into an ASH value. Currently, that just coerces the int to a long. If the KoLCharacter methods return longs, no additional memory will be used.

Conclusions:

- changing AdventureResult.count from an int to a long will change it from {int, int, int, String} to {int, int, long, String}. Which is to say, it'll add about an int to each one. I expect there are 10s of thousands of AdventureResults
- parsing and printing Meat/HP/MP/... will need to use Long methods rather than Integer methods
- methods that manipulate Meat/HP/MP/... (including using modifiers) will need to calculate using longs rather than ints
- (when using numeric modifiers, I would expect "loss of precision" warnings when coercing longs to doubles. I seriously doubt that Meat/HP/MP/... will really consume more than 53 bits, so I don't think there will be real overflow)
- ASH should require no changes since its ints are already longs.

So, yeah - there will be lots of places to change, and there will be a (somewhat) increased memory footprint, but it doesn't look that difficult or impactful.
 

Veracity

Developer
Staff member
On the other hand, perhaps it would be easier to do something similar to what we did when we removed the int[3] array from AdventureResult (used for substats and fullstats) and created AdventureMultiResult extending AdventureResult. We could create AdventureLongCountResult which extends AdventureResult and has a long count field and an appropriate getLongCount() method. We'd have to use that variant when manpulating MEAT/HP/MP/... but we wouldn't need to worry about all the places that assume that an int is good enough for item quantities, effect duration, and so on.

I'll try that.
 

Veracity

Developer
Staff member
I have a patch which uses that technique. It required changes to 61 files or so.

It would have been a lot easier if I had made only HP and MP have long values, but I decided to do Meat as well, and that opened up a whole bunch of pain, since Meat is an Ingredient for concoctions, is a Coinmaster token for Cosmic Ray's Bizarre, can be transferred to/from closet, from storage, via kmail, and so on. A fair amount of complication came because Item AdventureResults are still limited to an "int" count, so, for example, if you convert 300 million Meat into meat paste, we'll exceed that. I think my response is "don't do that".

I have a lot of testing to do before I release this into the wild.
 

lostcalpolydude

Developer
Staff member
A fair amount of complication came because Item AdventureResults are still limited to an "int" count, so, for example, if you convert 300 million Meat into meat paste, we'll exceed that. I think my response is "don't do that".

KoL caps out at 2^24 - 1 of an item in any location, so using an int for tracking item quantities should always be sufficient.
 

Veracity

Developer
Staff member
Well, I I limited item counts in creations, etc., to Integer.MAX_VALUE. Sounds like we could (if we wanted to protect people from doing stupid things), enforce the limit you mentioned to limit purchases, creations, etc., taking into account the number the user requested + the number already in inventory.

That's a bit beyond the scope of this.

Revision 19629 stores Meat, HP, and MP variables as longs, rather than as ints, and when they are made into AdventureResults, stores them in the AdventureLongCountResult subclass.

I tested a lot of things, including running my farming script for a bunch-of-turns, wherein Meat, HP, and MP were gained and lost, and it all looks normal.

I would not be at all surprised if there are obscure issues, but I expect other people doing other things will notice and report them, and I will fix them.

I look forward to the OP reporting on how MP and HP work for him now. :)
 
Ooohh, shiny :) I see max HP 2,727,798,026 and max MP 2,152,494,654 in the left pane. Unfortunately, I checked this thread after I did all my dailies (and overdrank), will have to wait until tomorrow for a full run through.

CHIT working nicely now. No more error messages.

Trying a full restore using "Your own black heart", not something I have done in a while. From the GCLI:

Code:
use your own
(usable quantity of your own black heart is limited to 0 by needed restoration).

Huh. Was able to use it from the relay browser. Left pane shows full HP and MP. GCLI shows correct gains.
Cast now works (e.g. "cast * dice").

Thank you very much for the early Crimbo present! I'll report more tomorrow.
 

Veracity

Developer
Staff member
Yeah - I didn't notice that the RestoreDatabase was chock full of code that assumed that HP and MP fit in an int.
Revision 19630 should fix that.

I tested with "ash $item[ your own black heart ]" and it showed the min & max HP & MP expected. Not that they are remotely close to yours. :)

Thank you for testing with Summon Dice; It was a fair amount of effort to make Libram skills handle mongo amounts of MP, and I had not yet tested them.
 
Last edited:

Veracity

Developer
Staff member
Revision 19631 simplifies (and rolls back) a lot of the Concoction stuff. I had made a bunch of variables longs, rather than ints, to allow for Meat as an ingredient being able to use more than an int's worth. Lost's comment that KoL itself won't allow more than 2^24-1 of a particular item in inventory makes it extremely unlikely that you'd want to do that.

So I made the maximum amount of Meat we'll consider during creation is Integer.MAX_VALUE - 2,147,483,647

This does mean you'll only be able to make 2,147,483 dense Meat stacks at a time, even though inventory could contain 16,777,215 of them. Too bad.
 
Everything went smoothly today. Retried "your own black heart", works as it should.

Meat: I was under the impression KoL only allowed maxint meat on hand. <checks wiki> Ok, that would be 2^32, apparently independently determined for inventory, closet, storage (speculating, it's not explicitly stated). So only 4,294,967 dense meat stacks can be created at a time anyway - ?

I assume the individual item limit is also the same for each storage location, including the display case?

Getting off on a tangent there. Thanks again for this fix!
 

Veracity

Developer
Staff member
2^32 is more than fits into a Java int - and Java has no unsigned int - so to represent it, Java needs to use a long. Which I did.

I’m still using an int to store how many things are available to be used to create something. “Thing” includes pieces of Meat.
 
Related, so resurrecting, but more for the record:

Passed 2^32 HP a while back, wasn't getting full HP restored, but I got lazy and didn't check into it until now. Appears to be that KoL itself limits current HP to that value (-1), while max HP can exceed it. Currently showing 4294967295 / 5058996231. Repeatedly casting cocoon doesn't change it.

Still a bit to go before MP gets that far. But mafia itself is still working in this regard, so thanks again!
 
Top