ryobg / JContainers

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

Support installations with Creation Club mods - lookup mod index by name #16

Closed tasairis closed 6 years ago

tasairis commented 6 years ago

The work that went into tracking this down is long and NSFW so I'll try to summarize:

  1. Creation Club ESL masters load after official ESMs and before other mods.

  2. Skyrim and SKSE track mods according to their literal load order - not the plugin order everyone is familiar with. They used to be the same thing, but with the advent of CC ESLs are not anymore.

  3. SKSE's GetModByName, and likely other functions based on mod names and "load order", work according to that literal load order value.

Therefore, if one CC ESL master is loaded then it will be in the FE plugin order group but occupy position 05 in the load order (00-04 are the five ESMs). If Mod.esp is at 05 in the plugin order then it will be bumped down one spot to 06 in the actual load order, and this latter value is what gets reported through SKSE.

4a. JContainers unserializes "__formData|Mod.esp|0x123456" by looking up the load order for Mod.esp, shifting left, adding the relative form ID, and calling GetForm with it. This example would be calculated as 06123456, however to Skyrim the form is actually 05123456. The form will not be loaded.

I think the best solution is to have JContainers use the native GetFormFromFile function, bypassing all the load order confusion. I've tested in-game through Papyrus and it works correctly for regular mods, but I only assume it works correctly for ESLs.

It could be argued that the bug is in SKSE - easily, even - however I'm not sure how it would deal with ESLs. They can't all be at load order FE/FF after all. Or could they? A new set of methods might be required. But frankly, I think GetFormFromFile is the most appropriate method for JContainers to use to begin with.

4b. Untested but I assume serializing will have a similar problem: a form 05123456 should correspond to Mod.esp's xx123456, but JContainers (through SKSE) will see that the mod at position 05 is whatever.esl and thus serialize the form incorrectly. It would accidentally unserialize back to the original Mod.esp form though.

I don't know what JContainers could do to solve this one.

4c. I don't know how this impacts SKSE cosaved data.

I can write some Papyrus repro code if needed, and right now there are a couple Creation Club mods that are temporarily free to download (the mudcrab and some spell/mage clothes thing).

ryobg commented 6 years ago

Hello @tasairis, thank you for all that.

I would like to ask about some abbreviations: FE and FF - the rest I'm familiar with, but I would like to avoid confusion.

I have no experience with these new .esl files, neither with Creation Club, so small and easy to setup test case would be greatly appreciated.

Then there is something in your example: Mod.esp is reported as 0x06123456 currently, but it should be 0x05123456. Which means that this .esl files form ids are skipped over - do they have ones at all?

tasairis commented 6 years ago

Ah sorry, FE/FF as in hex 0xFE/FF = 254/255.

Arthmoor's explanation of ESL files ESL files do get form IDs, but they're assigned as 0xFE(AAA)(BBB), where AAA=000-FFF according to the order the files are loaded and BBB is the relative form IDs that the mod can use for its own new forms. That means the game supports 4096 ESL files (=AAA) and each one can have 2048 forms (=BBB, 800-FFF). And apparently it supports more than 4096 ESLs because the 4097th will go into the 0xFF position.

So the normal method of calculating the final form ID as (load order<<24 + relative ID) won't work for ESLs. To make it worse, Creation Club ESLs marked as masters are always loaded after the Skyrim+DLC ESMs: they get load orders starting at 05 but their forms go into the 0xFE-prefixed range. And even worse, the first regular mod will have some load order after 05 (depends how many ESLs there are) but its forms will go into the 0x05-prefixed range, so the normal method doesn't work for those either. And that's the problem here.

Unfortunately I don't have a mod you can get and test this out, and unfortunately it takes a bit of effort to set up a test case.

  1. Go into Creation Club and get a free mod or two. Currently there's a dwarven mudcrab and something about mage clothes I didn't read too closely. Doesn't matter what, just need something installed.
  2. Find any ESP mod you want (or already have) that defines a new form. Quest, actor, whatever. Note the form ID for testing.
  3. Use some Papyrus with JContainers' serialization and somehow cause it to run. Quest fragment, spell, whatever.

The Papyrus code to test goes like this:

; first verify that the game is working
Form f1 = Game.GetFormFromFile(0x123456, "WhateverNormalModYouChose.esp")
if f1
    Debug.Notification("GetFormFromFile: found")
else
    Debug.Notification("GetFormFromFile: not found")
endif

; now test jcontainers with the same form
Form f2 = JString.decodeFormStringToForm("__formData|WhateverNormalModYouChose.esp|0x123456")
if f2
    Debug.Notification("decodeFormStringToForm: found")
else
    Debug.Notification("decodeFormStringToForm: not found")
endif

; use GetModByName to see what's going on
int loadorder = Game.GetModByName("WhateverNormalModYouChose.esp")
int effective = f1.GetFormID() / 0x1000000
Debug.Notification("GetModByName: " + loadorder + ", effective=" + effective)

What you'll see is that GetModByName reports one number, the "effective" number derived from the full form ID is a different number, and the difference is the number of ESLs you have. Disable the ESLs (eg, move the files out of Data) and everything works.

(edited for factual accuracy)

ryobg commented 6 years ago

Ok, I have digged on dry land for some time now.

In Papyrus terms, JC uses the SKSE's GetModByName, currently I see no way to use GetFormFromFile, but if we assume on 100% that:

I can elaborate a bit on the JC implementation and get the correct mod indices (anyway search continues - just dropping a note).

tasairis commented 6 years ago

Assuming the MSB is 0xFE is probably fine, since it only overflows to 0xFF after 4000+ ESL mods and surely no one in their right mind would have that many.

I know SKSE doesn't have it, so would it help if I could find the address for GetFormFromFile? It seems to be a bit trickier than in Skyrim x86, but with it you should be able to

typedef TESForm* (*_GetFormFromFile)(UInt32 id, const char* file);
RelocAddr<_GetFormFromFile> GetFormFromFile(0x???);

That way you wouldn't have to care about special handling for ESLs at all.

tasairis commented 6 years ago

Ugh. I don't understand x64 calling conventions. Address appears to be 0x9726C0. rcx=something (this pointer?), rdx=something, r8=something (mostly zero?), r9 and rbx=relative form ID, stack+0x28 is filename string pointer.

Maybe it's too complicated to deal with.

tasairis commented 6 years ago

One more thing I forgot to mention. I'm not sure how you're thinking of handling ESLs (wouldn't it require scanning the mod list for *.esl?) but note that (a) only CC ESLs, identified by a filename pattern, are explicitly loaded between the ESMs and regular user ESLs are subject to load order like normal, and (b) it's quite possible for a user to have a regular mod at the 0xFE position because that MSB is not reserved for ESLs... but if this happens it can potentially create terrible problems and is a bad idea.

... SKSE should probably get improvements to handle this. I know it has some additions for ESLs but I didn't see an API that would be useful here (not that I looked too hard).

edit: Waitwaitwait, something's wrong here. SKSE already separates regular and light master mods. I need to look some more.

tasairis commented 6 years ago

So ignore all that stuff about GetFormFromFile and whatever.

I made a couple simple changes and have a build of JContainers that works with and without CC mods installed. The key is that SKSE/Skyrim has three lists of mods: a complete list of everything according to the literal load order, a list of regular mods, and a list of Creation Club mods. If I modify JContainers to search the list of regular mods then it works correctly.

The problem: those two other lists are not exposed through the SKSE API so I had to hack that in. Hopefully you can see a better way?

from_string() in form_handling.h:

- modIdx = skse::modindex_from_name(long_string);
+ modIdx = skse::loaded_modindex_from_name(long_string);

skse.h:

  uint8_t modindex_from_name(const char * name);
+ uint8_t loaded_modindex_from_name(const char * name);

skse_real_api in skse.cpp (plus additions to the other structs):

+ uint8_t loaded_modindex_from_name(const char * name) override {
+     return DataHandler::GetSingleton()->GetLoadedModIndex(name);
+ }

There would also need to be similar changes for to_string() and the index-to-name direction. And ideally there should be more logic to support CC mods, but that could be treated as a separate feature request.

ryobg commented 6 years ago

Somehow this was my idea too - yes. But instead of using the list of regular mods only, I wanted to use and the list of CC mods so that you can load forms from them too. Btw, the 0xFFxxyyzz range can't be used by those lists.

tasairis commented 6 years ago

With some quick testing it appears that list of CC mods is not strictly Creation Club but all ESL files: I created an empty mod in Creation Kit, had it converted to a light master, tried it in game, and my test modification to JContainers worked - meaning the mod was not included in the regular mod list and was in the other "CC" list. So SKSE calling it loadedCCMods is not entirely correct. Which is very good.

I don't see any nice way to identify what's a CC mod and what isn't according to SKSE's ModInfo struct. The information could be in there but either I don't see it or it's one of the unknowns. That means the only way to tell is to look at the file extension. Could be worse.

But as things stand, non-CC mods will probably cover >99% of use cases for JContainers, and I think it's also most likely that a particular mod being looked for will have been loaded. It may not be as pretty but I think I would go for a fallback-style approach: look up in the regular list and derive the form ID normally, or else look up in the CC list and derive the form ID with the ESL algorithm. It's also very simple to do.

index = skse::get_from_regular_mod_list(name);
if (index == FormGlobalPrefix) {
    index = skse::get_from_cc_mod_list(name);
    if (index == FormGlobalPrefix) {
        return boost::none;
    } else {
        local_id = (index << 12) | local_id;
        index = 0xFE + (index >> 8);
    }
}

Anyway, it seems the research-heavy part of all this is taken care of so it's your decision on the best implementation.

ryobg commented 6 years ago

Yes, straightforward solution can do. I have something else in my mind though :rocket: Anyway, I will upload later one temp version here for you to verify. Thanks again.

ryobg commented 6 years ago

FYI I'm in the middle of this. I have tackled few improvements even, but I think not to add them. So far the implementation is clear, I do some refactoring and documentation. Has yet to build it on my Windows machine plus adapt the tests. Then I want to see and some other functions which assume certain range of bits... Interestingly esl files do have 3 nibbles for index, but actually only two of them are relevant - want to cross check that again.

tasairis commented 6 years ago

No rush. And if you'd like, I can build and test when it's at the point where it's ready for that.

ryobg commented 6 years ago

I haven't had much time to work on, but small update: fixing the unit tests. So I hope this weekend to have something to upload here.

ryobg commented 6 years ago

Hi again, while I was implementing this feature I discovered one nasty issue and spent several days (!) to discover it. As consequence, I think I may have broken the binary archives compatability with previous versions. I see currently no other way around it though...

Anyway, I have uploaded here one intermediate release which, I believe, should support CC mods. Please, have a look.

tasairis commented 6 years ago

Unserialization isn't working, with or without CC mods. I'll see if I can spot why.

And yes, there was a

caught exception (class std::bad_alloc) during archive load - 'bad allocation'

~Actually hold on, now my tests say that 4.0.1 works fine with CC mods. Something's wrong.~ I forgot I had hacked myself a custom version with CC support. Nevermind.

tasairis commented 6 years ago

form_handling.h

 inline std::uint8_t mod_index (FormId n) 
 {
     auto u32 = static_cast<std::uint32_t> (n);
-    return static_cast<std::uint8_t> (is_light (n) ? u32 >> 24 : u32 >> 12);
+    return static_cast<std::uint8_t> (is_light (n) ? u32 >> 12 : u32 >> 24);
 }

But I don't think that explains it.

I'm going to download the branch and try messing around.

tasairis commented 6 years ago

The problem is with the use of string_view in form_handling.h's string_to_form().

string_view const mod = str.substr (0, mpos);
string_view const fid = str.substr (mpos + 1);

Eventually it ends in SKSE calling string_view.data(). However string_view doesn't have a null-terminated copy - mod.data() basically just returns the whole str string. JC will effectively form_from_file("mod.esp|0x123456", 0x123456).

As a workaround so I could continue testing,

-return form_from_file (mod, form);
+return form_from_file (string(mod.data(), mod.size()), form);

With that in place JC passes 4/6 tests: serializing and unserializing forms from regular mods with and without CC mods loaded. It isn't able to load forms from CC mods themselves.

Turns out that GetLoadedLightModIndex and ModInfo->modIndex give the regular load order for CC mods. 0xFE. Not the CC-relative load order. I'll try firing up a debugger and seeing what in ModInfo gives the relative index.

ryobg commented 6 years ago

Ok. Going to fix these and rework some tests. After that will check that GetLoadedLightModIndex...

P.S. Ok, I have fixed these. Now onto some new tests...

ryobg commented 6 years ago

As for that function, you say that GetLoadedLightModIndex returns 0xFE always? A bummer.

tasairis commented 6 years ago

Yeah, but I have good news: I know where the light mod index is. It's right after the modIndex in the ModInfo struct. In fact,

UInt8   modIndex;   // 478 init to 0xFF
UInt8   pad479;     // always zero? I suspect modIndex is actually UInt16
UInt16  lightModIndex;  // 47A 0x0000 for regular mods
UInt8   pad47C[4];

Haven't seen the 2.0.7 source but I bet the SKSE folks don't know about this.

My debugger-fu wasn't good enough but my intuition was. I tested using

std::optional<std::uint8_t> loaded_light_mod_index (std::string_view const& name) override
{
    auto mi = DataHandler::GetSingleton()->LookupLoadedLightModByName(name.data());
    Console_Print("%s = 0x%08X", name.data(), *(UInt32*)(&mi->modIndex));
    //auto ndx = DataHandler::GetSingleton ()->GetLoadedLightModIndex (name.data ());
    //return ndx != 0xFF ? std::make_optional (ndx) : std::nullopt;
    return mi->modIndex != 0xFF ? std::make_optional(mi->modIndex) : std::nullopt;
}

(and some Papyrus) for multiple ESL mods and each one showed 0xhhhh00FE, where hhhh=the relative load order. I tried 300 ESLs and at #256 it overflowed from 0x00FF00FE to 0x010000FE, so it is a short there.

ryobg commented 6 years ago

So basically you have split the char pad47C[7] member on two 16-bit members, first being the mod index, 2nd the light one? I do not see this in your example though? You still return modIndex which the original code does?

P.S. Wait, I just saw that you are using the 32-bit cast so that's why your endian is the other way around... Ok, ok. I think I get it....

tasairis commented 6 years ago

Ah, right. That ModInfo change was just me picking the easiest way to explain the memory layout. I don't actually have it updated like that locally. Thing is, this means the method should now return a uint16_t...

tasairis commented 6 years ago

So ideally,

std::optional<std::uint16_t> loaded_light_mod_index (std::string_view const& name) override
{
    auto mi = DataHandler::GetSingleton()->LookupLoadedLightModByName(name.data());
    return mi->modIndex != 0xFF ? std::make_optional(mi->lightModIndex) : std::nullopt;
}

(or in SKSE modify GetLoadedLightModIndex directly)

ryobg commented 6 years ago

Don't care. Your layour hack looks good enough. It make sense to inform SKSE about this, eventually adding this into the code. Just a second I'm doing it...

So, yes, this way we can enable more than 256 ESLs....

tasairis commented 6 years ago

Cool. I'm going to quickly update my copy and test, but that was the only issue I saw that would have prevented loading from ESLs.

tasairis commented 6 years ago

With this second hack

std::optional<std::uint8_t> loaded_light_mod_index (std::string_view const& name) override
{
    auto mi = DataHandler::GetSingleton()->LookupLoadedLightModByName(name.data());
    return mi->modIndex != 0xFF ? std::make_optional(*(uint8_t*)(&mi->modIndex + 2)) : std::nullopt;
}

JC passes all my tests 👍 Except for supporting more than 256 ESLs (it'll overflow) but that will hardly matter to anyone, and if it does eventually then SKSE could easily have been updated with proper support by then.

tasairis commented 6 years ago

You know, this is the only mod I know of to actually properly support ESLs. Thanks for all your work with this. Hopefully the 2.0.7 update will be easy :)

ryobg commented 6 years ago

Ok, I haven't tested well, but this may do, can you eyeball?

ryobg commented 6 years ago

I already checked few stuff from the new & old, I think it may be easy. Will start do that just right we close this 👍

ryobg commented 6 years ago

Support looks fine as reported in MR #19

ryobg commented 6 years ago

Btw @tasairis if you wish, you may report to the SKSE team about this finding.

tasairis commented 6 years ago

Sure, I can fire off an email at them.

ryobg commented 6 years ago

Was just going to publish, then it crossed my mind that we did not touch the other way around. Taking a mod name by form id. They will match against their total order in the CC list and not as defined by their index property.

tasairis commented 6 years ago

I check both unserialization (JString.decodeFormStringToForm) and serialization (JString.encodeFormToString), and I have a pair of those testing a form in an ESL in the middle of my load order. I think everything's working...

tasairis commented 6 years ago
    string esltest_esl = "empty - Copy (9).esl"
    int esltest_npc = 0xD61
    string esltest_ser = "__formData|empty - Copy (9).esl|0xD61"
    GlobalVariable esltest
    Form esltest_f
    GlobalVariable esltest_a
    string esltest_s

    esltest_f = Game.GetFormFromFile(esltest_npc, esltest_esl)
    Log("esltest is " + esltest_f)
    esltest = esltest_f as GlobalVariable
    if esltest
        esltest_f = JString.decodeFormStringToForm(esltest_ser)
        if esltest_f
            esltest_a = esltest_f as GlobalVariable
            if !esltest_a
                Log("ERR: unserialize esltest actor 1")
            elseif esltest_a.GetFormID() == esltest.GetFormID()
                esltest_s = JString.encodeFormToString(esltest_a)
                if esltest_s == esltest_ser
                    Log("OK: serialize esltest 1")
                else
                    Log("ERR: serialize esltest 1")
                endif
            else
                Log("ERR: unserialize esltest 1")
            endif
        else
            Log("ERR: unserialize esltest form 1")
        endif

        esltest_s = JString.encodeFormToString(esltest)
        if esltest_s == esltest_ser
            esltest_f = JString.decodeFormStringToForm(esltest_s)
            if esltest_f
                esltest_a = esltest_f as GlobalVariable
                if !esltest_a
                    Log("ERR: unserialize esltest actor 2")
                elseif esltest_a.GetFormID() == esltest.GetFormID()
                    Log("OK: unserialize esltest 2")
                else
                    Log("ERR: unserialize esltest 2")
                endif
            else
                Log("ERR: unserialize esltest form 2")
            endif
        else
            Log("ERR: serialize esltest 2")
        endif

        int i = 15
        while i <= 300
            if !(JString.decodeFormStringToForm("__formData|empty - Copy (" + i + ").esl|0xD61") as GlobalVariable)
                Log("ERR " + i)
            endif
            i += 15
        endwhile
    endif

The ESL was made in Creation Kit by loading Skyrim.esm and Update.esm, making a new Global (which CK put at 0xD61), saving normally (as an .esp), doing CK's convert to light master action, then literally copy/pasting the empty.esl file in Windows hundreds of times (I did 300 to test the UInt8 overflow) letting each one get a new name.

esltest is [Form <zzzGlobal (FE009D61)>]
OK: serialize esltest 1
OK: unserialize esltest 2
ryobg commented 6 years ago

I see. It crossed my mind that this list order is actually the mod index order - lol. Btw, mind trying https://github.com/ryobg/JContainers/releases/tag/v4.1.0 ? I haven't actually tried with the new runtime, as I need a bit more time to adjust my local installation...

tasairis commented 6 years ago

I got my installation set up so I can switch between 1.5.23/2.0.6 and 1.5.39/2.0.7 with only a minute or two of work. Took a while to get to that point but I don't regret it now.

Anyway, 4.1.0 seems to be working just fine 👍

ryobg commented 6 years ago

Cool, thanks 👍

RealAntithesis commented 6 years ago

Just to revisit this, I myself don't have any esl mods, but someone on the AH Hotkeys nexus page is saying that items from esl mods are showing up as invalid. Was the resolution of this issue that esl assets could be referenced properly or was it something else? Thanks.

ryobg commented 6 years ago

@RealAntithesis, should be working fine with or without ESL mods. I'm not aware of specific problem. Speculation: it may be that the JC version used is old or the VR one.

RealAntithesis commented 6 years ago

Received more feedback from the AH Hotkeys user re: inventory items in esl files. Full discussion is here : https://www.nexusmods.com/skyrimspecialedition/mods/15761/?tab=bugs&issue_id=384663#

Is there a possibility there could be a timing issue with loading esl assets? The user is saying that inventory items (forms) in esl files work when added through the MCM (which does a scan of inventory items at that stage - well after the save game has loaded) but the same forms don't work on a freshly loaded save game (but they show up again when rescanning the inventory in the MCM).

tasairis commented 6 years ago

I don't remember exactly how I tested the changes to JC but I was satisfied that it was able to locate all the ESL mods I tried. It finds mods via SKSE's list of mods, so the only way I see this failing would be if the list wasn't initialized (no idea when that's done) by the time AH needed it.

Looking at the pastebin'ed logs, the reporter is using JC 4.1. However I see that not just the ESLs' forms but the Steel Arrows in Skyrim.esm aren't working either. The original ESL problem would only have affected mods located at positions 05 and later - forms in the base ESMs were always fine.

Can the reporter try JC 4.0? The ESL forms won't work but the Steel Arrows should. Possibly multiple tries to establish consistency.

RealAntithesis commented 6 years ago

Just had clarified: the steel arrows didn't work in equipsets that had invalid forms. While somewhat ambiguous, I'm understanding from this that the steel arrows were still there, but didn't equip (which AH Hotkeys probably wouldn't get to cycle to them to equip if there were errors with previous forms in the list).

I'm thinking of getting this dlc to test it out myself.