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

r19197. First showed a negative value (signed int overflow), then repeatedly shows the following in the gcli (edit: which I now realize is charpane.ash), and shows zero in the (mafia) character pane. Sometimes returns to the negative display.

Code:
2148486140 is out of range, returning 0
Division by zero (charpane.ash, line 2536)
  at addHP (charpane.ash:2573)
  at addSection (charpane.ash:2661)
  at bakeStats (charpane.ash:2754)
  at bakeBricks (charpane.ash:3874)
  at modifyPage (charpane.ash:4127)
  at main (charpane.ash:4140)2148486140 is out of range, returning 0

Otherwise seems to work normally.

I was curious what would happen when I reached that point. That's my answer.

Vanilla KoL is showing "2144789018 / 2148486431" in the character pane.

Also, sunuva…:

Cannelloni cocoon: "Was silently nerfed to heal up to 1,000 HP instead of all HP on April 24, 2019." wiki

Wow, that's huge.
 
Last edited:

heeheehee

Developer
Staff member
(actually, the out of range is returned by StringUtilities.parseInt, which is in turn called by CharPaneRequest.)

I'm poking around because I'm curious how intrusive a change this'll be, but don't hold your breath.
 

heeheehee

Developer
Staff member
I'd also like to know what api.php?what=status&for=testing yields, namely around the max HP readouts. JSON doesn't actually have a long type, although I guess our JSON library is okay with that.
 

Darzil

Developer
(actually, the out of range is returned by StringUtilities.parseInt, which is in turn called by CharPaneRequest.)

I'm poking around because I'm curious how intrusive a change this'll be, but don't hold your breath.

A thorough fix is hard, as I've looked at this before. Because AdventureResult will need to handle longs, you have to handle every use of an Item Number and similar entry to update to longs. Unless of course there is a simpler way that I didn't find. There are thousands!
 

heeheehee

Developer
Staff member
Are there AdventureResults that contain HP or MP? Stats, yes, and Meat, yes, but I don't think that's true for HP / MP.
 

heeheehee

Developer
Staff member
Yeah, that's what I was looking for. That says your HP is less than 2^31 (which starts with 214748...). Is that accurate?
Code:
"hp":"2147398484",
...
"maxhp":2147399416

Also a little bit weird that the hp field is a string, but I guess that's just how it usually is.

(also, Darzil: got it, it's most likely as invasive as you mention.)
 
Odd; I remember it being 2148... in vanilla KoL yesterday.

Vanilla: Current max hp is 2176875415 in both character pane and api result. Well, sort of. Every time I refresh either the character pane or the api the last four digits change. Also changing are the last few digits of max mp and mus/mys/mox. It looks like a precision problem somewhere, which for integer types is - weird. Or, possibly more likely, some oddity in the calculations on the KoL side (floating point calcs someplace?). You can get the same effect by visiting the character sheet for #2422322 and reloading.
 

heeheehee

Developer
Staff member
(the main reason I ask is that I vaguely recall some weirdness around KoL losing precision at some point, but I don't remember the details. PHP does something along the lines of promoting integer types to floating point, but it's not totally consistent with what I remembered.)
 
Any further thoughts on this? A handful of superficial point fixes might be enough, in lieu of revising the infrastructure. Perhaps where this shows up in the UI, the parseint method noted earlier, and/or the values most susceptible to this sort of thing.

Short of that, and recognizing that this isn't a widespread concern, I may have to take a look at it myself. At this point I don't use the relay browser while running mafia, since it (mafia) eventually locks up. When I need to use the browser I just log into KoL directly. I haven't looked into the lock-up issue at all so far. Or, if I really need to bypass this issue, I don't *have to* equip a sphygmayomanometer...
 

heeheehee

Developer
Staff member
As I understand it, things that would need to be updated, even for a bandaid fix:
- how we store the value of max/current hp/mp.
- where we parse the value (api.php, charsheet.php, charpane.php; I think that's all).

Healing large amounts of HP or MP may result in, uh, weird results until you re-sync with KoL per one of the above locations.

I might be able to look at this some more in a few days.
 

heeheehee

Developer
Staff member
Hm. Even ignoring the AdventureResult -> long change, there's a lot of changes caused by changing MP to be a long, as well (especially around skill casting); there's a few changes to Modifiers, as well. Probably a lot more changes required, which I haven't dug into all the way -- it'd probably require changes to 100+ files.

A secondary bandaid fix would be to leave most of the existing code intact, except have a separate accessor for "big" HP / MP values, used only in a few places (ASH, the gui). I don't really like that, since it's a hack upon a hack.
 
hacked hack: yeah, I get that, sort of what I was expecting. But given how extensive the changes are for the alternatives, it might be justified in this case. I guess one question is how much of a risk does the bandaid represent for operational stability, and for future maintenance.

Does changing to longs have value for other reasons? This particular case may not justify it.
 

heeheehee

Developer
Staff member
AFAIK, the arguments for switching to longs for internal representations mostly involve high-level (or wealthy) characters.

One downside of switching everything to longs will be noticeably increased RAM usage (no more than 2x, but I wouldn't be surprised to see it as high as a 30% increase).
 

xKiv

Active member
One downside of switching everything to longs will be noticeably increased RAM usage (no more than 2x, but I wouldn't be surprised to see it as high as a 30% increase).

Not necessarily, unless the bulk of changed int/long usage is arrays. JVMs are free to allocate with any alignment/padding they decide to, both between properties and at the end of an object. An int might actually be internally the same as long, on platforms where that's significantly faster. There's some per-object overhead, and other properties.
Not to mention, on the JVMs I am familiar with, using bigger objects will just normally means that the program chews through heap faster, triggering GC more often - but does not allocate more RAM. It will have higher permanent usage of heap, but likely the same heap, and thus the same RAM usage when viewed from the OS.

It's a thing that would have to be measured in real situation to see if the effect is significantly different from anybody's guess.
 
FYI - Update on this: just exceeded 32 bit int limit for MP, and that's far more painful than the HP situation, which I had learned to live with.

Re resource usage / GC / memory: unsigned int could work. But that probably saves no effort, and may leave things open to other issues.

Sounds like something I'll need to look into myself, but the earliest will be January.
 
Top