FBX import settings lost due to ConfigFile erasing keys when value set to null

[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