Names patcher ignored removed FULL subrecord

[Arthmoor]

The names patcher is supposed to respect a mod tagged for Names that removed the FULL subrecord from the record it's in. This should not happen.

https://i.imgur.com/nsBs4MM.jpg

File attached to reproduce the issue.
NamesFULLError.zip

[Utumno]

Minimal patch options/Load order ?

[Utumno]

Oopsie - woppsie - this has dragon born as master - could you give one that doesn't ?

[Arthmoor]

As of 307.201610020028 this issue is still present.

[Utumno]

Minimal patch options/Load order ?

[Arthmoor]

Is there something wrong with the zip file provided back in May?

[Utumno]

It had dragonborn as master - got over that but I also need the minimal patch options.

In general please always provide the minimal load order and minimal patch options to reproduce the bug. I have to run the debugger to squash those and the time it takes grows exponentially with load order and patch options

[Arthmoor]

Well I'm rather frustrated at this point. The test case file I gave you for this in May doesn't want to reproduce the problem, but I'm still able to do so with USLEEP itself, so here you go:

00 Skyrim.esm
01 Update.esm
02 Dawnguard.esm
03 HearthFires.esm
04 Dragonborn.esm
05 Unofficial Skyrim Legendary Edition Patch.esp [Version 3.0.6]

Names patcher is the only thing being selected for this, with All files except Skyrim.esm selected for that.

That's the best I can do. If you want anything more detailed than this, it's not possible to provide it.

[Utumno]

I hope I will get round to this soon - just a last question though - I am still looking at "Lygrleid" ?

[Arthmoor]

Yes, still looking at Lygrleid.

[Infernio]

Can't reproduce with the linked plugin - that edit gets forwarded correctly - and USLEEP does not appear to change DLC2RRLygrleidWindhelm "Lygrleid" [NPC_:040376F2] anymore?

[Infernio]

Please comment / reopen if this can be reproduced again.

[Arthmoor]

I'm still able to reproduce this one with 307.201912131952.

[Infernio]

See my post here - I need plugins that reproduce this, since USLEEP no longer seems to touch that NPC.

[Arthmoor]

USLEEP (and USSEP too) both still have that record in them, producing exactly the same issue as originally posted.

I'll see about constructing a new set to reproduce this in SSE since that's where I just ran the patch on this version.

[Infernio]

Ah, I was looking at DLC2RRLygrleidWindhelm "Lygrleid" [NPC_:040376F2], but USSEP touches DLC2RRLygrleidSolstheim [NPC_:040376F5].
With a default patch config for vanilla SSE + USSEP, I can't reproduce it. So yes, a minimal reproducer for SSE would be appreciated :)

[Arthmoor]

I just ran a test on the file from the original post (after converting it for SSE) and I can still reproduce the result. The only difference I see is that in your patch config, Dragonborn.esm doesn't have the Names tag, where in my testing it does, and the Names tag is thus ignored for any mod touching the same thing after it, even if it too has the Names tag.

USSEP's change should be propagated to the patch because the removal of the FULL there is intentional, and Dragonborn.esm is the master file which is being altered.

[Utumno]

Thanks for cleaning issues up @Infernio - if you are to try and fix this please do so on the 312-patchers branch

[Infernio]

This seems to be intended, from parsers.FullNames.readFromMod:

full = record.full or (type_ == 'LIGH' and u'NO NAME')
if record.eid and full:
    id_name[longid] = (record.eid,full)

It specifically checks if the FULL subrecord is present and contains a non-empty string and only allows that if the record type is LIGH. Otherwise, such removed/empty names are ignored when scanning the source mods. Not sure what to do here, perhaps this is needed in Oblivion?

[Infernio]

Interesting thing I just noticed: the code above originally compared type != 'LIGH', it got changed to use == instead in cce2b19 (giant commit, so expand bosh.py and search for NO NAME). Neither commit messages nor the version history mention anything about this change, but I don't really see how it could have been an accident...

[Infernio]

Should be fixed now, I dropped the weird logic entirely. Let's see if something breaks :)

[Utumno]

thanks @Infernio! I am on a dumb smartphone and won't be of much help next couple of weeks I am afraid had some thought on the fids conversion problem - maybe add the master map as a property of the ModFile (aka ins) - if not None convert all FID elements on load_data (disclaimer: I can't look at any code)

…

On Thu, Aug 6, 2020, 02:48 Infernio ***@***.***> wrote: Closed #304 <#304> via 9697b64 <9697b64> . — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#304 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKNIV4IJYBUYP6WIAPAJU3R7HVVJANCNFSM4CD2DMQQ> .

[Infernio]

I pushed a commit to inf-312-tweak-pooling that absorbs the fid conversions entirely into load/save. I was afraid of slowdown, especially on startup, but it seems to have cost pretty much no performance? So maybe the impact isn't nearly as bad as we thought it might be?
Needed rewriting some ancient and ancient parts of the code (_saves.py and faces.py) though, so it's still high risk.