Unable to assign Skeleton with Import Script

[scotmcp]

Tested versions

• Reproducible in all 4.x versions.

System information

Godot v4.3.rc3.mono - Linux Mint 22 (Wilma) - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4060 Ti (nvidia; 535.183.01) - AMD Ryzen 7 5700X 8-Core Processor (16 Threads)

Issue description

The ConfigFile class set_value method erases key/value pair when value is set to null (this is per class docs for ConfigFile.set_value). However, some files such as .glb.import files have required keys stored with explicit values for some options.

There is already the method erase_section() and erase_section_key(), and we need the ability to write null values to key/value pairs.

Example for when we need to be able to write an explicit null:

[params]

{_subresources={
"nodes": {
"PATH:Armature/Skeleton3D": {
"rest_pose/external_animation_library": null,
"retarget/bone_map": Resource("res://explosive_bone_map.tres")
}
}
}

If this value is not set, then the entire PATH:Armature/Skeleton3D node is over written by the engine on the next import. Thus an entry to set skeleton ex. {"nodes" : {"PATH:Armature/Skeleton3D": {"retarget/bone_map": Resource("res://bone_map.tres")}}} will also be erased and unset.

To justification that a change is needed:

If one copies and pastes a correct but partial entry, the editor will erase the incomplete entry upon next import...example:

_subresources={
"nodes": {
"PATH:Armature/Skeleton3D": {
"retarget/bone_map": Resource("res://explosive_bone_map.tres")
}
}
}

becomes: _subresources={}

If one copies and pastes the complete correct node entry with the required null value, the editor is fine. example:

_subresources={
"nodes": {
"PATH:Armature/Skeleton3D": {
"rest_pose/external_animation_library": null,
"retarget/bone_map": Resource("res://explosive_bone_map.tres")
}
}
}

Therefore one must conclude that the Class used to read and write configuration files such as .import files, should be able to write key/value pairs that include explicit null values.

There is already the method erase_section() and erase_section_key(), and we need the ability to write null values to key/value pairs.

Steps to reproduce

@tool # Needed so it runs in editor.
extends EditorScenePostImport

var config = ConfigFile.new()

func _post_import(scene):
	set_skeleton()

func set_skeleton() -> void:
	var bone_map : BoneMap = load("res://explosive_bone_map.tres")
	config.load("res://relax.glb.import")
	var rest_pose_node : Dictionary
	rest_pose_node = {"nodes" : {"PATH:Armature/Skeleton3D": {"rest_post/external_animation_library": null}}}
	config.set_value("params", "_subresources", rest_pose_node)
	config.save("res://relax.glb.import")

Minimal reproduction project (MRP)

Animation Test.zip

EDIT: I said FBX by mistake up above, I corrected that to GLB....I am not sure if that's important or not...I would expect the import file behavior to be the same for either.

[akien-mga]

The FBX code should be changed instead to accept a missing rest_pose/external_animation_library.

Changing the way ConfigFile behaves would break compat.

[fire]

@lyuma Could look into this.

[lyuma]

So there are two things going on here:

One is that there's a stray null in the .import file. I have a PR which should fix this, but the null doesn't do anything and so this is a purely cosmetic fix: I don't think this issue is a release blocker.

The second problem relates to the discussion from #95413, where I advised use of ConfigFile (typoed as ConfigMap) for modifying the .import file. The issue is that the config file likely will be overwritten after the import finishes, and hence it is not possible to modify the .import from within a PostImport script (or PostImportPlugin) in the way the MRP is doing.

Unfortunately I'm not aware of a clean way to do what that MRP is trying to do. In my code, I run a script which modifies the .import files specifically when an import is not currently happening, then trigger a reimport after. There's definitely a big API gap here.

I don't have a suggestion on how to fix the MRP to work correctly, but the closest thing I can think to suggest is run the code outside the import (perhaps in a call_deferred) and then run reimport_files on EditorFileSystem. Make sure to avoid an infinite loop, since the reimport_files will run the script again.

[scotmcp]

Thank you Lyuma.

yeah, ok....doing it outside of the import process kind of defeats the point. I was trying to automate all necessary configurations and modifications to an animation library during the import process because this stuff gets written to the res file in the .godot/import folder, the setting the bones up was the last piece of this particular puzzle to fully automate it.

EDIT: Another thought, I wonder if I could run a tool script that will set the import script up prior to import? Then I could just trigger an import and the skeleton could already be in place before that happens.

EDIT2: OK, that checks out ... I replicated that workflow manually by cutting the import script configuration item and the _subresources configuration with an external text editor, saving the file, then letting godot reimport and resetting those 2 settings back to default. Then, I pasted those lines back in to the file from the external text editor, saved the file, and triggered another import....Godot set the skeleton and ran the import script with intended results.

Very kludgy, but it works for now, so thanks

[lyuma]

doing it outside of the import process kind of defeats the point

Yeah I agree the usecase of changing settings during import makes sense. I'll try and see if there's a way we can improve the APIs in 4.4 to account for this. Though either way, EditorScenePostImport is too late because it literally is the last thing that runs.... so the way your script was written still would have to be changed.

In my opinion, EditorScenePostImportPlugin, and other such plugins that happen earlier in the import flow, would ideally have a way to adjust import options on the fly without doing a reimport. This is something I hope to make possible in 4.4

[scotmcp]

doing it outside of the import process kind of defeats the point

Yeah I agree the usecase of changing settings during import makes sense. I'll try and see if there's a way we can improve the APIs in 4.4 to account for this. Though either way, EditorScenePostImport is too late because it literally is the last thing that runs.... so the way your script was written still would have to be changed.

In my opinion, EditorScenePostImportPlugin, and other such plugins that happen earlier in the import flow, would ideally have a way to adjust import options on the fly without doing a reimport. This is something I hope to make possible in 4.4

Hi Lyuma, Have you had a chance to look into this? I don't see anything in the PRs that addresses it, but maybe I am just missing it. I am still hoping to fully automate this process so I am not doing it by hand 1600+ times.

Thanks, Scot

[lyuma]

This is something I hope to make possible in 4.4

Been fighting with stress and I haven't gotten anywhere near progressing on my ambitious priorities that I put down for 4.4 (I work on Godot as a volunteer).

If you want to increase the chance that I make this before the 4.4 feature freeze, could you propose some ideas for your dream API to change .import options during import? Like an extra function in post import plugin? Having some direction would help me focus.

[scotmcp]

This is something I hope to make possible in 4.4 Been fighting with stress and I haven't gotten anywhere near progressing on my ambitious priorities that I put down for 4.4 (I work on Godot as a volunteer). If you want to increase the chance that I make this before the 4.4 feature freeze, could you propose some ideas for your dream API to change .import options during import? Like an extra function in post import plugin? Having some direction would help me focus.

Lyuma, sorry you are dealing with stress. That really sucks, thank you for what you are able to accomplish. Being the lead dev (whether in name or action) on a section of the engine I am sure has a cost to you.

Really the most important thing to me is the ability to assign a skeleton from script. The way it is now, I have to manually import the asset (model or animation), assign the skeleton through the UI, reimport, and THEN assign the import script and reimport again.

Best case be ability to import through script only

BUT

It would already be much better if I could simply assign the skeleton within the script so I don't have so many manual operations per asset (i.e. import, assign script, reimport (3 ops) vs. import, open, assign skeleton, reimport, assign script, reimport (6 ops). That alone would cut the work of importing thousands of assets in half.

Thank you for all the hard work you do.

[lyuma]

From reading the Godot code, I did find out that you can actually call get_option_value() from the _pre_process method of EditorScenePostImportPlugin with the argument _subresources, and that will return a Dictionary which allows you to modify settings such as the BoneMap of the Armature/Skeleton3D node, without actually needing to modify .import before import. If it were better documented, I think this is pretty reasonable solution.

You do need to be careful of course when modifying such settings, and having the user go through this instead of having a convenient API will reduce the chance that someone does this unintentionally and makes a mistake, so I would be curious if this gets you unstuck.

That said, I came here to see if I could improve Godot's APIs.

So the two things I'm trying to decide between are:

1. To make the options dict mutable (the EditorFileSystem code actually writes the .import file afterwards, so there's no technical reason that it needs to be passed as const to the importers.

I do worry there is more of a stylistic reason not to do this: it feels a bit late to modify import options, while the file is already in the process of being imported, particularly the global options since many of them take effect before even the pre_process method of EditorScenePostImportPlugin. So making it too easy for users to accidentally edit the import options (if the options dictionary is mutable), could lead to inadvertent corruption of the .import settings. It also means we could get into conflicts between plugins since the order things get modified might matter. It just doesn't feel good to rush this into 4.4

2. Add a callback in EditorFileSystem before import, but after assigning the default project settings, so the import settings can be changed. The problem is, I don't think this helps your situation. I think the _pre_process is the perfect place to do this, since then you have access to the Node tree and you know which node is the skeleton and so on.

But if you want to set a setting like nodes/import_as_skeleton_bones (which is often useful when importing animation files, for example), that cannot be done from a post-import plugin, since that option is consumed by the GLTF import code. For this type of option, the ability to adjust import settings from EditorFileSystem might be helpful. The main problem I have here is it might imply building a whole plugin system, for a relatively niche usecase: the desired settings are not the default, but you only know the filename and nothing else, how would it know what settings to write to the .import file before the import begins, without knowing more context about the file.

Anyway, maybe I'm just making an excuse to not work on this API for 4.4. But I am really curious if the EditorScenePostImportPlugin idea I had with _subresources works for you, since that would solve most of the problems:

extends EditorScenePostImportPlugin

func _pre_process(scene: Node):
	subresources = get_option_value("_subresources")
	if not subresources.has("nodes"):
		subresources["nodes"] = {}

	var bone_map : BoneMap = load("res://explosive_bone_map.tres")

	# Possibly do some recursive search on scene to determine the actual Skeleton3D node. Sometimes it is not inside Armature.
	node_path = "Armature/Skeleton3D"
	if not subresources["nodes"].has("PATH:" + node_path):
		subresources["nodes"]["PATH:" + node_path] = {}
		# Set it in the if statement to allow the user to override after the first import.
		subresources["nodes"]["PATH:" + node_path]["retarget/bone_map"] = bone_map

Ooh I think I see the problem... resource_importer_scene.cpp pre-caches the nodes dictionary and fails if it was not assigned to begin with...

	Dictionary subresources = p_options["_subresources"];

	Dictionary node_data;
	if (subresources.has("nodes")) {
		node_data = subresources["nodes"];
	}

	Dictionary material_data;
	if (subresources.has("materials")) {
		material_data = subresources["materials"];
	}

	Dictionary animation_data;
	if (subresources.has("animations")) {
		animation_data = subresources["animations"];
	}

	Dictionary mesh_data;
	if (subresources.has("meshes")) {
		mesh_data = subresources["meshes"];
	}

I think we should simply move these if statements after the _pre_process function, and that's something I should be able to change for Godot 4.4

[scotmcp]

Hi Lyuma, Thanks for taking the time to look into this issue. I have run into a new issue which is the same problem as the skeleton assignment. If one needs to set an external pose, this also like the skeleton needs to be performed manually. I wonder if this import workflow just needs to be overhauled to make more sense and to allow full automation from beginning to end. I am not suggesting for for now, but maybe something to consider for next release ?!?

I will look into the suggestions you made, I was kinda waiting to see what might happen first. Give me a little bit of time though, because I am working on a new workflow and I am still in Blender, and not yet to the Godot Import phase with this assets.

Happy Holidays !!!

[scotmcp]

Hi Remi, I hope you didn't close this because I haven't answered yet, this has been a long conversation. I will test Llyuma's suggestion tonight if I can and post the results. I was trying to get into a space in my workflow where I wasn't breaking what has already been successfully imported through manual processes.

[lyuma]

@scotmcp it was closed automatically because my PR #100792 was merged which adds explicit support for assigning data into _subresources from an import script.

It actually worked before but only if "nodes" were present. My PR made it always work (an official advanced feature)

[SamDevelopsCode]

I am actually in need of almost the same thing as OP. I have read through most of this but I still have some questions:

1. Does @lyuma new PR allow us to modify things such as "rest_pose/load_pose", "rest_pose/external_animation_library", "retarget/bone_map"

on a fresh import using the EditorScenePostImport and _post_import() method?

If not, how do I actually use the EditorScenePostImportPlugin to assign these things?

Appreciate all the work being done on stuff like this!