ryobg / JContainers

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

Light form ID's fail to resolve #60

Closed Ryan-rsm-McKenzie closed 5 years ago

Ryan-rsm-McKenzie commented 5 years ago

If you just serialize the entire form ID and use SKSESerializationInterface::ResolveFormId, then you don't have to worry about changes to the load order.

RealAntithesis commented 5 years ago

I just came around to report this. Forms from ESL or ESPFE plugins referenced in JC objects become invalid when loading a saved game. In papyrus, this is shown as nil. In lua, there is still something there, but the reference changes to something else when using Form(formID).

For now, I've switched to using formIDs and checking all forms and replacing them with Form(formID). Seems to be the only way at the moment.

ryobg commented 5 years ago

Hello @Ryan-rsm-McKenzie and @RealAntithesis ,

I get there is some issue with ESLs, but your remarks are somehow confusing and I don't get what you mean. Could you be more specific? Drop an example(s) or something.

Thanks!

ryobg commented 5 years ago

Meanwhile, I digged around and I believe I see what could be the issue. During Co-Load, the forms have to be restored back. For that purpose, SKSE is asked to resolve the form handles, and I see that at least between 2.0.12 and 2.0.15 they have changed how this works. And without knowing much, it seems that there is a bug introduced in the ResolveHandle@Serialization.cpp:

                UInt8   modID = handle >> 24;

        if (modID == 0xFF)
        {
            *handleOut = handle;
            return true;
        }

        if (modID == 0xFE) // Here the ESL mod id is cut down to just 8 LSB
        {
            modID = (handle >> 12) & 0xFFFFF;
        }

Hence in some cases the resolving will fail and the form returned will actually reside in the Skyrim.esm (as fallback). Now, I can't verify my theory as I'm far from any test setup, nor I know how I can contact the SKSE devs about this.

RealAntithesis commented 5 years ago

Thanks for looking into this. Re: my comment about the form references changing, I was making an incorrect assumption about this I think. On reloading a save game, a form read in by JContainers from an existing JMap via lua, and output to the log using tostring(theForm), would show a different string value after using the function Form(theFormID). But the resulting string (tostring(theForm)) changes for all forms after using Form(theFormID) for some reason, not just the forms that were from an ESL or ESPFE plugin, so I'm not sure what tostring(theForm) is displaying. I was doing this to work around the issue of 'disappearing' forms by getting the formID for all forms I need to store in JObjects, and validating them all on reload by using Form(theFormID) in lua. This seems to work fine. It's been becoming more and more of a problem recently for myself and others due to the rising popularity of ESLs and ESL-flagged ESPs, with people (myself included) converting traditional ESPs to ESL-flagged ones to add more plugins into the load order. This was causing more items to become unrecognised in AH Hotkeys when reloading a save.

Ryan-rsm-McKenzie commented 5 years ago

I'm saying that from looking at your implementation, it's apparent that you attempt to resolve load order changes yourself. Resolving load order changes is as simple as serializing the form ID (you don't need to know anything about the mod this form came from). Then, during loading, use SKSESerializationInterface::ResolveFormId to get the form ID's new ID, accounting for load order changes. Then, look the form ID up. If you get a form pointer back, the form is still present in the load order, otherwise it was removed. Here is an example:

void Save(SKSESerializationInterface* a_intfc, TESForm* a_form)
{
    a_intfc->WriteRecordData(&a_form->formID, sizeof(a_form->formID));
}

void Load(SKSESerializationInterface* a_intfc)
{
    UInt32 formID;
    a_intfc->ReadRecordData(&formID, sizeof(formID));
    a_intfc->ResolveFormId(formID, &formID);
    auto form = LookupFormByID(formID);
    if (form) {
        // form is still present in load order
    } else {
        // form is not present in load order
    }
}
ryobg commented 5 years ago

@Ryan-rsm-McKenzie looks like I dont get it still... During load, the form handle is resolved not by JC, but by SKSE, and ResolveHandle is used (not ResolveFormId, though they are similar). There is no need for LookupFormById.

Ryan-rsm-McKenzie commented 5 years ago

You can't use resolve handle on a form ID. Resolve handle is for resolving script object handles, which are not the same as a form.

ryobg commented 5 years ago

I can't say on 100%, but looking at how the original author did it, it looks valid. Note that the handle is serialized, not the form id itself. Point is the backend (SKSE) implementation changed, and probably bugged.

Ryan-rsm-McKenzie commented 5 years ago

Perhaps you can help me. I'm under the impression that your plugin's serialization is done through this interface here. Is that correct?

ryobg commented 5 years ago

Yes, and judging by the issue reports, resolve_handle. Maybe, it will be faster/easier if you just fork and prove your theory there as it most likely the place which needs to handle the fix/workaround. I can't currently test anything though, so I'm just theoritzing and thats bad idea long-term.

ryobg commented 5 years ago

Fix is available for testing on https://github.com/ryobg/JContainers/releases/tag/v4.1.10 @RealAntithesis can you look at it too? Thanks!

RealAntithesis commented 5 years ago

Seems to be fine now as far as I can tell - forms that I had to 'reconstitute' on every game reload now seem to be fine. Thanks.

ryobg commented 5 years ago

Oke, will release soon.