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

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

Open
joeyeroq opened this issue Jan 2, 2022 · 16 comments

Comments

@joeyeroq
Copy link

joeyeroq commented Jan 2, 2022

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

@Calinou
Copy link
Member

Calinou commented Jan 2, 2022

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.

@joeyeroq
Copy link
Author

joeyeroq commented Jan 2, 2022

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.

@akien-mga
Copy link
Member

CC @godotengine/import

@joeyeroq
Copy link
Author

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

@akien-mga
Copy link
Member

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

@JoanPotatoes2021
Copy link

#56312 also related to gltf import process.

@joeyeroq
Copy link
Author

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.

@fire
Copy link
Member

fire commented Apr 21, 2022

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"

@JoanPotatoes2021
Copy link

JoanPotatoes2021 commented Apr 21, 2022

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
godotengine/godot-proposals#4296 (comment),

Import scenes as AnimationLibrary
#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

@joeyeroq
Copy link
Author

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

@fire
Copy link
Member

fire commented Aug 24, 2022

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.

@JoanPotatoes2021
Copy link

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,

@lyuma lyuma modified the milestones: 4.0, 4.x Feb 22, 2023
@Okxa
Copy link

Okxa commented Mar 6, 2023

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:

if (s.has("name") && !String(s["name"]).is_empty() && !((String)s["name"]).begins_with("Scene")) {
p_state->scene_name = _gen_unique_name(p_state, s["name"]);
} else {
p_state->scene_name = _gen_unique_name(p_state, p_state->filename);
}

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:

while (true) {
u_name = s_name;
if (index > 1) {
u_name += itos(index);
}
if (!p_state->unique_names.has(u_name)) {
break;
}
index++;
}

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)

@fire
Copy link
Member

fire commented Mar 6, 2023

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.

@Okxa
Copy link

Okxa commented Mar 7, 2023

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)

@maridany1999
Copy link

maridany1999 commented Mar 30, 2024

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! 😄

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

No branches or pull requests

8 participants