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

Added support for mod defined window icon #19528

Open
wants to merge 1 commit into
base: bleed
Choose a base branch
from

Conversation

Mailaender
Copy link
Member

Closes #19289

@pchote
Copy link
Member

pchote commented Jul 16, 2021

Can you explain the benefit of having a separate IconManager class in Game, as opposed to having this managed by the IPlatformWindow? IMO it would be cleaner if IPlatform.CreateWindow simply took the icon and title string as arguments.

@pchote
Copy link
Member

pchote commented Jul 16, 2021

On closer inspection #19289 appears to be a bug in your OS's appimage integration, not with OpenRA.

The current release shows the mod's launcher icon in all the places I expect, except for the macOS compat build which we had accepted is not worth the trouble to fix. This includes Gnome on my Ubuntu system (in the top bar):

Screenshot 2021-07-16 at 20 39 20

While this is effectively a no-op for me on Linux and Windows, it actively regresses macOS (edit: Gnome too) by replacing the dock icon (which has a much larger size / higher resolution than 32px) with the low resolution pixellated version.

I see no problem with allowing mods to customize the window title, so can we please limit this PR to just that and close #19289 as not-an-openra-bug?

@Mailaender
Copy link
Member Author

Superseded by #19533. I want to keep this one for reference as the implementation itself seems correct.

@Mailaender Mailaender closed this Jul 16, 2021
@Mailaender Mailaender deleted the custom-icon-title branch July 16, 2021 21:01
@pchote
Copy link
Member

pchote commented Jul 16, 2021

Note that this also had a crash issue after switching from the content installer into the mod for the first time. Subsequent launches were ok.

@Mailaender
Copy link
Member Author

#19553 didn't solve the problem as promised, so I rebased this, adhering to the naming guidelines established in #19528 and made the SDL icon a Linux only feature due to the regressions on other platforms.

@Mailaender Mailaender changed the title Added support for mod defined application icon and title Added support for mod defined window icon Sep 26, 2021
@pchote
Copy link
Member

pchote commented Sep 26, 2021

I still don't support this PR because it is hacking around one small part of the problem while being fundamentally incapable of fixing all the rest.

If the problems of DE integration are important enough to solve then we should be focusing on building a system that shows a "Do you want to install system integration" prompt on first run that writes out the desktop file and icons to the user directories. This is the proper fix to these problems, without risking collateral damage.

@Mailaender
Copy link
Member Author

This is not about desktop integration. This is about the simplest thing an application should do: have an icon, and I don't think https://wiki.libsdl.org/SDL_SetWindowIcon is anywhere near a hack. It is an essential thing that has been missing for years.

@pchote
Copy link
Member

pchote commented Sep 26, 2021

The thing is that icon's aren't simple, and haven't been for many years. The creation of HiDPI displays and Wayland fundamentally changed the way that many of these legacy systems work. KDE and Gnome both rely on the XDG desktop / related specifications for properly handling icons now.

@abcdefg30
Copy link
Member

What is the status here?

@PunkPun
Copy link
Member

PunkPun commented Jun 12, 2023

What's the mainstream way of doing it? On mac I don't think I've seen an application have their icon at the window header.

I tried launching a bunch of apps, direct ports like Gimp, Blender, Inscape have an SDL/OpenRA-like empty header with just the window options and the app name. Spotify doesn't even include the name. Many of others either have the window options integrated into the UI (header doesn't exist) or fill the header with extra useful information / options

@Mailaender
Copy link
Member Author

What's the mainstream way of doing it?

On Linux, everything has a proper icon, even closed-source Electron stuff. OpenRA stands out as broken.

@pchote
Copy link
Member

pchote commented Jun 12, 2023

OpenRA does have proper icons though, so long as your desktop environment properly supports the XDG Desktop Entry spec and you have the .desktop file installed to an appropriate location.

@Mailaender
Copy link
Member Author

This covers the case where you don't have that, e.g. during development, startup and older desktop environments OpenHV/OpenHV#336 (comment).

@pchote
Copy link
Member

pchote commented Jun 12, 2023

I remain opposed to adding new mod.yaml definitions (creating confusion for modders) to control something that won't be seen by >99.9% of players.

I filed #20917 to help inform future discussions of this nature.

@pchote
Copy link
Member

pchote commented Jun 12, 2023

I'll also note that this doesn't work under Wayland (confirmed with #20914) as mentioned above.

@Mailaender
Copy link
Member Author

Where would you source SDL.SDL_SetWindowIcon instead if not from mod.yaml?

@Mailaender
Copy link
Member Author

Mailaender commented Jun 18, 2023

Presently, the task bar shows an icon but the window defaults to the X11 icon on released AppImages.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KDE window titlebar icon is not displayed
4 participants