Remove default skeleton path in MeshInstance3D#112267
Remove default skeleton path in MeshInstance3D#112267akien-mga merged 1 commit intogodotengine:masterfrom
Conversation
aaronfranke
left a comment
aaronfranke
left a comment
There was a problem hiding this comment.
I am in favor of this. Personally I prefer a hard break like in PR #107526, but either way works.
|
I suppose it already is a "hard break" as this is off by default. The setting really exists "just in case". |
akien-mga
left a comment
akien-mga
left a comment
There was a problem hiding this comment.
Looks good to me.
@lawnjelly You might want to backport this to 3.7 to fix #63210.
|
Thanks! |
|
This completely broke all of our official demos that use skeletons. I suspect this is going to be a much more painful and widespread compat breakage than expected. Given that there wasnt even a bug report for this issue we may want to re-evaluate if merging this was the correct decision. |
|
Perhaps a safe alternative would be for new projects to set this to |
There are two bug reports for this issue linked in the OP. I've also seen personally a number of examples of this issue on social media which users seem to not even bother reporting, so it seems to be a widespread papercut that users internalized as background noise.
Yeah this would be a better approach to only change the behavior for 4.6+ projects. I believe we might have some facility to do this already since #105737. |
|
As someone who ran into this, yes it was an annoying UX papercut |
Here is code that should do that (untested): diff --git a/core/config/project_settings.cpp b/core/config/project_settings.cpp
index 6b865fd470..5ce32f4d60 100644
--- a/core/config/project_settings.cpp
+++ b/core/config/project_settings.cpp
@@ -1687,7 +1687,7 @@ ProjectSettings::ProjectSettings() {
GLOBAL_DEF("animation/warnings/check_invalid_track_paths", true);
GLOBAL_DEF("animation/warnings/check_angle_interpolation_type_conflicting", true);
#ifndef DISABLE_DEPRECATED
- GLOBAL_DEF_RST("animation/compatibility/default_parent_skeleton_in_mesh_instance_3d", false);
+ GLOBAL_DEF_RST("animation/compatibility/default_parent_skeleton_in_mesh_instance_3d", true);
#endif
GLOBAL_DEF_BASIC(PropertyInfo(Variant::STRING, "audio/buses/default_bus_layout", PROPERTY_HINT_FILE, "*.tres"), "res://default_bus_layout.tres");
diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml
index a8078b2a4f..5af0371ccd 100644
--- a/doc/classes/ProjectSettings.xml
+++ b/doc/classes/ProjectSettings.xml
@@ -282,8 +282,8 @@
<member name="accessibility/general/updates_per_second" type="int" setter="" getter="" default="60">
The number of accessibility information updates per second.
</member>
- <member name="animation/compatibility/default_parent_skeleton_in_mesh_instance_3d" type="bool" setter="" getter="" default="false">
- If [code]true[/code], [member MeshInstance3D.skeleton] will point to the parent node ([code]..[/code]) by default, which was the behavior before Godot 4.6. It's recommended to keep this setting disabled unless the old behavior is needed for compatibility.
+ <member name="animation/compatibility/default_parent_skeleton_in_mesh_instance_3d" type="bool" setter="" getter="" default="true">
+ If [code]true[/code], [member MeshInstance3D.skeleton] will point to the parent node ([code]..[/code]) by default, which was the behavior before Godot 4.6. This setting is enabled by default for compatibility with pre-existing projects, but newly created projects after Godot 4.6 have it set to false.
</member>
<member name="animation/warnings/check_angle_interpolation_type_conflicting" type="bool" setter="" getter="" default="true">
If [code]true[/code], [AnimationMixer] prints the warning of interpolation being forced to choose the shortest rotation path due to multiple angle interpolation types being mixed in the [AnimationMixer] cache.
diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp
index 16350ad101..64966275ca 100644
--- a/editor/editor_node.cpp
+++ b/editor/editor_node.cpp
@@ -7784,6 +7784,9 @@ void EditorNode::notify_settings_overrides_changed() {
HashMap<String, Variant> EditorNode::get_initial_settings() {
HashMap<String, Variant> settings;
settings["physics/3d/physics_engine"] = "Jolt Physics";
+#ifndef DISABLE_DEPRECATED
+ settings["animation/compatibility/default_parent_skeleton_in_mesh_instance_3d"] = "false";
+#endif
return settings;
}The downside however is that all new projects until Godot 5.0 will have My preference would be to have an automated conversion of scenes when loading projects in 4.6 to set any unsaved So let's revert for now and work on a better approach instead of patching the hole further. |
This reverts commit d27fb9b. This change is still wanted, but the compatibility handling needs to be improved to avoid disrupting existing projets relying on that default skeleton path. See discussion in godotengine#112267.
8 participants
Currently MeshInstance3D's
skeletondefaults to parent node (..), which results in assigning unrelated nodes as skeleton and causes many problems.Edit by @akien-mga:
MeshInstance3D's auto-initializedskeleton_pathproperty leads to confusing errors when no skeleton is desired #101206See that PR for discussion about the problem and mitigation of the compat breakage. We came to this solution which fixes the issue by default going forward, but lets users who need it to enable compatibility mode with a project setting, if they relied on the previous behavior.