ryobg / JContainers

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

JFormMap functionality not working in VR #93

Closed Piranha91 closed 7 months ago

Piranha91 commented 2 years ago

Apologies if this is a mistake on my end, but I've twice encountered an issue where a script I compiled is functional in SSE using JContainers SE , but not in VR using JContainers VR (both version 4.1.13). My latest instance of the issue is in a very simple script:

Scriptname SynthEBDHeadPartLoaderQuestScript extends Quest

import PSM_SynthEBD

GlobalVariable Property loadingCompleted Auto

Event OnInit()
    debug.Notification("HeadParts OnInit")
    LoadHeadPartDict("OnInit")
EndEvent

Function LoadHeadPartDict(string caller)
    loadingCompleted.SetValue(0)
    int assignmentDict = JValue_readFromFile("Data/SynthEBD/HeadPartDict.json")
    if (assignmentDict)
        debug.Trace("SynthEBD: HeadPart Dict Read")
    else
        debug.Trace("SynthEBD: Failed to read HeadPart Dict")
    endIf
    int count = 0
    int maxCount = JFormMap_count(assignmentDict)
    debug.Trace("SynthEBD: HeadPart Count = " + maxCount as string)
    ;more functionality here, but irrelevant for the given issue
EndFunction

The supplied json file exists in both cases, and appears in its entirety as follows:

{
    "__metaInfo": {
        "typeName": "JFormMap"
    },
    "__formData|Skyrim.esm|0x13476": {
        "Hair": "__formData|Skyrim.esm|0x85DC8"
    }
}

The papyrus log from SSE says:

[06/05/2022 - 11:33:08PM] SynthEBD: HeadPart Dict Read
[06/05/2022 - 11:33:08PM] SynthEBD: HeadPart Count = 1

The papyrus log from VR says:

[06/05/2022 - 11:35:45PM] SynthEBD: HeadPart Dict Read
[06/05/2022 - 11:35:45PM] SynthEBD: HeadPart Count = 0

This seems to be identical to the issue I posted about on the SSE Nexus Page on March 27, just in a different context. Am I missing something about how JContainers works, or is the JFormMap functionality not working correctly in the VR build?

W-Drew commented 10 months ago

I can confirm this. Just a simple dummy test where we write a Form to a file & read it back fails:

; Write to file
int testData = JMap.object()
Form bilegulchForm = Game.GetForm(0x00019269)
If !bilegulchForm
    Debug.MessageBox("Hardcoded bilegulch form invalid")
EndIf
JMap.setForm(testData, "bilegulch", bilegulchForm)
JValue.writeToFile(testData, JContainers.userDirectory() + "testData.txt")

; Read back the very same data JContainers wrote
int loadedTestData = JValue.readFromFile(JContainers.userDirectory() + "testData.txt")
Form loadedBilegulch = JMap.getForm(loadedTestData, "bilegulch")

If !loadedBilegulch ; This path is triggered
    Debug.MessageBox("Failed to load bilegulch form we just saved")
ElseIf loadedBilegulch.GetFormID() != bilegulchForm.GetFormID()
    Debug.MessageBox("Form loaded, but does not match. Loaded="+loadedBilegulch.GetFormId()+" vs Hardcoded="+bilegulchForm.GetFormId())
Else
    Debug.MessageBox("Successfully loaded saved bilegulch Form")
EndIf

The written string to testData.txt is valid

{
  "bilegulch": "__formData|Skyrim.esm|0x19269"
}

and it can be parsed just fine in JContainers SE.

ryobg commented 10 months ago

I'm sorry I can't help you here guys. No VR, even less time to try and guess. A good start would be to look here I think: src/skse/skse.cpp:123.

RafearTheModder commented 7 months ago

I have ran across this issue when building my list for VR as well, and I think I am onto a fix for it. It appears getting base functionality is as simple as just removing the "#ifndef JC_SKSE_VR" and its related #endif from the function you linked at src/skse/skse.cpp:123 so that the code runs regardless.

Worth noting however that these days we also have VR ESL support which would require extra steps in there and should be accounted for as well, so it would probably be better to add an #else clause instead and handle the logic there to either just do the normal method or handle via VR ESL support if it is installed.

I should be able to get that done and make a PR to resolve this issue this weekend if you want. If so, would you rather the PR go to the latest development and cut a new release from there, or fork off of the v4.1.13-vr tag to minimize changes? Or, since you do not have VR to test would you rather I fork the repo and maintain a separate VR version of this myself so you don't wind up maintaining something you cannot test? I am fine either way.

ryobg commented 7 months ago

I should be able to get that done and make a PR to resolve this issue this weekend if you want. If so, would you rather the PR go to the latest development and cut a new release from there, or fork off of the v4.1.13-vr tag to minimize changes? Or, since you do not have VR to test would you rather I fork the repo and maintain a separate VR version of this myself so you don't wind up maintaining something you cannot test? I am fine either way.

For the time being a tested PR would be better, it is easier for the folks to see the releases at one place. For example, Nexus Mods.

RafearTheModder commented 7 months ago

I have now made a pull request with my tested and locally proven fixes for this issue.