pygame-ce icon indicating copy to clipboard operation
pygame-ce copied to clipboard

Fix passing `parent_window=None` to `message_box`

Open Matiiss opened this issue 1 year ago • 5 comments
trafficstars

Currently passing parent_window=None to message_box raises a TypeError. This should fix that. What exactly does the parent_window even do? The SDL docs don't really explain it either from what I could find. The only notable thing was that message_box should be called from the thread that the parent window was created in, unless it's None, in which case, it should be called from the main thread. Should that be added to the docs as well? Can that parent_window param in the docs be uncommented now? Should the test be an interactive one instead?

Matiiss avatar Feb 22 '24 23:02 Matiiss

Well, I guess the CI really doesn't like multiprocessing, going to move the test to the interactive tests section.

Matiiss avatar Feb 22 '24 23:02 Matiiss

What exactly does the parent_window even do? The SDL docs don't really explain it either from what I could find

Perhaps this should be a part of the research process for this PR?

Starbuck5 avatar Feb 26 '24 06:02 Starbuck5

What exactly does the parent_window even do? The SDL docs don't really explain it either from what I could find

Perhaps this should be a part of the research process for this PR?

Well, in my understanding all this does is sort out which thread the messagebox will be opened in. That is, if you may not wish to block your process running on the main thread, you could create a new window in a child thread and then pass that window as the parent to the message box function (that you also probably need to call from that thread). Otherwise even if you call it in a thread, but don't specify a parent window, it will block the main thread. That's what I understand from the docs

This function should be called on the thread that created the parent window, or on the main thread if the messagebox has no parent. It will block execution of that thread until the user clicks a button or closes the messagebox.

https://wiki.libsdl.org/SDL2/SDL_ShowSimpleMessageBox

There's a slight issue, unrelated to this PR. Something must be broken in the implementation because this does not work the way I have explained (and believe it should be the way it should work) with our implementation. It's beyond the scope of this PR though.

Matiiss avatar Mar 25 '24 12:03 Matiiss

What exactly does the parent_window even do? The SDL docs don't really explain it either from what I could find

Perhaps this should be a part of the research process for this PR?

As I understand it, setting the parent window of a modal dialog makes it so the window is greyed out until the modal dialog is closed/dealt with. It's not a thing the toolkit does (SDL, GTK, Qt), but something that must be actively supported by the desktop environment/window manager/compositor. The window manager might decide to always focus automatically on the dialog when you alt-tab to the parent window or click in the talk bar, to group the dialog together with the parent window in the dock/task bar, to grey out the parent window, and so on. If there is no parent window set, a tiling window manager might just show the dialog centered and on top of the tiling area. Otherwise it could show the window on top of the parent window.

Given that wayland does not allow windows to position themselves, this information is needed for the dialog to be spawned on top of the application and on the right monitor.

robertpfeiffer avatar Mar 27 '24 09:03 robertpfeiffer

The thing you should take away from my previous comment is it depends on whether you are on Win32, Quartz, X11, Wayland, or Android. Consoles that don't have a classic "window manager" might still have limited support for modal dialogs, the same way and Android and iOS do.

robertpfeiffer avatar Mar 27 '24 09:03 robertpfeiffer

This is what parent Window does on Windows (from SDL source):

/* If we have a parent window, get the Instance and HWND for them
       so that our little dialog gets exclusive focus at all times. */
    if (messageboxdata->window) {
        ParentWindow = messageboxdata->window->driverdata->hwnd;
    }

And from a Microsoft blog on the attribute:

The hwndParent field stores a handle to the parent window. This allows the resulting dialog to behave as a modal window and optionally lets you position the window relative to the parent.

Looking at the SDL code it looks like they don't touch the 'position the window relative to the parent' flag so unless it is on by default for this window type you would just get the modal effect from setting the parent window on Windows.

MyreMylar avatar May 24 '24 15:05 MyreMylar