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

settings: made monitors selectable in fullscreen modes #17648

Conversation

@everclear72216
Copy link
Contributor

everclear72216 commented Feb 5, 2020

An integer value has been added to the graphics settings and is
pushed though the Platform interface to the Sdl2PlatformWindow.
The integer represents the display index used by SDL to identify
a display device.

The platform window hinterface has been extended to provide a
method which can be used to retrieve the available monitors.
The renderer class has been extended to give access to this
information within the settings logic class.

In the display tab of the settings menu the monitor can be
selected via a dropdown iff a non-windowed mode is selected.

"restart necessary"-logic has been updated to cater for the new
configuration option which can only be applied after restart.

The UI has been adaptet for mods/common and mods/cnc

closes #12363

Thank you for your contribution to OpenRA!

Please be aware that we do not have enough project maintainers to match the rate of contributions, so it may take several days before somebody is able to respond to your Pull Request.

You can help speed up the review process by following a few steps:

  • Make sure that you have read and understand the OpenRA Coding Standard (see https://github.com/OpenRA/OpenRA/wiki/Coding-Standard).
  • Write quality commit messages (see https://chris.beams.io/posts/git-commit/).
  • Only commit changes that directly relate to your Pull Request. Use your Git interface to unstage any unrelated changes to project files, line endings, whitespace, or other files.
  • Review the code diff view below to double check that your changes satisfy the above three points.
  • Use the make test and make check commands to check for (and fix!) any issues that are reported by our automated tests.
  • If you are changing shared mod or engine code, make sure that you have tested your changes in all four default mods.
  • Respond to review comments as soon as you reasonably can. Reviewers will usually prioritize Pull Requests that are still fresh in their minds. Make sure to leave a comment when you push new changes, otherwise GitHub does not automatically notify reviewers!
  • Leave a polite comment asking for reviews if a week or more has passed without feedback.

If you need any help you can ask in the #openra IRC channel on freenode (most active during European evenings).

@everclear72216 everclear72216 requested a review from pchote Feb 5, 2020
Copy link
Member

pchote left a comment

Looks pretty good already, just a few smallish comments:

OpenRA.Mods.Common/Widgets/Logic/SettingsLogic.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/Logic/SettingsLogic.cs Outdated Show resolved Hide resolved
OpenRA.Platforms.Default/Sdl2PlatformWindow.cs Outdated Show resolved Hide resolved
OpenRA.Platforms.Default/Sdl2PlatformWindow.cs Outdated Show resolved Hide resolved
@pchote pchote added this to the Next Release milestone Feb 6, 2020
@pchote

This comment has been minimized.

Copy link
Member

pchote commented Feb 6, 2020

It looks like you maybe forgot to push the latest fixes?

@everclear72216

This comment has been minimized.

Copy link
Contributor Author

everclear72216 commented Feb 6, 2020

Sorry, I was just hesitant because i forgot to test tiberian dawn, which has a different settings.yaml.

Copy link
Member

pchote left a comment

Looking good. Just a couple more points, then LGTM.

Can you please also squash the fixups into the first commit?

OpenRA.Mods.Common/Widgets/Logic/SettingsLogic.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/Logic/SettingsLogic.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/Logic/SettingsLogic.cs Outdated Show resolved Hide resolved
@everclear72216 everclear72216 force-pushed the everclear72216:feature/make-monitos-selectable-in-fullscreen-modes branch from 64509b6 to e086661 Feb 8, 2020
@everclear72216

This comment has been minimized.

Copy link
Contributor Author

everclear72216 commented Feb 8, 2020

Took care of the comments and squashed the commits. Reworded commit message.

@dragunoff

This comment has been minimized.

Copy link
Contributor

dragunoff commented Feb 8, 2020

Tested on Ubuntu, works as promised 👍

I'm just wondering whether the display dropdown should be visible on single monitor setups as it serves no purpose in that case 🤔

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Feb 8, 2020

We should still show it (otherwise it makes the UI code even more of a nightmare), but should disable the dropdown so its not clickable.

OpenRA.Game/Settings.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/Logic/SettingsLogic.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/Logic/SettingsLogic.cs Outdated Show resolved Hide resolved
@everclear72216 everclear72216 force-pushed the everclear72216:feature/make-monitos-selectable-in-fullscreen-modes branch from e086661 to f02068b Feb 8, 2020
@everclear72216 everclear72216 requested a review from teinarss Feb 8, 2020
@pchote
pchote approved these changes Feb 8, 2020
@pchote pchote dismissed teinarss’s stale review Feb 8, 2020

Requests addressed.

@pchote pchote merged commit 98aef70 into OpenRA:bleed Feb 8, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@everclear72216 everclear72216 deleted the everclear72216:feature/make-monitos-selectable-in-fullscreen-modes branch Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.