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

TrayBroker class for interaction with desktop services #60501

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

YakoYakoYokuYoku
Copy link

Both native file picker dialogs (godotengine/godot-proposals#1123) and notifications (godotengine/godot-proposals#1338) need to communicate with the desktop so they can be used inside the engine.
I found that doing the interaction in OS might be cumbersome so instead this PR proposes a new singleton tasked with doing communications with the desktop through the native methods (e.g. DBus for LinuxBSD and WinAPI for Windows platforms).
It is essentially a broker end for engine classes (hence the name TrayBroker) so these use an unified and reusable interface.

This was tested via implementing a Notification class and sending notifications from GDScript.

The LinuxBSD version of TrayBroker requires an Optional class for uninitialized values in the event poller.

@Calinou Calinou added this to the 4.0 milestone Apr 25, 2022
@YakoYakoYokuYoku YakoYakoYokuYoku marked this pull request as ready for review April 25, 2022 19:26
@YakoYakoYokuYoku YakoYakoYokuYoku requested review from a team as code owners April 25, 2022 19:26
@fire
Copy link
Member

fire commented Apr 25, 2022

Your pr has been approve for cicd testing. I'll see if I can get some time and ask if others would be interested in reviewing this. @RandomShaper and others.

@Calinou
Copy link
Member

Calinou commented May 5, 2022

We discussed this pull request in this week's proposal review meeting. We ended up agreeing that exposing a generic TrayBroker files may not be useful in most projects, and that only higher-level functionality (such as native file dialogs) should be made available to the user instead.

Either way, exposing native file dialogs as-is may also be problematic when specific permissions are required to read/write files (e.g. when macOS sandbox mode is enabled). Instead, it may be better to focus on adding an API to request generic permissions from the OS first. Feel free to open a proposal for that 🙂

@YakoYakoYokuYoku
Copy link
Author

We discussed this pull request in this week's proposal review meeting. We ended up agreeing that exposing a generic TrayBroker files may not be useful in most projects, and that only higher-level functionality (such as native file dialogs) should be made available to the user instead.

Either way, exposing native file dialogs as-is may also be problematic when specific permissions are required to read/write files (e.g. when macOS sandbox mode is enabled). Instead, it may be better to focus on adding an API to request generic permissions from the OS first. Feel free to open a proposal for that 🙂

Then how about TrayBroker is only internal to the engine, much like Joystick in Linux/BSD, as the reason of exposure of it was to perform the per platform constructor of the singleton (e.g. TrayBrokerDBus::create_dbus) in the ClassDB registration, though it seems that it could be placed on another much more appropriate part. Still, I'm sided with the idea of having an universal interface that has common functionalities internal to the engine, like explicitly requesting permissions to the user or registering background services. Anyways I'll write up the proposal.

@clayjohn clayjohn modified the milestones: 4.0, 4.x Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

4 participants