Fix import for unused animation slice information.

[GustJc]

Fixes #68936

Problem brief: The gltf importer would import 256 slice information for each animation if a single animation option was changed. Making the .import file unnecessarily big.

This makes the gltf importer only save slice data default values for slices created by the user.
This won't fix imported assets that have already been bloated. But it won't create bloat on new imported assets.

---

This is my first time with Godot source so I chose this simple approach first to see if it works. It did.

Maybe a better way to solve this is to avoid creating the 256 ImportOptions in the first place here

godot/editor/import/3d/resource_importer_scene.cpp

Lines 2028 to 2036 in 33c30b9

	for (int i = 0; i < 256; i++) {
	r_options->push_back(ImportOption(PropertyInfo(Variant::STRING, "slice_" + itos(i + 1) + "/name"), ""));
	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "slice_" + itos(i + 1) + "/start_frame"), 0));
	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "slice_" + itos(i + 1) + "/end_frame"), 0));
	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "slice_" + itos(i + 1) + "/loop_mode", PROPERTY_HINT_ENUM, "None,Linear,Pingpong"), 0));
	r_options->push_back(ImportOption(PropertyInfo(Variant::BOOL, "slice_" + itos(i + 1) + "/save_to_file/enabled", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_UPDATE_ALL_IF_MODIFIED), false));
	r_options->push_back(ImportOption(PropertyInfo(Variant::STRING, "slice_" + itos(i + 1) + "/save_to_file/path", PROPERTY_HINT_SAVE_FILE, ".res,*.tres"), ""));
	r_options->push_back(ImportOption(PropertyInfo(Variant::BOOL, "slice_" + itos(i + 1) + "/save_to_file/keep_custom_tracks"), false));
	}

This is created when the import settings window is loaded, and it will always create those 256 slice default data.
But as they are all the same anyways, maybe its better to just load something like "slice_data" and use a similar strategy, where I checked for the slice_amount to decide if the slice default values needs to be filled or not.
Inside both the post_fix_node and post_fix_animation where we have that value.

[lyuma]

Hmm I see what you're doing and it could work... basically it stops checking for slices (sets has_slices to false) at 1 + slice count.

It does feel a little hacky, and I would prefer if the underlying cause could somehow be addressed directly without having to make this loop kore complicated.

but honestly if it comes time for 4.4 or a 4.3 bugfix release and there isn't a better fix, I could go for this. I don't think it should cause any issues.

[GustJc]

It does feel a little hacky, and I would prefer if the underlying cause could somehow be addressed directly without having to make this loop kore complicated.

but honestly if it comes time for 4.4 or a 4.3 bugfix release and there isn't a better fix, I could go for this. I don't think it should cause any issues.

Glad to hear that. I'm still getting familiarized with Godot's code so I'm a bit unsure about bigger changes.
PS: I just got my clang-formater working and forced push the formatting changes.

---

I took a closer look how the import scene worked.
It uses those 256 default values to pass to its InspectorEditor with inspector->edit(scene_import_settings_data); at scene_import_settings.cpp:L952.
Which automatically creates the inspector for all the slice data values, and the resource_importer_scene.cpp uses their parameter paths and the slice_amount to check if they should be visible or not.
Essentially, the slice_data slots are all create only once when you select the animation node in the tree(at _select():L788).
And changing the slice_amount just changes its visibility.

To fix the root of this problem we'd need to change the behavior of creating all of the slice_data slots at once, and procedurally add or remove them only when the slice amount is changed.
But that's a big change for me right now, not really sure what breaks what yet.

I'll leave this PR open in case this can be accepted, but feel free to close it and implement a better solution if anyone wants to.

[lyuma]

I do like how short this approach is, and it does solve an issue. That said, I'm a bit concerned that it is technically a hack. But seeing as the code immediately before this also has a bunch of special casing for animation slices, I guess it's okay.

What do others think?

[jitspoe]

Seems less hacky than having hundreds of unset settings stored in the .import file for every animation. :)

It'd be nice to get this in sooner rather than later to avoid file bloat.