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

Minor graphical bug fixes for Linux. #1839

Merged
merged 4 commits into from
Jan 2, 2025
Merged

Conversation

Lymia
Copy link
Contributor

@Lymia Lymia commented Jan 1, 2025

This fixes a few small issues on Linux:

  • Issues with background windows showing through due to how transparency is handled on Linux.
  • Prevents flickering of the window on game startup due to resetting the graphics mode.

This also adds a flake.nix script to help build Commander Wars on NixOS/with Nix. I added this for my personal use, but no sense in not committing it.

@Lymia Lymia changed the title Minor grpahical bug fixes for Linux. Minor graphical bug fixes for Linux. Jan 1, 2025
@Robosturm
Copy link
Owner

I went through the request and I saw several changes which from my understanding are just a result of testing and toying around in order to narrow down the bug.
Mainly the changes in:

  • main.cpp
  • mainapp mainly the removal of screen size change
  • Stage.cpp

@Lymia
Copy link
Contributor Author

Lymia commented Jan 1, 2025

The changes in main.cpp and mainapp.cpp are meant to avoid a weird issue where on KDE, the window would open, play the animation for a window closing, then immediately open again. To my understanding, this is caused by the window being opened once then somehow reset by changeScreenMode during the case UpdateManager stage.

This moves the call to changeScreenMode to main.cpp (replacing the existing screen size change code) and removes the call from mainapp.cpp avoiding this issue. I can try to make videos of the before/after to demonstrate there.

The change in Stage.cpp was from when I was trying to determine the issue, but generally seems the more correct thing to do so I left it in. If that change isn't kept, then VideoDriver::begin should probably be deleted in general since it's otherwise unused.

@Robosturm
Copy link
Owner

  1. The changes in main.cpp remove important restore code and recovery code for settings. So I can't accept that.
  2. The addition of the setBrightness and setGamma in mainapp.cpp are also inaccetable. They are in a compiletime define which is turned off for certain devices and are done in main.cpp and 100% don't cause your issue
  3. The removal of the line changeScreenMode(getScreenMode()); seems to be fine. If this removes your issue than go for it.

@Robosturm
Copy link
Owner

My guess would be that in changeScreenMode:
The hide -> show code results in your sound......... and nothing happens if the window is hidden already.
I agree that changing the screenmode twice during startup isn't needed.

@Robosturm
Copy link
Owner

Okay your changes lead to immediatly crashes on my end.
I changed the code on the master so that it works.
Could you please see if it works for you as well?
If so remove all the changes so that only the flake files and the ignore list changes remain.
The main thing that's different is the surface format since the game uses opengl and not opengles --> If that leads to your behaviour the flag needs to be determined at runtime or compile time instead of beeing static in the windowbase cpp.

@Lymia
Copy link
Contributor Author

Lymia commented Jan 2, 2025

Does GLES not work on Windows? I set that as a general code cleanup thing, since that's the common denominator between all the platforms, since that's the set Commander Wars uses in practice. I'm going to have to see if I can get it compiling on Android to make sure I don't break anything, at the very least.

I'll put it on a minimum set of changes for now, and sort it out more when I can test stuff on more platforms.

@Robosturm
Copy link
Owner

Robosturm commented Jan 2, 2025

GLES crashes the game on my end at least.
As mentioned I've created a working version on my end.
I start a pre-release deployment. So android comes from the gitlab pipeline.
That's the pipeline in question:
https://github.com/Robosturm/Commander_Wars/actions/runs/12585419247

@Lymia
Copy link
Contributor Author

Lymia commented Jan 2, 2025

Tested on Linux: Not only does this work, but it clears the issue I was having earlier with it acting strangely on startup, despite me removing the change to remove the redundant mode switch. wtf.

@Lymia
Copy link
Contributor Author

Lymia commented Jan 2, 2025

Rebased and fixed. I'll investigate further later, probs not a thing for this PR.

@Robosturm Robosturm merged commit e09134d into Robosturm:master Jan 2, 2025
2 checks passed
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.

2 participants