godot icon indicating copy to clipboard operation
godot copied to clipboard

When importing mesh objects as seperate files, gltf filename should not be included in the name (godot 3.3+, godot 4+)

Open joeyeroq opened this issue 3 years ago • 15 comments

Godot version

godot 3.4+, godot 4.0

System information

Windows 10

Issue description

When importing gltf files you can import materials, animations and meshes as separate files. The meshes get gltf filename and underscore as a prefix, and the name after the prefix is wrong because the name should be from mesh object, not mesh data object. If you have a lot of meshes it's just wrong and very distracting, especially if you have a long gltf file name. This is also inconsistent as for animations and materials the names are correct as they don't get an unnecessary prefix of the gltf filename + underscore.

Steps to reproduce

  1. import .gltf with separate objects
  2. look at names of resulting .mesh files

Minimal reproduction project

No response

joeyeroq avatar Jan 02 '22 20:01 joeyeroq

Related to https://github.com/godotengine/godot/issues/54922 and https://github.com/godot-extended-libraries/godot-ideas/discussions/26. The original issue should be fixed in 3.4.1/3.4.2 as the Use Legacy Names option works again.

Calinou avatar Jan 02 '22 22:01 Calinou

Related to #54922 and godot-extended-libraries/godot-ideas#26. The original issue should be fixed in 3.4.1/3.4.2 as the Use Legacy Names option works again.

Use Legacy Names does not work, I checked in godot 3.4.2 and does not exist in godot 4.0.

Edit: Here's an example, I usually name exported gltf files from blender "{godot_project}{blender_version}###.gltf" gltf_import_01 I also checked on 'Store In Subdir' to further illustrate the problem gltf_import_02 gltf_import_03 Notice the discrepancies of the mesh names in FileSystem tab and Scene tab. For example "suzanne_friendly" mesh is named "import-gltf-test-godot_bl-03_Torus.mesh", last part of the name is called "_Torus" because I joined a torus and Suzanne monkey. The mesh names should be the same like in previous godot versions.

gltf_import_04 In godot 3.2 the names are correct.

I would be content if there was a bulk rename option in FileSystem tab like in the Scene tab (shift+F2) to remedy this horrible design choice, but the gltf filename prefix and mesh data name is seriously unacceptable.

joeyeroq avatar Jan 02 '22 22:01 joeyeroq

CC @godotengine/import

akien-mga avatar Jan 06 '22 12:01 akien-mga

Godot 4.0 alpha 1, issue is still present. Please resolve this issue or at least defend why it is implemented this way.

joeyeroq avatar Jan 25 '22 12:01 joeyeroq

Please be patient, contributors will do what they can to help when they have time.

akien-mga avatar Jan 25 '22 18:01 akien-mga

#56312 also related to gltf import process.

JoanPotatoes2021 avatar Feb 03 '22 17:02 JoanPotatoes2021

After thinking about it and testing a few projects, the mesh data name is actually a better option than the object name for the separate mesh objects, because multiple mesh objects could have the same mesh data. The file prefix is still a bad idea imho.

joeyeroq avatar Mar 17 '22 20:03 joeyeroq

We need to design if we want to be globally unique or meshes that are the same name will be deduplicated.

  1. "import-gltf-test-godot_bl-03_Torus.mesh"
  2. "03_Torus.mesh"

fire avatar Apr 21 '22 14:04 fire

I would think it's best to leave as a open option, like a project settings or importer bool, mainly because every developer will organize their data differently, or we can use the logic of blender about data blocks and save the mesh name only, but then you would need to deal with name conflicts, which could be resolved through unique indexes but that is a whole different beast.

I guess this discussion is bigger than only importing meshs, we can metion reduz's proposal to separate animation of the models which is great, but will also face this same discussion, should the .gltf source name be included in the animation files?

Implement AnimationLibrary resource and respective importer https://github.com/godotengine/godot-proposals/issues/4296#issuecomment-1100979637,

Import scenes as AnimationLibrary https://github.com/godotengine/godot/pull/60177

I would want to implement the method C for my project, only saving the mesh name, possibly through import scripts if not built-in, I think it's the best of both worlds, and it's easy to organize your models and visualize all their parts, but that's just my opinion,

GodotIdea

JoanPotatoes2021 avatar Apr 21 '22 15:04 JoanPotatoes2021

Import of .blend file meshes is even more horrendous, it adds a dash, random 32 alphanumeric and underscore prefix on top of the unwanted blend file name prefix!!

Clipboard10

joeyeroq avatar Aug 24 '22 19:08 joeyeroq

Can you update your design with the fact that godot takes a .glb, converts it to a .scn and saves it inside of .godot/ folder. Then there's a .import file that does the remapping.

fire avatar Aug 24 '22 19:08 fire

For the record, I changed my approach and did another system to deal with this due to something I found later:

It was the fact that the skinning data doesn't belong to .mesh resources, but are inside MeshInstance3D, so if I wanted to swap body parts which would follow a specific rig, loading only the .mesh resources won't work aside from undeformable meshs like helmets, the easier way I found was to load the actual MeshInstance3D nodes from the .glb files.

So an autoload cycle through a spreedsheet data exported as a text file and creates a reference library using a dictionary and integer references, whenever I need a specific mesh, I pick it's ID from the spreedsheet accessing the autoload dictionary, and it load into the scene the MeshInstance3D from the .glb file using find_node() by it's object name exported through blender, the skin data is preserved and I only have to assign a skeleton later to work properly,

This however isn't great to create scenarios or anything you want to see in the editor, as I can only see the meshs when I run the game, but for dynamic objets like character bodyparts, helmets, weapons, it works.

I would consider the approach to expose more internals to allow users to create their own systems which they can understand and maintain the way they desire, which was what reduz wanted exposing various servers from the engine that now allow us to access deeper than we could in 3.x a couple of years back,

Every project will be different and every user will want something different, so trying to streamline the import process is a complex task, this discussion could also be applied to importing materials and animations, which have their own structures,

JoanPotatoes2021 avatar Aug 25 '22 14:08 JoanPotatoes2021

I'll note this here, as seems related; Aside from the prefix issue, there are some other hardcoded behaviours that get in the way if you try to do something different from the intended workflow. (Intended workflow being importing full scenes)

Preface: I have model files where there often is a single node (or Object in blender) named same as the filename, and created a custom import script to extract only meshes.

The issues:

1. GLTFDocument::_parse_scenes(...) makes some assumptions about the wanted structure. If the scene name is empty or begins with "Scene" (which is default in blender), it uses the file name instead:

https://github.com/godotengine/godot/blob/e14fa5532bd87addc8c019e2b3770758b36d0c09/modules/gltf/gltf_document.cpp#L570-L574

2. If the file has a node name (or any other name) identical as any other name, (like the scene name), the name gets a postfix number, as per the GLTFDocument::_gen_unique_name(...) function:

https://github.com/godotengine/godot/blob/e14fa5532bd87addc8c019e2b3770758b36d0c09/modules/gltf/gltf_document.cpp#L469-L479

Which makes sense because duplicate names are not allowed, and if you only want to import the .gltf file as a single scene (as then the root node name will be the filename, as opposed to "Scene").

But if you do not care about the full scene and instead you want to, for example, export a single node/mesh from a file, while keeping its original name, the node getting renamed because the scene name was "Scene" is (imo) unexpected.

Ideally the scene would remain as intact as possible between the .gltf file and godot. Maybe the function GLTFDocument::_parse_scenes(...) would rename the scene (eg. in the end root node) to something like <filename>_scene.

Of course that is very much a use case dependant problem, in the default full-scene workflow it makes sense to have the root node named by filename, as that is seen the most (and the renamed node is not usually important). Also this of course happens only if there is a node named same as the scene/filename.

PS: So right now, if you want to keep the node names consistent between blender & godot, you'd need to make sure that the scene name is set to something else than "Scene". (and even then the mesh names have the prefix, but for a custom import plugin you can get around it)

Okxa avatar Mar 06 '23 17:03 Okxa

Is this asking for a feature to move the hard coded behaviours to the end and then have a enum/bool to switch them?

I want to find a way of doing this that doesn't complicated the standard design of instancing only nodes. A packed scene is a tree of nodes when instantiated.

fire avatar Mar 06 '23 18:03 fire

Is this asking for a feature to move the hard coded behaviours to the end and then have a enum/bool to switch them?

I want to find a way of doing this that doesn't complicated the standard design of instancing only nodes. A packed scene is a tree of nodes when instantiated.

I'm thinking that the GFTFDocument class should output the .gltf file as close to as possible to the original file, eg. same names to make using it for custom imports/tasks easiers.

As for the normal import process, one idea I had was that the various functions to construct GLTFDocuments (eg. append_from_file(...)) could have options like "use Scene/File name as root node"/"do name deduplication". That would allow to use the GLTF libs more easily for custom tasks where you want to have the original names, but also when needed, have names that make more sense for instanced scenes. (Or possibly do the renaming elsewhere, outside of the GLTFDocument class.)

(The class still could do the deduplication if necessary, I do not know of the gltf spec enough if it itself allows duplicate names)

Okxa avatar Mar 07 '23 08:03 Okxa

After a few hours, I manage to find a temporary solution, which requires Writing and Importing a Script. For those who might try this approach, I'm not sure if this will work 100% efficiently on all cases, but has been working without issues for me. Please, to be cautious, first test on a empty project...

After importing the Script, select the path to the script inside the .glb file, and press Reimport: image

The Resources (Mesh Data) should then be generated: image

Btw, the Script that I wrote is in C#, and is the following:

`

#if TOOLS
using Godot;
[Tool]
public partial class ImportGLB : EditorScenePostImport 
{
	// -------------------------------------
	// Variables
	// -------------------------------------
    string folderPath;

	// -------------------------------------
	// Methods -> Standard
	// -------------------------------------
    public override GodotObject _PostImport(Node node) {

        if (node != null) {
            folderPath = GetSourceFile().GetBaseDir();
            IterateRecursively(node);
        }

        return base._PostImport(node);
    }

	// -------------------------------------
	// Methods -> Private
	// -------------------------------------
    void IterateRecursively(Node _node) {
        if (_node != null) {

            foreach(Node child in _node.GetChildren()) {
                if (child is MeshInstance3D _meshInstance3D) {
                    if (_meshInstance3D.Mesh != null) {
                    
                        if (ResourceLoader.Exists(folderPath + "/" + _meshInstance3D.Name + ".res")) {
                            ResourceSaver.Save(_meshInstance3D.Mesh, folderPath + "/" + _meshInstance3D.Name + ".res");
                            _meshInstance3D.Mesh.TakeOverPath(folderPath + "/" + _meshInstance3D.Name + ".res");
                        } else {
                            ResourceSaver.Save(_meshInstance3D.Mesh, folderPath + "/" + _meshInstance3D.Name + ".res", ResourceSaver.SaverFlags.ReplaceSubresourcePaths);
                        }

                    }
                }

                IterateRecursively(child);
            }
        }
    }
}
#endif

`

PS: After Overwriting an Existing Mesh, it requires Restarting the Scenes to Update the Meshes

Hope this gets fixed soon. Until then, for those who might stumble upon this issue, hope this temporary solution might help someone. Cheers! 😄

maridany1999 avatar Mar 30 '24 05:03 maridany1999