godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.01k stars 21.09k forks source link

MenuButton doesn't remove buttons properly if I force it it crashes #97327

Open MarkoGrbec opened 2 weeks ago

MarkoGrbec commented 2 weeks ago

Tested versions

Reproducible 4.4 dev2

System information

Linux manjaro

Issue description

either I don't understand sub Menus or it doesn't deallocate the sub menus

I expected to remove all submenus with clear()

Steps to reproduce

  1. click on 1 shape it works
  2. click on 2. shape (first it uses sub menu of the 1. shape)
  3. if you uncomment queue_free() it crashes on the 2. step

quick sample of code:

class_name PopUp extends MenuButton

func _ready() -> void:
    g_man.pop_up = self

var dict_name__sub_menus = {}

## multi_array [0] name of button [1] name of sub_menu [id] is number of sub menu
func pop_up(position, multi_array, call: Callable):
    var main_menu = get_popup()
    var sub_menus = dict_name__sub_menus.values()
    if sub_menus:
        for sub in sub_menus:
            for item in sub.id_pressed.get_connections():
                sub.id_pressed.disconnect(item.callable)

    ## TODO: uncomment and it'll crash after clicking 2 times
    #for subs in dict_name__sub_menus.values():
        #subs.queue_free()

    dict_name__sub_menus.clear()
    main_menu.clear()
    if multi_array:
        var id = -1
        for item in multi_array:
            var sub_menu = dict_name__sub_menus.get(item[1])
            if not sub_menu:
                id += 1
                sub_menu = PopupMenu.new()
                sub_menu.name = item[1]
                dict_name__sub_menus[item[1]] = sub_menu
                main_menu.add_child(sub_menu)
                main_menu.add_submenu_item(item[1], item[1])
                sub_menu.id_pressed.connect(
                    func(index_submenu):
                        call.call(id, index_submenu)
                )
            sub_menu.add_item(item[0])
    set_position(position)
    show_popup()

Minimal reproduction project (MRP)

MRP mrp-rpc.zip

file was too large even on minimal.

MRP without .godot

AThousandShips commented 2 weeks ago

file was too large even on minimal.

What are you including? This shouldn't be possible if all it contains is some code, did you exclude the .godot folder?

MarkoGrbec commented 2 weeks ago

Uhm no.

Wasnt that .godot has to be included?

AThousandShips commented 2 weeks ago

No it says:

Be sure to not include the .godot folder in the archive (but keep project.godot).

MarkoGrbec commented 2 weeks ago

ok I excluded only .godot this time now it's 4.2k :)

I've misread "not" include darn and too fast reading...

MarkoGrbec commented 2 weeks ago

ok looking deep in to documentation I've fixed my bug with:

main_menu.clear(true)

added "true" in clear

it also doesn't crash this time

but the crash still exists if there's sub menus that aren't deallocated and that I want to deallocate them with force (queue_free())

AThousandShips commented 2 weeks ago

Can confirm with the following crash trace:

================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.4.dev.custom_build (f606c73fa8c65326149b9a54a5dd62d6b93cdb5d)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] WorkerThreadPool::`RTTI Complete Object Locator'
[1] Window::get_contents_minimum_size (godot\scene\main\window.cpp:1958)
[2] Window::get_clamped_minimum_size (godot\scene\main\window.cpp:1967)
[3] Window::_update_window_size (godot\scene\main\window.cpp:1042)
[4] Window::set_size (godot\scene\main\window.cpp:393)
[5] Window::reset_size (godot\scene\main\window.cpp:404)
[6] PopupMenu::_activate_submenu (godot\scene\gui\popup_menu.cpp:348)
[7] PopupMenu::_input_from_window_internal (godot\scene\gui\popup_menu.cpp:630)
[8] PopupMenu::_input_from_window (godot\scene\gui\popup_menu.cpp:448)
[9] Window::_window_input (godot\scene\main\window.cpp:1675)
[10] Viewport::_sub_windows_forward_input (godot\scene\main\viewport.cpp:2898)
[11] Viewport::push_input (godot\scene\main\viewport.cpp:3159)
[12] Window::_window_input (godot\scene\main\window.cpp:1682)
[13] call_with_variant_args_helper<Window,Ref<InputEvent> const &,0> (godot\core\variant\binder_common.h:304)
[14] call_with_variant_args<Window,Ref<InputEvent> const &> (godot\core\variant\binder_common.h:419)
[15] CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call (godot\core\object\callable_method_pointer.h:104)
[16] Callable::callp (godot\core\variant\callable.cpp:58)
[17] Callable::call<Ref<InputEvent> > (godot\core\variant\variant.h:875)
[18] DisplayServerWindows::_dispatch_input_event (godot\platform\windows\display_server_windows.cpp:3733)
[19] DisplayServerWindows::_dispatch_input_events (godot\platform\windows\display_server_windows.cpp:3704)
[20] Input::_parse_input_event_impl (godot\core\input\input.cpp:803)
[21] Input::flush_buffered_events (godot\core\input\input.cpp:1084)
[22] DisplayServerWindows::process_events (godot\platform\windows\display_server_windows.cpp:3186)
[23] OS_Windows::run (godot\platform\windows\os_windows.cpp:1772)
[24] widechar_main (godot\platform\windows\godot_windows.cpp:181)
[25] _main (godot\platform\windows\godot_windows.cpp:206)
[26] main (godot\platform\windows\godot_windows.cpp:220)
[27] WinMain (godot\platform\windows\godot_windows.cpp:234)
[28] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[29] <couldn't map PC to fn name>
-- END OF BACKTRACE --
================================================================

But not sure what is exactly going wrong here or if the use is valid, but we should avoid crashing if possible

WhalesState commented 1 day ago

Note: it doesn't crash on 4.3.stable release.

Using await get_tree().process_frame also fixes the issue.

    ## TODO: uncomment and it'll crash after clicking 2 times
    for subs in dict_name__sub_menus.values():
        subs.queue_free()
    await get_tree().process_frame