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

.gltf importer warnings for unskinned meshes or invalid materials #414

Open
sleepybnuuy opened this issue Apr 25, 2024 · 3 comments
Open

Comments

@sleepybnuuy
Copy link

via https://discord.com/channels/884363610640498698/1231774132979630140/1231774132979630140, it'd be helpful to surface warning/error messages at time of import and in the Model-IO wiki doc around the following crash cases:

  • defining a mdl with invalid material paths (missing leading /)

    • while it's likely out of scope to warn/notify users of any paths to materials that don't exist in XIV, a missing slash in the material path can cause a crash against penumbra's ResolveMtrl hook when drawn, so warning or preventing these would be ideal. can often occur by accident through editing an FBX out of textools in any part of the modding process - TT fails to include the leading / and closing .mtrl that penumbra expects on each material name.
  • importing a gltf with unskinned submeshes

    • best practice notes for gltf importing should note that all submeshes must be parented to the n_root armature in a gltf export from blender (as well as linked to n_root via armature modifier) in order to have any weight/skinning data when imported into penumbra
    • if a mdl's submesh is missing BlendWeights and/or BlendIndices and is drawn, this can cause a game crash. a warning on each malformed mesh or error on import should help modders' troubleshooting
@ackwell
Copy link
Contributor

ackwell commented Apr 26, 2024

  • material path validation is already implemented on dev branch. it will validate that paths contain at least one /, but not it's position. i may add a .mtrl suffix check.
  • i won't be adding warnings about skinning, as there are many cases where a lack of skinning is valid. models exist outside character gear. i intend to surface vertex data so it is at least visible what data is in use.

at some point it may be possible to narrow these validations down further, but i'm more interested in catching the common cases in the short term.

@sleepybnuuy
Copy link
Author

i'm definitely of the mind that surfacing warnings around the most common use cases for model imports, especially for issues which can cause game crashes, would be expected by most users - even if those warnings would be unnecessary or intentionally ignored for some edge cases. having worked with other mod authors of all skill levels, gotchas like these won't exactly be common or intuitive knowledge for the userbase

could I take adding a post-import warning around unskinned meshes as an issue for a first PR, or are there any other QoL/ease of use changes already scoped that time would be better spent on? also happy to start compiling a troubleshooting wiki page if you'd prefer hands off at this dev stage

@ackwell
Copy link
Contributor

ackwell commented Apr 26, 2024

documenting expected data on the wiki page is a good start, it's much easier to write "character models will need skinning attributes" than it is to encode that sanely in the codebase 😓

i might kick ny to work out if there's more in-depth validation capabilities - i suspect it would be possible to preemptively determine if the vertex attributes in a mesh are accepted by the shader in its material - but this is a larger body of work to implement.

showing warnings for it on import would just be erroneous in cases. i don't mind raising warnings for things people may want to know but don't need to action, but i do mind raising warnings for things that are incorrect.

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

2 participants