niftools / blender_niftools_addon

The Blender Niftools Addon is a Blender add-on to enable import and export of NetImmese File Formats including .nif, .kf, .egm.
http://www.niftools.org
Other
388 stars 104 forks source link

Incorrectly named vertex groups #261

Closed neomonkeus closed 4 years ago

neomonkeus commented 7 years ago

@niftools/blender-nif-plugin-reviewer

Issue Overview

When exporting armatures the Left vs Right versions of many bones (vertex groups) are named incorrectly

Version Information

NifTools 2.6.0 Dev 4 (from June 18th 2016),

with added changes from this commit to fix the armature export: https://github.com/niftools/blender_nif_plugin/pull/252/files

Blender Version Info

Blender 2.78c

Platform information

[Provide a brief overview of what OS your running on]

Context

When exporting armatures the Left vs Right versions of many bones (vertex groups) are named incorrectly (missing a space after the L/R). This causes CTD in-game. It can be fixed in NifSkope but is quite tedious since there are several dozen bones in most models.

Steps to Reproduce

[Ordered list of the steps required for recreating the issue, including settings]

.

Expected Result

Example: Vertex group in Blender: "NPC Thigh [Thg].L" Bone name in original Nif: "NPC L Thigh [LThg]" Bone name in exported Nif: "NPC LThigh [LThg]"

Actual Result

Sometimes the misnaming is more complicated. I've seen "NPC LThigh [LRThg]" note the extra R. But it doesn't usually happen so maybe there are some very specific circumstances to cause it.

Possible Fix

In file io_scene\nif_common.py In function get_bone_name_for_nif Replace: name = name.replace("NPC ", "NPC L") With: name = name.replace("NPC ", "NPC L ")

This should be on two lines (176 and 181 for me).

Screenshot

[If relevant, include a screenshot]

Logs and Files

[Provide logs file generated during the error as well as the blend and nif files are related to the issue]

Info Bar Output

[Output from the Info View, available at top of Blender viewport, drag to expand]

Console Output

[Set the logging level to 'Debug' and attach the output of the console. Enable via Window -> Toggle Console]

Blend File

[Attach the blend file if issue is reproducible]

Nif File

[Attach input or output files, samples of what the expected output should be and reproducing the issue]

Similar Known Issues

[Reference any known issues - https://github.com/niftools/blender_nif_plugin/issues]

Additional Information

I've also discovered another problem with the skeleton export. It seems that the Rotation is set to 0,0,-90 for every bone, which causes the armature to be terribly deformed in-game. Not sure if these problems were caused by the edits that I applied from those git commits, or if they're an actual issue in the latest version of the plugin. In any case the plugin is unusable for armatures without those edits so I can't take them out. I might try diving into the code but I feel like I will probably just get lost

Ghostwalker71 commented 7 years ago

Since this deals directly with code I added and the answer likely won't be obvious, I'll respond. The bone naming issue is related to the numerable ways that beth named their bones. Kormgar and I had a table made to handle it but I didn't manage to add it into the code. Kormgar did the lions share of the table so maybe he still has the files or would be willing to remake them. If I recall rightly there are 5 variations of NPC L in various bone names for creatures. and I believe the "NPC LThigh [LRThg]" is a valid bone but can't be certain. I think it is the Lower Thigh on the frostbite spider

opusGlass commented 7 years ago

Hi Ghostwalker,

It looks like some things got lost in translation when this bug report was opened. The main issue is actually the missing space between L and Thigh (LThigh vs L Thigh), which happens consistently for every left/right bone under these circumstances, not just that specific one. This bug is easily fixed with the code replacement I've suggested. Perhaps some meshes don't require that space, but all of the ones I've seen do.

The "NPC LThigh [LRThg]" was actually an additional issue that I was having trouble locating, but now I've figured it out. In the Frost Giant mesh, there is a bone named "NPC L Thigh [RThg]". During import, the R isn't removed because this is a left bone; then during export, an L gets prepended in front of the R, because that's what most left bones would require. I don't imagine there's an easy fix for this second bug, since "NPC L Thigh [LThg]" also exists and there would be no way to tell at the time of export which one was intended.

More detail on the original issue in this thread (4th comment): http://niftools.sourceforge.net/forum/viewtopic.php?f=21&t=6700

Ghostwalker71 commented 7 years ago

Yes I am aware, in the table that Kormgar and I put together we noted and set code to handle "NPC L" NPC L " NPC Left" "NPC LEFT" "NPC Left " "NPCLEFT " ETC. There were also dozens of variations for the brackets as well. I glaced back at some notes I still have and it appears there are more than 250 name variations for skyrim bones. Those are style variation and don't list the actual bone name differences, only the style of naming But I'm glad you have it under control and I do look forward to seeing a release.

CunningMoss6833 commented 7 years ago

Question: Is mirroring armatures important? If not, I would suggest removing this whole section of code. I thought this would be an issue easy enough for me to work on my python skills. However, after looking at a few nifs to see if I could find some sort of pattern to the bone naming I quickly realized there is absolutely no naming standard. The dwarven spider has a few different naming conventions within the same nif. The only way I can see that I can see to do this is with some sort of lookup table and that will only work for known bones.

opusGlass commented 7 years ago

I'm not sure what you mean by mirroring, and I'm out of my depth here. It is important to have separate left and right bones of course, if that's what you mean. I wonder if it's necessary to edit their names at all though, instead of keeping their names exactly as they are in the Nif to prevent any issues with conventions, but again I'm way out of my depth here so that's just speculation.

It's true that the bone naming conventions aren't consistent, as Ghost pointed out. The change that I suggested only makes this piece of code consistent with the primary code that is used when exporting a Nif. In my experience, the code here is only invoked when multiple Nif files have been imported into a single blend file. Because of a tiny difference in code between the export code used for single nifs vs that for multiple nifs, files made from multiple nifs have consistently failed to export for me. Changing this snippet of code to match the code that I found for exporting single nifs (as described in the report) fixes it for all the files I've tried.

On Tue, May 23, 2017 at 12:07 PM, CunningMoss6833 notifications@github.com wrote:

Question: Is mirroring armatures important? If not, I would suggest removing this whole section of code. I thought this would be an issue easy enough for me to work on my python skills. However, after looking at a few nifs to see if I could find some sort of pattern to the bone naming I quickly realized there is absolutely no naming standard. The dwarven spider has a few different naming conventions within the same nif. The only way I can see that I can see to do this is with some sort of lookup table and that will only work for known bones.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/niftools/blender_nif_plugin/issues/261#issuecomment-303484715, or mute the thread https://github.com/notifications/unsubscribe-auth/AZU_dBRlKV0U25KjXZOasrB_K44B2lx_ks5r8yBVgaJpZM4MigXV .

Ghostwalker71 commented 7 years ago

@opusGlass You misunderstand the purpose of this code. When importing the model into blender, the names need to be adjusted so that the mirror function in blender can be used effectively. So that a model only need to be edited on one side to have the resulting changed translate to the opposite side correctly. The way the Bethesda names are set up does not allow for a clean mirror edit and vertex weighting becomes confused and unusable. Also when setting the mirror the bone nodes become misnamed and do not get handled correctly on export. The code used to account for multiple imports simply adds a numeric value to the end of the name.

opusGlass commented 7 years ago

Ah, makes sense that you would rename them to try to get the mirror code working. My two cents is that mirroring almost never works for me, at least on the subset of models that I work with. Although I only work with creature models, which always have intentional left-right differences. I try to use mirroring every time and it works on probably less than 5% of them.

In any case, the missing space in the prefix string only occurs after importing multiple models. Because the code above is different from the single model export. Regardless of whether or not it works for every model, this fix causes multiple export to work on the same models that single export works on. Without this fix, multiple export produce consistent CTD for models that can be imported and exported individually without issue. I could be grossly misunderstanding some things, but this fix has worked 100% for me in practice.

On Tue, May 30, 2017 at 8:17 AM, Ghost notifications@github.com wrote:

@opusGlass https://github.com/opusglass You misunderstand the purpose of this code. When importing the model into blender, the names need to be adjusted so that the mirror function in blender can be used effectively. So that a model only need to be edited on one side to have the resulting changed translate to the opposite side correctly. The way the Bethesda names are set up does not allow for a clean mirror edit and vertex weighting becomes confused and unusable. Also when setting the mirror the bone nodes become misnamed and do not get handled correctly on export. The code used to account for multiple imports simply adds a numeric value to the end of the name.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/niftools/blender_nif_plugin/issues/261#issuecomment-304892350, or mute the thread https://github.com/notifications/unsubscribe-auth/AZU_dMQcnHNcbHgdUxp9pp4kHCtLeCGqks5r_CT8gaJpZM4MigXV .

shaidien commented 5 years ago

Well it did not fix anything. It changed lower class l and r that has in error been moved to end of name back to cap but they still are not right. It's been years sence skyrim had come out. Hardly any one works on it any more are we waiting tell skyrim 6 comes out tell you address skyrim nifs loading into blender. I guess I shouldn't be suprised you get what you pay for.

shaidien commented 5 years ago

Ok found problem haven't found solution yet. Problem is when ever bone had [anyinfo] brackets it messes up. Solution possibly add lines for it to understand [brackets]

neomonkeus commented 5 years ago

Well it did not fix anything. It changed lower class l and r that has in error been moved to end of name back to cap but they still are not right. It's been years sence skyrim had come out. Hardly any one works on it any more are we waiting tell skyrim 6 comes out tell you address skyrim nifs loading into blender. I guess I shouldn't be suprised you get what you pay for.

Indeed you are not paying for it, nor is any contributors getting paid. The observation doesn't contribute toward this issue or likely to encourage anyone. 👍

shaidien commented 5 years ago

I am trying to learn py to fix problem but it will take me a while. I am glad someone still uses this forum. I have a ? I believe I am on to why its breaking names just do not know how to address it yet. The only bone names that are getting messed up is the skyrim originals that have something like [the bones abbreviation] the custom bones that do not have that seem perfect. I looked at code in nif common py file. I do not see anything that addressed the brackets so I am currently teaching myself how to add something. O and even though NPC Spine [Spn] as well as any other bone names that even though they have the [brackets] but do not have L or R in name are also good. So something about them brackets and left or right is causing it so it should be as simple as adding the brackets into the left and right code. I am just a beginner so maybe I am off base you tell me.

CunningMoss6833 commented 5 years ago

Its been a few years since I've looked at the code so I might not be 100% accurate with my statement.

You need to take a look at a large sampling of NIFs to see the various ways Left and Right are specified. Not all Left and Right have a bracket in the designation. A good example of the complication is in the Skyrim Dwarven Spider in which there are several different designations for left and right in the same NIF. If I remember my investigation correctly, the various designations for left are: L, l, Left, left, [L, [l, and probably a few more. In the Dwarven Spider 'l' also stands for 'lower'. So, how do we know if 'l' stands for left and not something else? I am sure 'right' has some complications as well.

The other complication is reversing the process and getting the branch name back to the original when exporting. I had an idea of inserting a numerical code into the name to count the insertion point and form designation. (The Left designation is 7 characters into the name and it uses the form [ Left ].) The original complication remains of how do we figure out if 'L' stands for left or something else.

What is the cost/benefit of including this code? How many people are going to be modifying bone weights of existing NIFs and mirroring it verses other reasons for importing bone weights?

My opinion is that this section of code is not worth the effort and should be removed.

neomonkeus commented 4 years ago

@opusGlass Thanks for the contributions.