Godot built from the master branch crashes when pressing enter on the project manager screen

[Klemmbaustein]

Tested versions

This happens when building the engine from the master branch, and recent builds of other PRs (for example: #101552)
4.3 and 4.4-dev7 do not crash like this.

System information

Godot v4.4.beta (4ce466d) - Windows 10 (build 19045) - Multi-window, 1 monitor - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 Ti (NVIDIA; 32.0.15.6636) - AMD Ryzen 9 5950X 16-Core Processor (32 threads)

Issue description

The engine crashes with the following error message when pressing enter on the project manager screen if no project is selected:

ERROR: FATAL: Index p_index = 0 is out of bounds (size() = 0).
   at: CowData<struct ProjectList::Item>::get (.\core/templates/cowdata.h:219)

I've looked into this for myself, and it looks like It's crashing because it's trying to open the currently selected project, but it doesn't check if there are no selected projects.

(editor/project_manager.cpp line 1052)

(editor/project_manager.cpp line 615)

From my testing, adding a check to verify that project_list->get_selected_projects() returns at least one project fixes this crash and works fine. I have no idea why this is only happening in very new versions of the engine though, this code for this hasn't changed for months (EDIT: I misread dates in the git blame, it looks like #92563 caused this). Maybe something in the input code is not marking that input as handled properly, so it's treated as a shortcut incorrectly?

Steps to reproduce

Press enter on the project manager screen if no project is selected.

Minimal reproduction project (MRP)

N/A

[SpockBauru]

Just compiled the beta locally, and can confirm the crash when you just open Godot and press ENTER.

My system: Godot v4.4.beta (4ce466d7f) - Windows 11 (build 22631) - Multi-window, 1 monitor - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3080 (NVIDIA; 32.0.15.6614) - 12th Gen Intel(R) Core(TM) i7-12700KF (20 threads)

[akien-mga]

Confirmed, backtrace with debug symbols:

ERROR: FATAL: Index p_index = 0 is out of bounds (size() = 0).
   at: get (./core/templates/cowdata.h:219)

================================================================
handle_crash: Program crashed with signal 4
Engine version: Godot Engine v4.4.beta.custom_build (4ce466d7fa79e351d4295d5bb47e3266089c3a59)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib64/libc.so.6(+0x1a090) [0x7fa0ed33e090] (??:0)
[2] CowData<ProjectList::Item>::get(long) const (/home/akien/Godot/godot/./core/templates/cowdata.h:219 (discriminator 3))
[3] Vector<ProjectList::Item>::operator[](long) const (/home/akien/Godot/godot/./core/templates/vector.h:102)
[4] ProjectManager::_open_selected_projects_check_recovery_mode() (/home/akien/Godot/godot/./editor/project_manager.cpp:615 (discriminator 1))
[5] ProjectManager::_on_search_term_submitted(String const&) (/home/akien/Godot/godot/./editor/project_manager.cpp:834)
[6] void call_with_variant_args_helper<ProjectManager, String const&, 0ul>(ProjectManager*, void (ProjectManager::*)(String const&), Variant const**, Callable::CallError&, IndexSequence<0ul>) (/home/akien/Godot/godot/./core/variant/binder_common.h:315 (discriminator 2))
[7] void call_with_variant_args<ProjectManager, String const&>(ProjectManager*, void (ProjectManager::*)(String const&), Variant const**, int, Callable::CallError&) (/home/akien/Godot/godot/./core/variant/binder_common.h:430)
[8] CallableCustomMethodPointer<ProjectManager, void, String const&>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Godot/godot/./core/object/callable_method_pointer.h:109)
[9] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Godot/godot/./core/variant/callable.cpp:57)
[10] Object::emit_signalp(StringName const&, Variant const**, int) (/home/akien/Godot/godot/./core/object/object.cpp:1237)
[11] Node::emit_signalp(StringName const&, Variant const**, int) (/home/akien/Godot/godot/./scene/main/node.cpp:4021)
[12] Error Object::emit_signal<String>(StringName const&, String) (/home/akien/Godot/godot/./core/object/object.h:933)
[13] LineEdit::gui_input(Ref<InputEvent> const&) (/home/akien/Godot/godot/./scene/gui/line_edit.cpp:777 (discriminator 3))
[14] Control::_call_gui_input(Ref<InputEvent> const&) (/home/akien/Godot/godot/./scene/gui/control.cpp:1825)
[15] Viewport::_gui_input_event(Ref<InputEvent>) (/home/akien/Godot/godot/./scene/main/viewport.cpp:2166)
[16] Viewport::push_input(Ref<InputEvent> const&, bool) (/home/akien/Godot/godot/./scene/main/viewport.cpp:3273 (discriminator 2))
[17] Window::_window_input(Ref<InputEvent> const&) (/home/akien/Godot/godot/./scene/main/window.cpp:1694)
[18] void call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ul>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, Callable::CallError&, IndexSequence<0ul>) (/home/akien/Godot/godot/./core/variant/binder_common.h:315 (discriminator 2))
[19] void call_with_variant_args<Window, Ref<InputEvent> const&>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, int, Callable::CallError&) (/home/akien/Godot/godot/./core/variant/binder_common.h:430)
[20] CallableCustomMethodPointer<Window, void, Ref<InputEvent> const&>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Godot/godot/./core/object/callable_method_pointer.h:109)
[21] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Godot/godot/./core/variant/callable.cpp:57)
[22] Variant Callable::call<Ref<InputEvent> >(Ref<InputEvent>) const (/home/akien/Godot/godot/./core/variant/variant.h:952)
[23] DisplayServerX11::_dispatch_input_event(Ref<InputEvent> const&) (/home/akien/Godot/godot/platform/linuxbsd/x11/display_server_x11.cpp:4148 (discriminator 2))
[24] DisplayServerX11::_dispatch_input_events(Ref<InputEvent> const&) (/home/akien/Godot/godot/platform/linuxbsd/x11/display_server_x11.cpp:4125)
[25] Input::_parse_input_event_impl(Ref<InputEvent> const&, bool) (/home/akien/Godot/godot/./core/input/input.cpp:922)
[26] Input::flush_buffered_events() (/home/akien/Godot/godot/./core/input/input.cpp:1203)
[27] DisplayServerX11::process_events() (/home/akien/Godot/godot/platform/linuxbsd/x11/display_server_x11.cpp:5279)
[28] OS_LinuxBSD::run() (/home/akien/Godot/godot/platform/linuxbsd/os_linuxbsd.cpp:960)
[29] godot-git(main+0x14b) [0x6624031] (/home/akien/Godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:85)
[30] /lib64/libc.so.6(+0x3248) [0x7fa0ed327248] (??:0)
[31] /lib64/libc.so.6(__libc_start_main+0x8b) [0x7fa0ed32730b] (??:0)
[32] godot-git(_start+0x25) [0x6623e25] (??:?)
-- END OF BACKTRACE --
================================================================

Given ProjectManager::_open_selected_projects_check_recovery_mode(), this is likely related to #92563, CC @rsubtil.

[akien-mga]

There's two issues in the code that @Klemmbaustein linked to:

• It's missing a check that at least one project should be selected
• The function assumes a single project is selected, but the code it then calls handles multiple projects being selected

[mateuseap]

I've also been able to reproduce this error locally:

• System information: Godot v4.4.beta (4ce466d) - Windows 11 Pro (build 22631.4602) - AMD Radeon(TM) RX Vega 11 Graphics - AMD Ryzen 5 3400G (8 threads)

I’m currently working on fixing the ProjectManager::_open_selected_projects_check_recovery_mode() method to open a PR later. I've added a check to ensure that at least one project is selected. I've tested my changes by rebuilding the project and the crash no longer occurs. Here's my current code:

void ProjectManager::_open_selected_projects_check_recovery_mode() {
    Vector<ProjectList::Item> selected_projects = project_list->get_selected_projects();

    if (selected_projects.is_empty()) {
        return;
    }

    ProjectList::Item project = selected_projects[0];
    if (project.missing) {
        return;
    }

    ... // The rest of the code remains unchanged
}

Regarding the rest of the method, I’m unclear about the statement:

"The function assumes a single project is selected, but the code it then calls handles multiple projects being selected"

Could you clarify this further, @akien-mga? It's not clear to me yet. Specifically, I’d like to understand whether we should:

1. Adjust the logic to consider multiple projects being selected, as in the example below:

void ProjectManager::_open_selected_projects_check_recovery_mode() {
    ...

    // Check each selected project for recovery mode
    bool has_recovery_project = false;
    for (const ProjectList::Item& project : selected_projects) {
        if (project.missing) {
            continue;
        }

        // Check if any project failed to load during the last startup.
        if (project.recovery_mode) {
            has_recovery_project = true;
            break;
        }
    }

    if (has_recovery_project) {
        open_in_recovery_mode = false;
        _open_recovery_mode_ask(false);
        return;
    }

    _open_selected_projects_check_warnings();
}

OR

2. Ensure that the subsequent code explicitly handles only one project being selected, aligning with the current behavior of the function.

Clarifying this will help me decide the right approach to proceed. Thank you!

[akien-mga]

Regarding multi-selection, I'm not sure what the best approach should be to be honest. The project manager handling of this is pretty hacky, and there's already some functionality that only caters to the first selected projects, e.g. the project migration handling.

I think it's reasonable to expect that only one project should be opened when doing project recovery, but then we need to give proper feedback to the user that only the first project in their selection is being considered.

@rsubtil (who implemented the recovery mode) might need to chime in.

[rsubtil]

To be honest, I only learned that Godot supports opening multiple projects when I implemented this feature 😅

So, to give my two cents, I'd say that if a project is in a bad state that requires being opened in recovery mode, it is a temporary problem that warrants some concentration on each specific project in order to fix. So I'm more inclined to support opening recovery mode for individual projects only.

[mateuseap]

I've added a dialog to warn users which project is being considered for opening in recovery mode:

What do you think? Here's the code:

void ProjectManager::_open_selected_projects_check_recovery_mode() {
    Vector<ProjectList::Item> selected_projects = project_list->get_selected_projects();

    if (selected_projects.is_empty()) {
        return;
    }

    const ProjectList::Item &project = selected_projects[0];

    if (selected_projects.size() > 1) {
        open_in_recovery_mode_dialog->set_text(vformat(TTR("Only the first selected project will be considered for opening in recovery mode: %s"), project.path));
        open_in_recovery_mode_dialog->popup_centered(Size2i(400.0 * EDSCALE, 0));
    }

    if (project.missing) {
        return;
    }

    ... // The rest of the code remains unchanged
}

I can include this in my PR if it makes sense. CC @akien-mga @rsubtil

Edit:

Upon checking the console, I found the following error:

ERROR: Attempting to make child window exclusive, but the parent window already has another exclusive child. This window: /root/@ProjectManager@1015/@ConfirmationDialog@564, parent window: /root, current exclusive child window: /root/@ProjectManager@1015/@AcceptDialog@826
   at: Window::_set_transient_exclusive_child (scene\main\window.cpp:983)

I haven’t figured out how to fix this yet. Any ideias?