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

Add to README.md missing Linux dep #1037

Closed
wants to merge 2 commits into from

Conversation

metaleap
Copy link
Contributor

@metaleap metaleap commented Jan 19, 2025

I was wondering why on my system, SDL_ShowSimpleMessageBox (and thus wi::helper::messageBox) was a total no-op returning -1. This had me perplexed since SDL's docs on the function state:

On X11, SDL rolls its own dialog box with X11 primitives instead of a formal toolkit like GTK+ or Qt.

Well, SDL_GetError informed me that "zenity failed to start" and a quick check affirmed that this wasn't already installed on my KDE system, zenity being a GNOME project (and apparently SDL2 won't try for kdialog if no zenity 😁 — either of which I wouldn't call "X11 primitives" but rather "external-program dependencies").

Well not all Linux users are on Ubuntu or are GNOME-using, so in the interest of completeness, this nano PR =)

It's kind of an optional rather than hard "dependency" this way, given that things keep working without message boxes — OTOH usually one does not want to miss them when they have sth to say.

README.md Outdated
@@ -49,7 +49,7 @@ If you have questions or stuck, please use the `windows` communication channel o
<img align="right" src="https://github.com/turanszkij/wickedengine-gifs/raw/main/character_grass.gif" width="320px"/>

#### Linux
To build the engine for Linux, use Cmake. You can find a sample build script for Ubuntu [here](.github/workflows/build.yml) (in the linux section). On the Linux operating system, you will need to ensure some additional dependencies are installed, such as Cmake (3.7 or newer), g++ compiler (C++ 17 compliant version) and SDL2. For Ubuntu 20.04, you can use the following commands to install dependencies:
To build the engine for Linux, use Cmake. You can find a sample build script for Ubuntu [here](.github/workflows/build.yml) (in the linux section). On the Linux operating system, you will need to ensure some additional dependencies are installed, such as Cmake (3.7 or newer), g++ compiler (C++ 17 compliant version), zenity and SDL2. For Ubuntu 20.04, you can use the following commands to install dependencies:
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you said, zenity isn't required. It only seems to be required on KDE, and even then it's not a hard dependency.

I would at least add a note on why zenity is required. It seemingly comes out of the blue. [Add "refuses to elaborate, leaves" meme here]

Copy link
Contributor Author

@metaleap metaleap Jan 19, 2025

Choose a reason for hiding this comment

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

OK tweaked. Even though it's not really that "optional", since every user realistically would like to not silently miss errors or warnings that Wicked wants to tell them. And it's not "only required on KDE" — rather, it's just not pre-installed on non-GNOME Linuxes, of which there are plenty out there and all with non-zero user bases. But whatev, I tweaked it, so there 😁

Copy link
Collaborator

@brakhane brakhane Jan 19, 2025

Choose a reason for hiding this comment

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

are you sure about the zenity requirement? It seems to me that it is required for dialog boxes, but not message boxes. And the messagebox I see is definitely neither GTK nor Zenity:

grafik

messageboxes seem to use raw X11 or wayland.

Edit: oh, the "implementation" for wayland is calling zenity. kek. That's pretty stupid, I agree

Copy link
Contributor Author

@metaleap metaleap Jan 19, 2025

Choose a reason for hiding this comment

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

are you sure about the zenity requirement?

My "surety" such as it is, merely comes from what SDL_GetError reported when my SDL_ShowSimpleMessageBox call returned -1. I was surprised myself.. given what they stated in their docs. But sometimes docs are wrong, sometimes error messages are wrong.. so, not-100%-sure truthfully, as I haven't taken the time to study in-depth the SDL2 codebase, but took that error message in faith and verified that installing zenity fixes it and now Wicked shows messageBoxes =) so let's just say, having it installed looks to be a reliable way not to run into the silent-absence-of-messageBoxes.

Edit: oh, the "implementation" for wayland is calling zenity. kek. That's pretty stupid, I agree

Even odder since I'm on X11 😁

As per @brakhane's suggestions
@turanszkij
Copy link
Owner

Is there no way to do message box for linux that doesn't require an other dependency?

@metaleap
Copy link
Contributor Author

metaleap commented Jan 20, 2025

Is there no way to do message box for linux that doesn't require an other dependency?

Nah, the kernel has no such syscalls on offer (and hence neither does libc) because any & all windowing systems, window managers, display managers and desktop shells are not its business but decidedly userland software =)

Reusing the SDL function is really the best choice here if we don't want to fiddle with somehow invoking "either X11 or Wayland" APIs directly, linking those libs & includes & all that, for a mere messagebox. (All it is, is SDL2 being apparently not as fully-capable in its programs-search as Wicked's own cross-platform-file-dialogs .h is.) And also, for our Linux users who are well used to the Linux openness=fragmentedness status quo, stating such "scattered extra packages you'll want" will not be a headscratcher for them or confusion or annoyance, but simply a welcome "completeness of system-requirements information".

@turanszkij
Copy link
Owner

I think this dependency should be removed from the readme, because it should be responsibility of the SDL function, and it should work on default linux and only error on some other systems (if I understand correctly). But if the messagebox didn't succeed, then it could be logged as an error.

@metaleap
Copy link
Contributor Author

it should work on default linux and only error on some other systems (if I understand correctly)

There is no one "default Linux" 😁 but yeah whatev 🕺

@metaleap metaleap closed this Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants