ryobg / JContainers

JSON-based data structures for Papyrus - the TESV Skyrim SE scripting language
MIT License
107 stars 23 forks source link

JLua.evalLuaInt issue with long numbers #40

Closed RealAntithesis closed 6 years ago

RealAntithesis commented 6 years ago

I've been trying to replicate in lua the SKSE CRC32 function in HashUtil.cpp in order to calculate an itemID hash of the item display name and the formID. I've successfully done this - lua is calculating the correct hash. The problem is getting this number back into papyrus.

The hash calculated in lua is the same as the hash returned using the SKSE functions GetWornItemId() and GetEquippedItemId(), as well as calling the CRC32() and CalcItemId() functions directly in HashUtil.cpp and PapyrusActor.cpp respectively (albeit, via an online c compiler), so I'm pretty sure my translated lua function is correct.

However, after a certain length, numbers don't seem to be returned from lua correctly and start showing errors - the hashes being returned to papyrus were not the same as the hashes calculated in lua or the original c function. So I did a test, just passing numbers into lua and back out again to papyrus, and found the following (extracted from my log):

[07/19/2018 - 09:26:35PM] Debug(): test lua result. 1 = 1 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 12 = 12 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 123 = 123 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 1234 = 1234 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 12345 = 12345 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 123456 = 123456 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 1234567 = 1234567 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 12345678 = 12345678 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 123456789 = 123456792 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 1234567890 = 1234567936 [07/19/2018 - 09:26:35PM] Debug(): test lua result. 12345678901 = -2147483648

Passing 12345678 to lua returned 12345678 back to papyrus, but passing 123456789 into lua incorrectly returned 123456792 to papyrus.

Is there some sort of length restriction to numbers somewhere in the pipeline? These numbers seem to be passed into lua ok (I printed them out into a lua log and they were exactly the same as what I entered in papyrus), but something gets messed up on the way out.

Papyrus JLua.evalLuaInt() function:

JLua.evalLuaInt("return AH_Hotkeys.NumTest(123456789)", 0))

Lua function:

function AH_Hotkeys.NumTest(testNum)
    AH_Hotkeys.LogMessage("NumTest = "..testNum)
    return testNum
end

Edit: I am able to return the hash results from lua correctly using strings (using evalLuaStr and a wrapping function at the lua end for tostring()), and then converting them back into ints in papyrus. Which I suppose is fine for now (this isn't a time critical operation), but this is a workaround and ideally returning the full int values should be possible?

ryobg commented 6 years ago

Around the 9/10 digit you may start to kick in some limitations, which I haven't confirmed.

Is the Lua function NumTest the same one which outputs the log mentioned up there? In fact, I would need much more information to understand what's happening. So, if it is not a problem, I guess you can continue use the "string" solution.

RealAntithesis commented 6 years ago

Thanks. The log output comparing numbers sent to lua and returned back to papyrus is from a papyrus function. I confirmed that the numbers being received by lua were correct in lua using the log function in the lua code. The discrepancy in the number being output from lua to papyrus is being shown in the papyrus function.

While I have worked around the problem, I'm wondering whether the root cause of the errors could potentially affect other things. At the very least, it's something that will need to be kept in mind when using lua functions and which may also be an issue for others who don't know about it (I'm assuming this isn't particular to me, but I don't know that yet).

What more information do you need? I can have a go. Thanks.

ryobg commented 6 years ago

Would be nice to see the actual Lua/Papyrus code used for that test. I can make some tests myself later these days too.

SilverIce commented 6 years ago

double -> int conversion may cut-off some numbers. double type can hold much larger values

How you could get such a large value? Skyrim SE Int has 8 bytes length?

ryobg commented 6 years ago

I'm not sure what you talk about.

The integers are 32-bit ones.

RealAntithesis commented 6 years ago

The following code reproduces the issue: Lua:

function AH_Hotkeys.NumTest(testNum)
    -- AH_Hotkeys.LogMessage("NumTest = "..testNum)
    logFile = io.open("AH_Hotkeys Lua.log", "a+")
    logFile:write(tostring(testNum), "\n")
    logFile:close()
    return testNum
end

Papyrus:

function TestLua()
        int testLuaInt = JLua.evalLuaInt("return AH_Hotkeys.NumTest(1234567890)", 0)
        string debugText = "Debug(): test lua result. 1234567890 = " + testLuaInt
        ; DebugMessage(debugText, debugLevels_kDebug)
        Debug.Notification(debugText)
endFunction

Lua log result:

1234567890

Papyrus log result:

[07/24/2018 - 07:40:33AM] Debug(): test lua result. 1234567890 = 1234567936
ryobg commented 6 years ago

I have played a bit. And I have a theory. By this time you can only use evalLuaInt for numbers between [-16 777 216; +16 777 216]. It connects a bit with what @SilverIce mentioned. I do not know why the Lua integration was designed like that, so I'm reluctant to change anything by this time.

@RealAntithesis you can continue with strings or use other kind of hash function. Or just stick to the lowest 24-bit of CRC32 ^^

SilverIce commented 6 years ago

I do not know why the Lua integration was designed like that

I.e. you don't know why integers were 32-bit or why Lua supports only double numbers? 12345678901 is 2 DF DC 1C 35 which takes more space than 4 bytes

ryobg commented 6 years ago

I haven't dig enough. Most likely the shipped Lua implementation does not supports integers. Then I see that the current number type looks like 32-bit float, if it was with double precision, then it would be possible to correctly represent all integers in the 32-bit range. Also I have to correct myself - it was not an intentional constraint.

It is an worthy note to users.

ryobg commented 6 years ago

@RealAntithesis I just want to clarify it for you.

Your first failing number is 123 456 789, which in your test case is the one first to pass over 16 777 216 boundary which I mentioned before. All integers outside this zone cannot be represented exactly. This is limitation of the current Lua implementation.

You can either stick to 24-bit numbers or use strings or other means to achieve your goals. I may put a note in the API docstrings about this. Let me know if you have further questions.

ryobg commented 6 years ago
RealAntithesis commented 6 years ago

Is that what is happening though? The numbers being received in lua are fine, which shows that both lua and papyrus support these numbers (I think).

It's only when the numbers are going in the opposite direction (lua to papyrus), that errors start being introduced. Also, the errors don't appear to be simple overflow type errors (except in the previous example when the number became negative) - the number being returned back to papyrus is actually greater than the number sent to and received by lua, but only by a relatively small amount and affecting only the last few digits.

I believe the maximum size of an int in papyrus is 2,147,483,647 plus a sign (int32), which would explain why the number became negative when adding the 11th digit. However, I understand that lua should accept bigger numbers? And it doesn't seem to explain why errors start appearing after the 8th digit. If the maximum size was 16bit, then it would overflow after the 5th digit.

Still confused. Edit: just read your recent post, so the limitation is a 24bit number?

ryobg commented 6 years ago

Yes.

I'm not sure what conversion exactly are going on when doing your test, but with the same luck it can be just a text what you log out. Then for sure, there is a conversion to integer when evalLuaInt returns and there you loose your precision.

Back then I haven't had the time, but nice proof would be to see how the number type in Lua behave. You can for example write a function which logs out each consequent integer after 16 777 216 and look for holes.

ryobg commented 6 years ago

I have inserted one warning into the Papyrus evalLua* functions.

RealAntithesis commented 6 years ago

Just to follow up, this may be an underlying issue with papyrus. Someone else has also tried to return a result from the crc32 function, but from the original actionscript version of it, and encountered the same problem with the last few digits being incorrect when the 32bit number was passed from actionscript to papyrus. The solution was the same - pass the number back as a string and cast back to an int.

ryobg commented 6 years ago

ActionScript? First time hearing it has to do something with Papyrus/SSE... Or you are not talking about the Adobe language?

RealAntithesis commented 6 years ago

This is from someone working with the ui swf files for the inventory menu, journal menu etc, which use Actionscript for the menu functionality. I gather that Bethesda decided to use Adobe flash for the Skyrim user interface (there are some skse functions that allow queries to be sent and returned to access actionscript functions and properties). Since this seems to be a common issue for both Actionscript and Lua, I'm guessing it's got something to do with papyrus itself.

ryobg commented 6 years ago

I can't confirm or deny. Clearly though, there is limitation of the transfer of integers between JC and its Lua.

На пн, 10.09.2018 г., 17:05 RealAntithesis notifications@github.com написа:

This is from someone working with the ui swf files for the inventory menu, journal menu etc, which use Actionscript for the menu functionality. I gather that Bethesda decided to use Adobe flash for the Skyrim user interface. Since this seems to be a common issue for both Actionscript and Lua, I'm guessing it's got something to do with papyrus itself.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ryobg/JContainers/issues/40#issuecomment-419924420, or mute the thread https://github.com/notifications/unsubscribe-auth/ANWRWH7Sx68Lrlm0W8RkbfRVaj68jMkMks5uZnGOgaJpZM4VWbcp .

SilverIce commented 5 years ago

From what I found, the issue is that there is a conversion: Lua (double - lua dont have Int) -> JC (float). Ofc float can't hold such a large integer value..

    void JCValue_fillItem(tes_context& context, const JCValue *v, item& itm) {
        switch (v ? v->type : item_type::no_item) {
        case item_type::form:
            itm = collections::make_weak_form_id((FormId)v->form.___id, context);
            break;
        case item_type::integer:
            itm = v->integer;
            break;
        case item_type::real:
            itm = v->real;
            break;

there must be something like:

    void JCValue_fillItem(tes_context& context, const JCValue *v, item& itm) {
        switch (v ? v->type : item_type::no_item) {
        case item_type::form:
            itm = collections::make_weak_form_id((FormId)v->form.___id, context);
            break;
        case item_type::integer:
        case item_type::real:
            // assign as int or float (float if it has numbers after coma)
            itm = v->real;
            break;

solution #2 is to make item::Real a Double