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 ability to edit game-specific GFX settings from game properties tab. #13063

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

Conversation

TryTwo
Copy link
Contributor

@TryTwo TryTwo commented Sep 11, 2024

edit Reworked and ready to be reviewed.
Follows iwubcode's suggestion below. Adds tabs to game properties window that allows editing game-specific GFX settings. Uses the existing GFX widgets as new tabs, but passes a game layer to them that's outside the global game layer system. This is the solution I came to after trying multiple things.

Features:

  • Right click gfx option to remove it from the ini
  • Edit multiple games at once
  • Edit while a game is running, but only activates settings when window is closed (not sure how safe it is to do it more often).

Notes:
This PR integrates with the ConfigBool, etc widgets that set most GFX settings. The GFX settings outside of this system require too much editing for this PR. They would need to be conditionally redirected to the custom layer, be bold-able if in the game ini, right clickable to remove them from the game ini, and save only when interacted with (they usually batch save). All of those are hard to do for the few options that require it.

Possible issue:
If multiple game ini's are somehow loaded into the editor widget as tabs (maybe from game modding?) it won't refresh the game ini tab correctly.

@iwubcode
Copy link
Contributor

iwubcode commented Sep 11, 2024

I voiced my opinion several times on discord, so this is probably unnecessary to state. This is more in case whoever is reviewing/merging doesn't see those comments and for some reason doesn't think of this.

I don't think we should be using a button to open the Dolphin global windows. We should be using the widgets themselves as new tabbed entries in the game specific window. This is why I broke various pieces of code into their own widgets in the first place, see also my mocked out example window which uses those widgets.

I haven't looked at other emulators but I'd assume this is how they are doing it (most seem to be using a unified window as well but baby steps..). This gives a consistent experience to the user (settings are in the same place, they behave the same as global settings, users have full control, etc).

Regardless of approach, will we keep the ini editing capability? I think it was primarily for graphics...

All that being said, happy to see this picked up. Thanks @TryTwo !

@TryTwo
Copy link
Contributor Author

TryTwo commented Sep 11, 2024

Not part of this PR

@iwubcode

You're thinking something like this is better? There needs to be a way to switch between three states. On, Off, and Not Local. I have this problem too in this PR. I'm also not sure if there's a simple way to remove global settings and only show local ones.

@iwubcode
Copy link
Contributor

iwubcode commented Sep 11, 2024

@TryTwo - yeah, if we go that route, I'm sure on of the UI experts can provide ideas on what best to do. My thinking was to add a "Graphics" tab at top. Then have a "General", "Hacks", "Advanced" set of tabs underneath, just like we do today in the main window. But maybe there's a better visual look. (also depends on if we want to keep General/Editor under "Game Config"...but I don't want to blow out your PR lol)

There needs to be a way to switch between three states. On, Off, and Not Local.

I was thinking the global window would be the "Not Local" setting.

But to your point, how do you unset a game specific setting and go back to global. I wonder how other emus do this. At least looking at a PCSX2 screenshot, seems their comboboxes have a "Use Global" option (that would be more work that I wasn't considering). But also not sure how their checkboxes work... 🫤

Maybe just a global "reset" button to just clear the layer? Definitely an interesting question!

@LonelySpaceDetective
Copy link

But to your point, how do you unset a game specific setting and go back to global. I wonder how other emus do this. At least looking at a PCSX2 screenshot, seems their comboboxes have a "Use Global" option (that would be more work that I wasn't considering). But also not sure how their checkboxes work... 🫤

I can't speak on a technical level, but PCSX2 has three states for checkboxes when editing game-specific settings; on, off and use global.
Checked is on, empty is off and a filled grey box is global.

@TryTwo
Copy link
Contributor Author

TryTwo commented Sep 11, 2024

I can't speak on a technical level, but PCSX2 has three states for checkboxes when editing game-specific settings; on, off and use global. Checked is on, empty is off and a filled grey box is global.

Thanks, that's a good solution for a more robust PR.

Unfortunately it's too much work for a quick fix, since we're using existing widgets that don't have that. If we duplicate widgets into game config then a reset button per widget to clear sections would be fine. Otherwise I think a reset everything button will be needed.

There's also the suggestion for right clicking to disable the User state for an individual option. It's possible to do.

@TryTwo
Copy link
Contributor Author

TryTwo commented Sep 12, 2024

My conclusion is that the game config layer system needs to be expanded before this can be done correctly. imo the game specific settings window needs to operate on a separate instanced layer outside the global layer system (which isn't built to handle this).

Game properties bypasses the layer system entirely, so that's why there's no problem currently. A few important settings could be added to the current game properties method as a quick fix, but any robust improvement will require more than I originally thought. I'll leave it to someone else to tackle those bigger tasks.

The Android version works because its quite limited. One game at a time, not when a game is running , not when the global settings are up. I can duplicate those restrictions, but I think the end result won't feel good on PC. It also prevents @iwubcode 's idea of keeping the settings in the game properties window.

@TryTwo
Copy link
Contributor Author

TryTwo commented Sep 13, 2024

I was able to add right-click to delete a local game change.
I fixed a lot of issues it was having by changing how it detected if we wanted a local game edit.
It assumes if a globalgame layer is added, then we are not doing a local game edit and nothing should be done. This happens when a game is playing, not sure if it happens at another time.

For general config it's hard to tell what's appropriate to edit for a local game ini. My method just attempts to write any change to the game ini. Not sure the general config window should be added.

@TryTwo
Copy link
Contributor Author

TryTwo commented Sep 14, 2024

I tried doing what I said I wasn't going to do. We'll see if it worked out.

I managed to fit all the stuff into game properties and make it work with multiple game properties open at once. Any custom-written game options haven't been converted to work with this.

@TryTwo TryTwo force-pushed the PR_GameSettings branch 2 times, most recently from 1b641c3 to 636099a Compare September 16, 2024 01:04
…set to the user game ini.

Creates a layer outside the game config layer system and passes it to the created gfx widows, so as to not interfere with the global config system.

Supports multiple game properties being open at once.
Supports editing while a game is playing, but the options only save and update the active game when the window is closed.
Right-clicking will remove a property from the game ini.

Does not support a few options that would require heavily editing the GFX widgets.  This is for options using the ConfigBool, etc system. Other options can be added later.
@TryTwo TryTwo changed the title TEST: Open graphics widget from game config and make game-local changes Add ability to edit game-specific GFX settings from game properties tab. Sep 18, 2024
@dreamsyntax
Copy link
Member

dreamsyntax commented Sep 19, 2024

Did some testing.

Overall it works just fine for the editable fields.

The only 'issue' is editing Backend or Adapter in GFX -> Basic changes it for Global.
Additionally Adapter is not selectable at all in the Game Properties dialog.

image

Presumably this is already known based on your prior comment:

The GFX settings outside of this system require too much editing for this PR.

Calling it out just in case.

The sub-views within views is... not pretty to look at, but this is perfectly functional as is.

I already have two practical uses for this:

  • force disabling Vertex Rounding if utilizing Graphics Mods
  • forcing aspect ratio while keeping it as "Auto" for global for widescreen hacks or games with bad widescreen heuristic values

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

Successfully merging this pull request may close these issues.

4 participants