Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WNAN from Custom Record Template missing on patched custom NPCs #80

Open
ghost opened this issue Jan 13, 2023 · 9 comments
Open

WNAN from Custom Record Template missing on patched custom NPCs #80

ghost opened this issue Jan 13, 2023 · 9 comments

Comments

@ghost
Copy link

ghost commented Jan 13, 2023

Hi Piranha, here's another one...

I'm testing your FA Dibellan asset pack that comes with a custom Record Template. So far so good.

However, while checking logs I noticed that almost all custom NPCs aren't being patched with the FemaleBodyDefaultDibellan WNAN record. The ones that do get patched like that are those who don't have a WNAN record by default, while those who already do have the record (majority of them) are patched in another way (they get the _patched0001 WNAN).

Maybe a bug? Or is it a compatibility workaround?

@Piranha91
Copy link
Collaborator

I’ll double check tonight when I can be at my computer but this sounds like the intended behavior. The patcher only uses record templates to fill in records or objects that the NPCs don’t already have. So if an NPC already has a worn armor, the patcher will make a copy of it rather than providing one from the record template. If I recall correctly, the record template for Dibellan has an extra Alternate Texture in the Worn Armor (or torso armature, I can’t remember) which is supposed to point to an _etc texture. As long as that’s getting copied into the patched WNAM, and its texture is getting assigned, everything is working correctly. If not, I’ll try to look tonight to see what I broke since it was released.

@ghost
Copy link
Author

ghost commented Jan 13, 2023

Ah yes, I've now found those records on SSEEdit. I can confirm that NPCs being patched with FemaleBodyDefaultDibellan do get the extra 3bbb_etc texture record, but about those patched with _patched0001, some do and most do not (!)... for example:

This follower (ESL) (https://www.nexusmods.com/skyrimspecialedition/mods/25112) was patched but did not get the extra texture entry on its torso armature

This one (https://www.nexusmods.com/skyrimspecialedition/mods/55712?tab=files&file_id=228952&nmm=1) had its torso armature patched correctly with the extra texture, but hands/feet textures were not patched

These two (https://www.nexusmods.com/skyrimspecialedition/mods/29852) were patched correctly, with extra texture.

Now, I wonder what kind of incompatibility we may run into... for example, what would happen if the patcher tries to assign Alternate Texture records to a custom NPC that already has one or two. This one (https://www.nexusmods.com/skyrimspecialedition/mods/77812?tab=files) for instance, have 3 Alternate Textures on its FemaleWorldModel entry.

@Piranha91
Copy link
Collaborator

Finally had some time to take a look..

Ahlalia: Originally was not getting patched at all for me due to my rule set in the BodyN category.

Fanny: Was not getting patched for me because the whole-config rules for dibellan prohibit Age40/50 NPCs.

Vorgha: Same as Ahlalia.

So, I edited the Dibellan config to remove these rule sets:

Ahlalia: You are correct - the new Alt texture set is not getting created in the ARMA record. I will investigate this.

Fanny: Everything seems to be getting patched correctly on my end. Got torso (with alt texture), hands, and feet all patched. Can you verify that this repeatable on your end?

Vorgha: same as Ahlalia.

So I've verified your observation that the extra texture set isn't being applied - will try to figure out if this is a bug with SynthEBD or this specific config file. I suspect it may have to do with how I defined the destination:

image

This means:

  • get the character's Worn Armor
  • from the worn armor, get the Armature that matches the conditions:
    • Body Template Flag is "Body" (e.g. the torso armature as opposed to hands/feet/whatever else the NPC may have)
    • The Patchable Races menu has the current NPC's race (this is an outdated thing that has since been replaced with the MatchRace command which just makes sure the armature race is the same race as the NPC, but in this case won't be an issue because the Patchable Races list should always contain Redguards and Orcs)
  • from the armature, get the World Model object
  • from the World Model object, get the Female World Model (this doesn't quite match what you see in SSEedit but it's right)
  • from the female world model, get the Alternate Textures list **
  • Within the Alternate Textures list, find the alternate texture whose name is "3BBB_etc" ***
  • From that texture, get the "New Texture" texture set
  • From the texture set, get the Diffuse path
  • From the Diffuse path, get the raw path from the data folder (again not the same as SSEedit but correct).

If at any point the patcher can't get x from the NPC, it should shift over to getting it from the record template. For most NPCs this should occur where I wrote ** (because they have no alternate torso arma textures). For some NPCs that do, it'll occur at *** (they won't have one named "3BBB_etc"). I'm not sure where this process is failing with Ahlalia and Vorgha, will be interesting to check it out.

One thing to note is that I actually expected Vorgha to get an "extra" alternate texture in addition to her three - in reality the already has the same texture that Dibellan is trying to apply (the second one) but because it's not named "3BBB_etc" in the list, the config file doesn't realize it's the same. One thing I could do is edit the config file a bit - intead of looking for the alternate texture named "3BBB_etc", I could have it look for the alternate texture whose NewTexture.Diffuse.RawPath is set to femalebody_etc_v2_1.dds. This would probably catch more NPCs, such as Vorgha, but it won't be 100% error-free because a mod author has complete freedom to rename that texture to anything, such as "MyNewTexture.dds". The only way to guarantee the config replaces the correct texture would be some sort of pixel-by-pixel comparison between that texture and the one referenced in the record template, which would both take forever and be well beyond the scope of the mod and probably my own coding abilities. But I could at least compare on the texture file name instead of the alt texture's name in the list, and that would hopefully catch most NPCs.

@ghost
Copy link
Author

ghost commented Jan 15, 2023

Ok... lot's of things to talk about then...

About the PatchableRaces, I was under the impression that for Custom Races to work, it was only needed to have a Race Alias set (alias set to actual patchable races). That's how I've set up all my custom races... does the PatchableRaces/MatchRace functions consider Race Alias?

About these alt textures from Female World Model, do you know which identifier is actually used to match the texture to the .nif? Is it the "3D Name" record or the "3D Index" record?

This is mere speculation on my end since I'm not familiar with the engineering behind skyrim's meshes/textures, but if the popular body mods out there generate .nifs that actually follow a pattern (as in 3D Name or 3D index always correlating to the same body parts), I think we can consider some approaches:

  1. Have the patcher consider the 3D Index instead of 3D Name (if 3D index is the one that is actually being used by the engine)
  2. Have SynthEBD ship with custom Record Templates with accurate alt text records for each body mod (3BA, BHUNP, etc)
  3. Have the patcher check the texture file name (as you suggested). However, I think this may cause conflicts with NPC mods that ships both femalebody_etc_1.dds and femalebody_etc_v2_1.dds. I don't know which is actually being used or what's the actual difference between them, but maybe this situation could cause problems for the patcher.

For the 2nd approach for example, maybe something like these test files:

Record Templates.zip

I've basically modified your Dibellan .esp to match the alt texture names I could find with nifskope (BSTriShape property), from a basic CBBE 3BA .nif generated with bodyslide.

Regarding Fanny NPC, my previous patch run did create a separate _Patched record for torso only, but for hands/feet the patcher created these same records as a record overwrite over the original ones (without the _Patched name) keeping the original npc texture paths. However, maybe that's probably because I ran the patcher on a zmerged esp which included said NPC (who knows what zmege messed up), or because I had only two body textures as enforced via specific npc assignments...
Anyway, I did try patching again the merged and non-merged original .esp, and patcher worked fine for both this time... 🤔 I'm clueless on this one.

@Piranha91
Copy link
Collaborator

Piranha91 commented Jan 15, 2023

does the PatchableRaces/MatchRace functions consider Race Alias?

PatchableRaces does consider Race Alias (as of a recent update) but it's a deprecated call anyway - I just forgot to update it in this config file. MatchRace works better, and doesn't check the Patchable Race list at all - it just checks to make sure the race matches between the NPC and its amature. For example, in that same config file, for the primary body textures, the destination looks like

WornArmor.Armature[BodyTemplate.FirstPersonFlags.Invoke:HasFlag(BipedObjectFlag.Body) && MatchRace(Race, AdditionalRaces, MatchDefault)].SkinTexture.Female.Diffuse.RawPath

MatchRace(Race, AdditionalRaces, MatchDefault)] is saying "check this object's Race and AdditionalRaces entries to see if they're the same as (or contain) the NPC's race (the "MatchDefault" at the end is an optional parameter that tells it to accept "DefaultRace" as a match). This function doesn't care if the NPC has a custom race or not; it's just looking for a match between the NPC and, in this case, its armature. I left the PatchableRaces.Contains call accessible for backwards compatibility but MatchRace is what should be used. For clarity, the reason it's necessary is that most followers ship with beast armature to make sure they don't turn invisible when transforming into vampires or werewolves:

image

If the Destination only had the BodyTemplate.FirstPersonFlags.Invoke:HasFlag(BipedObjectFlag.Body) component, then it would match the Torso armature, but it would also match the werewolf & other beast armatures:

image

In this case it would still patch the correct one because it comes first in the list, but there's no guarantee that would be the case for all NPCs. That's why the Destination includes the race check. Just FYI, bit of a tangent.

Regarding the alt textures:

  1. I don't know if the game uses the 3D name or index. Should probably find out; so far it just hasn't been relevant.
  2. I don't think we can expect any of the NPC mods out there to follow a consistent pattern. To some extent there may have to be hand-patching. Maybe once SynthEBD gets the full release, if enough people adopt it some consensus can be reached.
  3. I could potential include those record templates in the Record Templates folder, but it would still be on config file authors to select them as opposed to the default. I want to keep the default one the same because many users still don't use 3BA/3BBB/BHUNP, and I don't know what off-target effects there might be from the patcher trying to apply the etc texture to original CBBE/UNP bodies.

I think in the meantime the best solution would to just have the destination path accept "femalebody_etc_1.dds" OR "femalebody_etc_v2_1.dds". I'm unclear myself on what the difference is or why an NPC would use one over the other, but as long as it's what the patcher is looking for it shouldn't matter.

@ghost
Copy link
Author

ghost commented Jan 16, 2023

Thanks for the primer on those functions... I tried testing now with MatchRace instead of PatchableRaces, and things did improve a bit. Ahlalia has now been correctly patched with alt textures. Vorgha was patched, but patcher kept original alt textures records. However, I used that custom .esp I've created now, but behaviour should probably be the same with Dibellan's Record Template too.

And yeah, all things considered, your approach of checking both .dds files should probably be the best compromise for now indeed 👍

@Piranha91
Copy link
Collaborator

Check out the new Dibellan config I just uploaded - as discussed before it can't be perfect, but in my testing it actually worked nicely for an array of custom followers.

@ghost
Copy link
Author

ghost commented Jan 20, 2023

Yeah I'm testing Dibellan config with that new Destination path and its working, can't find any non-patched NPC now 👍

However, I think that approach only works when patching old CBBE bodies that only need one alttexture entry (which so far I haven't found any, I haven't been modding for long).
From what I could check, all 3BBB or 3BA bodies must have that same etc.dds assigned to two separate AlternateTexture entries. With Dibellan's destination path, only the first Alt Texture gets patched (like Vorgha NPC for example), which leads to blue missing texture problem on one of those body parts when using 3bbb or 3ba bodies as default in-game.

For 3BBB/3BA to work correctly, I tried making a new record template copying the dibellan one, but with two default alt textures instead of just one. Then on synthebd, tried these:

Creating two asset paths for etc.dds, both with this destination:

WornArmor.Armature[BodyTemplate.FirstPersonFlags.Invoke:HasFlag(BipedObjectFlag.Body) && MatchRace(Race, AdditionalRaces, MatchDefault)].WorldModel.Female.AlternateTextures[NewTexture.Diffuse.RawPath.Invoke:EndsWith("femalebody_etc_v2_1.dds", StringComparison.OrdinalIgnoreCase) || NewTexture.Diffuse.RawPath.Invoke:EndsWith("femalebody_etc_1.dds", StringComparison.OrdinalIgnoreCase)].NewTexture.Diffuse.RawPath

The result was a patcher crash right at the end with:

Failed to write new patch. Error: 
A referenced mod was not present on the load order being sorted against: Record Templates - 3BA.esp.  This mod was referenced by MajorRecord: 000B2B:SynthEBD.esp
   at Mutagen.Bethesda.Plugins.Binary.Translations.ModHeaderWriteLogic.SortMasters(List`1 modKeys)
   at Mutagen.Bethesda.Plugins.Binary.Translations.ModHeaderWriteLogic.ConstructWriteMasters(IModGetter mod)
   at Mutagen.Bethesda.Plugins.Binary.Translations.ModHeaderWriteLogic.PostProcessAdjustments(MutagenWriter writer, IModGetter mod, IModHeaderCommon modHeader)
   at Mutagen.Bethesda.Plugins.Binary.Translations.ModHeaderWriteLogic.WriteHeader(BinaryWriteParameters param, MutagenWriter writer, IModGetter mod, IModHeaderCommon modHeader, ModKey modKey)
   at Mutagen.Bethesda.Skyrim.SkyrimModBinaryWriteTranslation.Write(MutagenWriter writer, ISkyrimModGetter item, ModKey modKey, GroupMask importMask, BinaryWriteParameters param)
   at Mutagen.Bethesda.Skyrim.SkyrimModBinaryTranslationMixIn.WriteToBinary(ISkyrimModGetter item, FilePath path, BinaryWriteParameters param, GroupMask importMask, IFileSystem fileSystem)
   at Mutagen.Bethesda.Skyrim.SkyrimMod.Mutagen.Bethesda.Plugins.Records.IModGetter.WriteToBinary(FilePath path, BinaryWriteParameters param, IFileSystem fileSystem)
   at SynthEBD.PatcherIO.WritePatch(String patchOutputPath, ISkyrimMod outputMod, Logger logger, IEnvironmentStateProvider environmentProvider) in D:\a\SynthEBD\SynthEBD\SynthEBD\Patcher\PatcherIO\PatcherIO.cs:line 87

Looks like patcher gets confused while trying to assign same texture file to two different entries haha

Then I've tried creating two asset paths for etc.dds, each with different destination paths, one for each alt texture entry (sorting by Index instead of record name or .dds name, which IMO feels like better compatibility):

WornArmor.Armature[BodyTemplate.FirstPersonFlags.Invoke:HasFlag(BipedObjectFlag.Body) && MatchRace(Race, AdditionalRaces, MatchDefault)].WorldModel.Female.AlternateTextures[Index == 1].NewTexture.Diffuse.RawPath

WornArmor.Armature[BodyTemplate.FirstPersonFlags.Invoke:HasFlag(BipedObjectFlag.Body) && MatchRace(Race, AdditionalRaces, MatchDefault)].WorldModel.Female.AlternateTextures[Index == 2].NewTexture.Diffuse.RawPath

This time the patcher ran fine, both alt texture got the same .dds, and all npcs got patched, even those with custom alt textures already assigned (like Vorgha NPC).

I also did try simplifying it a bit with one asset path only and a one-liner like "Index == 2 || Index == 1", but patcher gets confused again and crashes 🤣

@Piranha91
Copy link
Collaborator

Interesting! I didn’t realize 3BA needs two textures - I guess I never looked closely enough to check ;). Thank you for investigating, and finding a solution. I’ll look in and see why the first condition causes a crash - it kind of looks similar to the crash that I fixed with the “array can have non-record objects” fix from 0.9.0. I wonder if I’m still missing an edge case.

Fortunately, SynthEBD config installers can link assets and record templates, so I can make one config file that uses the original record template .esp modeled on CBBE, and another modeled on 3BA, depending on what the user chooses during installation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant