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

Shader flags and BSLightingShaderProperty updates, and minor fixes. #469

Merged
merged 18 commits into from
Nov 3, 2021

Conversation

Candoran2
Copy link
Member

@Candoran2 Candoran2 commented Oct 22, 2021

@niftools/blender-niftools-addon-reviewer

Overview

  1. Normals are not exported when using a face tint.
  2. Added hair tint import/export.
  3. Fixed export warning for negative scales.
  4. Fixed long texture path search.
  5. Fixed BSLightingShaderProperty slot 6 export to agree with import.
  6. Changed texture export to not strip file path when not in textures folder when the file is not found, and work with relative file paths.
  7. Changed imported object name generation to agree with expected name from the skeleton root field.
  8. Fixed issue where export would error if the root object was a mesh object.
  9. Changed shader flag UI/transference to be dynamic, rather than using hardcoded keys.
  10. Changed use of is in comparison with string literals.
  11. Remove default=0 from collision_layer EnumProperty definition.
  12. Changed armature export: no longer sets pose to bind pose, and pose gets exported.
  13. Tangent space converter is not added when model_space_normal shader flag is present in nif.

Detailed Description

  1. Vanilla Skyrim meshes do not have normals when they have a face tint shader, just like with skin tints. I added face tints to the skin tint check.
  2. Hair tint was not imported due to wrong way of checking skin tint presence, and was not exported at all. This has been rectified to use the same checks that the hair (and skin) tint field does. As a useful side effect, imported meshes without skin or hair tint no longer become black.
  3. Negative scales were not detected, due to scales being calculated from matrix_local, rather than directly accessing the scale field. As a consequence, non-uniform scale (-10, 10, 10), for example, was not detected as non-uniform and no warning was issued to the user. This has now been fixed.
  4. Some bpy.path.resolve_ncase() calls took up to 20 seconds. This was due to os.path.exists taking a long time on file paths starting with \\\\ or //, which Blender uses to indicate file paths relative to the current file. This has been circumvented by converting any relative file paths to absolute paths for resolve_ncase, and converting it back when actually setting the field.
  5. The detail texture was exported to slot 6 on BSLightingShaderProperties, whereas import used decal_0. Export now also uses decal_0 (detail texture was already used by another slot).
  6. Some vanilla Skyrim meshes do not use the textures folder in their nif (even though the texture is in the texture folder). This means that if the textures aren't linked in Blender, they will retain the file path without textures, and export would strip them. This has been changed so that textures which have a non-linked file path do not get stripped if they are not in a 'textures' folder (under the assumption that when this happens, they are imported vanilla textures).
  7. Upon import of a skeleton root with no name, the corresponding Blender object would get given a name by Blender, but the new skeleton root field used 'noname'. This would lead to an error on export, because the 'noname' object didn't exist in the exported nif file and the skeleton root could not be assigned. Import now also uses 'noname'.
  8. With the new skeleton code, the assignment of the nif root was changed such that it would not happen if the root object was a mesh, leading to an error on export. The assignment now also happens when the root object is a mesh.
  9. Shader flag properties/ui tickboxes were all hardcoded. This has now been changed to dynamically get them from the associated bitflags, retaining existing functionality (and fixing a few misspelled/misnamed properties).
  10. The use of is with string comparison is likely not intended, can give wrong results (for the intended effect) and CPython complains about it.
  11. Blender 2.8 can't specify a default value for EnumProperties with a callback, leading to error upon trying to register collision_layer. Since it was default=0, it was not changing any behaviour and can be removed without issue. This was the cause of issue Error on activation of v0.0.10 #470 .
  12. The nif pose was already imported as a Blender pose, but was not yet exported back. This is now corrected. In order to do this, update_bind_position had to be changed, since that set the bind positions of the geometry data to the exported pose. This was previously not an issue, since the exported pose used to be the bind position in Blender. However, now it corresponds to Blender's pose position. In order to export the bind position and the pose position, I've simply reversed the matrix operations/conversions that the import does.
  13. Shaders using model space normals do not use the tangent space in-game/in nifskope. This change merely reflects that.

Fixes Known Issues

#470

Documentation

[Overview of updates to documentation]

Testing

[Overview of testing required to ensure functionality is correctly implemented]

Manual

[Set of steps to manually verify updates are working correctly]

Automated

[List of tests run, updated or added to avoid future regressions]

Additional Information

[Anything else you deem relevant]

… exported. Added condition to not export normals when using a face tint (as done in face meshes).
…tected. Fixed issue where finding texture paths would take a long time (~20s).
…Changed texture export to handle relative file paths, and ignore missing texture folder if no file found (i.e. path is likely directly from import).
…n a root node due to mismatch between object and skeleton root name generation.
@Candoran2 Candoran2 requested a review from HENDRIX-ZT2 October 25, 2021 18:51
@HENDRIX-ZT2
Copy link
Contributor

There's no real reason why we set the export to pose. The only change for the pose system to be functional is exporting the pose matrix from pose bones instead of edit bones. Geom's inv bind matrix has to stay edit bone matrix, obviously.

@neomonkeus
Copy link
Member

Looking good, do we need to do much further tests with the matrix updates?

@HENDRIX-ZT2
Copy link
Contributor

Looking good, do we need to do much further tests with the matrix updates?

If @Candoran2 has tested them we're good. Not sure if they want to implement the export of pose for nodes as well or do that in another PR.

@neomonkeus
Copy link
Member

Looking good, do we need to do much further tests with the matrix updates?

If @Candoran2 has tested them we're good. Not sure if they want to implement the export of pose for nodes as well or do that in another PR.

What would the impact be of not implementing before doing a release.
There is a fix for the current v0.0.10 that is breaking 2.92- but 2.93 is LTS so 50/50 as to priority.

@Candoran2
Copy link
Member Author

Looking good, do we need to do much further tests with the matrix updates?

If @Candoran2 has tested them we're good. Not sure if they want to implement the export of pose for nodes as well or do that in another PR.

What would the impact be of not implementing before doing a release. There is a fix for the current v0.0.10 that is breaking 2.92- but 2.93 is LTS so 50/50 as to priority.

That is now implemented, should be good to go.

@HENDRIX-ZT2 HENDRIX-ZT2 merged commit 92509dc into niftools:develop Nov 3, 2021
@neomonkeus neomonkeus mentioned this pull request Nov 7, 2021
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

Successfully merging this pull request may close these issues.

3 participants