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

Allow to load multiple animation/libraries at once in the animation manager #83503

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

YeldhamDev
Copy link
Member

Currently, you can only load animations and animation libraries one-by-one in the "Edit Animation Libraries" dialog. This PR allows the user to now pick multiple ones at once to load.

Copy link
Member

@SaracenOne SaracenOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, big UX improvement, LGTM

@SaracenOne SaracenOne self-requested a review October 19, 2023 13:37
Copy link
Member

@SaracenOne SaracenOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you implement the changes @AThousandShips requested please?

@KoBeWi
Copy link
Member

KoBeWi commented Nov 4, 2023

Some crash when loading multiple (empty) AnimationLibraries:

CrashHandlerException: Program crashed
Engine version: Godot Engine v4.2.beta.custom_build (5ee983188de97ae027f9b9c1443438063f708a7e)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] VectorWriteProxy<Variant>::operator[] (C:\godot_source\core\templates\vector.h:52)
[1] Array::operator[] (C:\godot_source\core\variant\array.cpp:90)
[2] AnimationLibraryEditor::_load_files (C:\godot_source\editor\plugins\animation_library_editor.cpp:364)
[3] call_with_variant_args_helper<AnimationLibraryEditor,Vector<String>,0> (C:\godot_source\core\variant\binder_common.h:308)
[4] call_with_variant_args<AnimationLibraryEditor,Vector<String> > (C:\godot_source\core\variant\binder_common.h:418)
[5] CallableCustomMethodPointer<AnimationLibraryEditor,Vector<String> >::call (C:\godot_source\core\object\callable_method_pointer.h:105)
[6] Callable::callp (C:\godot_source\core\variant\callable.cpp:58)
[7] Object::emit_signalp (C:\godot_source\core\object\object.cpp:1127)
[8] Node::emit_signalp (C:\godot_source\scene\main\node.cpp:3608)
[9] Object::emit_signal<Vector<String> > (C:\godot_source\core\object\object.h:920)
[10] EditorFileDialog::_action_pressed (C:\godot_source\editor\gui\editor_file_dialog.cpp:398)
[11] call_with_variant_args_helper<EditorFileDialog> (C:\godot_source\core\variant\binder_common.h:308)
[12] call_with_variant_args<EditorFileDialog> (C:\godot_source\core\variant\binder_common.h:418)
[13] CallableCustomMethodPointer<EditorFileDialog>::call (C:\godot_source\core\object\callable_method_pointer.h:105)
[14] Callable::callp (C:\godot_source\core\variant\callable.cpp:58)
[15] Object::emit_signalp (C:\godot_source\core\object\object.cpp:1127)
[16] Node::emit_signalp (C:\godot_source\scene\main\node.cpp:3608)
[17] Object::emit_signal<> (C:\godot_source\core\object\object.h:920)
[18] AcceptDialog::_ok_pressed (C:\godot_source\scene\gui\dialogs.cpp:121)
[19] call_with_variant_args_helper<AcceptDialog> (C:\godot_source\core\variant\binder_common.h:308)
[20] call_with_variant_args<AcceptDialog> (C:\godot_source\core\variant\binder_common.h:418)
[21] CallableCustomMethodPointer<AcceptDialog>::call (C:\godot_source\core\object\callable_method_pointer.h:105)
[22] Callable::callp (C:\godot_source\core\variant\callable.cpp:58)
[23] Object::emit_signalp (C:\godot_source\core\object\object.cpp:1127)
[24] Node::emit_signalp (C:\godot_source\scene\main\node.cpp:3608)
[25] Object::emit_signal<> (C:\godot_source\core\object\object.h:920)
[26] BaseButton::_pressed (C:\godot_source\scene\gui\base_button.cpp:139)
[27] BaseButton::on_action_event (C:\godot_source\scene\gui\base_button.cpp:177)
[28] BaseButton::gui_input (C:\godot_source\scene\gui\base_button.cpp:70)
[29] Control::_call_gui_input (C:\godot_source\scene\gui\control.cpp:1810)
[30] Viewport::_gui_call_input (C:\godot_source\scene\main\viewport.cpp:1596)
[31] Viewport::_gui_input_event (C:\godot_source\scene\main\viewport.cpp:1862)
[32] Viewport::push_input (C:\godot_source\scene\main\viewport.cpp:3186)
[33] Window::_window_input (C:\godot_source\scene\main\window.cpp:1553)
[34] call_with_variant_args_helper<Window,Ref<InputEvent> const &,0> (C:\godot_source\core\variant\binder_common.h:303)
[35] call_with_variant_args<Window,Ref<InputEvent> const &> (C:\godot_source\core\variant\binder_common.h:418)
[36] CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call (C:\godot_source\core\object\callable_method_pointer.h:105)
[37] Callable::callp (C:\godot_source\core\variant\callable.cpp:58)
[38] Callable::call<Ref<InputEvent> > (C:\godot_source\core\variant\variant.h:847)
[39] DisplayServerWindows::_dispatch_input_event (C:\godot_source\platform\windows\display_server_windows.cpp:2714)
[40] DisplayServerWindows::_dispatch_input_events (C:\godot_source\platform\windows\display_server_windows.cpp:2684)
[41] Input::_parse_input_event_impl (C:\godot_source\core\input\input.cpp:750)
[42] Input::flush_buffered_events (C:\godot_source\core\input\input.cpp:1012)
[43] DisplayServerWindows::process_events (C:\godot_source\platform\windows\display_server_windows.cpp:2399)
[44] OS_Windows::run (C:\godot_source\platform\windows\os_windows.cpp:1474)
[45] widechar_main (C:\godot_source\platform\windows\godot_windows.cpp:182)
[46] _main (C:\godot_source\platform\windows\godot_windows.cpp:204)
[47] main (C:\godot_source\platform\windows\godot_windows.cpp:218)
[48] WinMain (C:\godot_source\platform\windows\godot_windows.cpp:232)
[49] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[50] <couldn't map PC to fn name>
-- END OF BACKTRACE --

@YeldhamDev YeldhamDev force-pushed the grab_all_the_animations! branch from 513e5d3 to 7cd6f9c Compare November 4, 2023 14:47
@YeldhamDev
Copy link
Member Author

@SaracenOne @AThousandShips Changes made.

@KoBeWi How many libraries are you loading? I tried to reproduce the crash on my end, but no success.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 4, 2023

I used Create New Resource to make libraries (empty) and loaded them in a newly created AnimationPlayer.

@YeldhamDev
Copy link
Member Author

Welp, I loaded 10, and no crash.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 4, 2023

godot.windows.editor.dev.x86_64_9owj7UepNG.mp4

Seems like you need to add an animation before loading libraries.

@YeldhamDev YeldhamDev force-pushed the grab_all_the_animations! branch from 7cd6f9c to 24f7823 Compare November 6, 2023 15:36
@YeldhamDev
Copy link
Member Author

@KoBeWi So, I found the problem... I mixed up the i and j in the for loop. 😬

Copy link
Member

@SaracenOne SaracenOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, LGTM! 👍

@Illauriel
Copy link
Contributor

Oh hey! I've actually made a similar PR recently, because this one didn't pop up in the search as it doesn't mention Animation Library Editor.

It can probably be closed now, but there's also another small QOL improvement, (re)enabling the tree fold arrow in the same screen, because scrolling through hundreds of animations is a PITA. Can be cherrypicked from second commit. (it's one line)

@YeldhamDev
Copy link
Member Author

@Illauriel

Oh hey! I've actually made a similar PR recently, because this one didn't pop up in the search as it doesn't mention Animation Library Editor.

It happens, don't worry. However, in your PR, each undo/redo action is done separately, instead of being contained into a single one.

It can probably be closed now, but there's also another small QOL improvement, (re)enabling the tree fold arrow in the same screen, because scrolling through hundreds of animations is a PITA. Can be cherrypicked from second commit. (it's one line)

It would probably be easier to just remove the first commit and keep the second one in your PR instead.

@YeldhamDev YeldhamDev force-pushed the grab_all_the_animations! branch from 24f7823 to bb20c94 Compare November 7, 2023 02:23
@YeldhamDev YeldhamDev force-pushed the grab_all_the_animations! branch from bb20c94 to b61fe92 Compare November 7, 2023 13:08
@YeldhamDev
Copy link
Member Author

@AThousandShips Changes made.

Conflicts resolved.

@YeldhamDev YeldhamDev force-pushed the grab_all_the_animations! branch from b61fe92 to 3b249e3 Compare November 7, 2023 13:23
@YeldhamDev YeldhamDev force-pushed the grab_all_the_animations! branch from 3b249e3 to 0f37ee8 Compare November 7, 2023 14:01
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 4, 2024
@akien-mga akien-mga merged commit 52ab49e into godotengine:master Jan 4, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@Illauriel
Copy link
Contributor

Great news! LFG

@YeldhamDev YeldhamDev deleted the grab_all_the_animations! branch January 4, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants