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

HacksWidget: Convert texture cache accuracy option to radio buttons. #13234

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

Conversation

TryTwo
Copy link
Contributor

@TryTwo TryTwo commented Dec 24, 2024

cache

The only option I didn't fold into the game properties widgets PR. It required UI changes, which I didn't want to make in that PR.
Works better with the config system, and allows user to toggle off/on a preset custom value during play.

There is the small issue with the game properties widgets not having global/system game ini data to set/show the options with. When accessed within game properties, I coded it for users changing between the preset options and left custom values out of it, as that's usually done in a system and not user ini afaik.

Works better with the config system, and allows user to toggle off/on a preset custom value during play.
@MayImilae
Copy link
Contributor

This is largely a UX downgrade. The radio buttons mean more clicks to do things, and present more visual elements to to user so it looks busier and less friendly. The only real expansion of functionality is an indicator/selector for a custom value that is in the INI, but that is a developer feature and not something users should really be exposed to anyway, so I don't think that's is worth the UX cost to expose it to users.

So, my suggestions.

The slider makes it so multiple settings are exposed as one element, reducing the UX impact. If you want to move to radio buttons, you should remove the moderate setting and just have Safe and Fast. It's something we've considered, but since the slider makes having moderate a non-issue we haven't gone for it yet. I'm not sure that is worth doing, but if you really prefer radios that would make moving to radios an even trade, UX-wise.

We should not expose custom texture accuracy to users who have not touched an INI. We don't expose custom in Graphics > Hacks for a reason (with hundreds of values that do nothing, the placebo potential is enormous), and if users can't edit a setting in the GUI, they shouldn't be able to see it in the GUI. I'd be fine with a tooltip that sneakily reveals the value under the hood though. I'd also be fine with a custom indicator appearing if a user has messed with INI values though. That sounds annoying to implement to me, but that's an option you could explore.

As another option, you could have an entirely separate Game Properties control that exposes more than the standard Graphics settings. So basically, leave graphics > hacks alone, and make an entirely new control in the game properties that allows switching between the presents or adjusting the actual value directly. That would reduce UX impact but also give more control for power users wanting to tweak things without dipping into INIs (which is what the game properties is for, more or less.)

@JMC47
Copy link
Contributor

JMC47 commented Dec 24, 2024

The safe texture cache bug needs to be fixed before medium could be removed.

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 24, 2024

This is largely a UX downgrade. The radio buttons mean more clicks to do things, and present more visual elements to the user so it looks busier and less friendly.

I don't fully agree on this. It's the same number of clicks to use and the only real difference is that I labeled the middle option as "moderate". The slider makes you think there's a smooth range of values when in actuality there are 3 discreet values. I feel radios communicate the option a little better. Also, the slider requires a layer of interpretation in the code that I'm trying to avoid so it can be streamlined to work with the config system.

So, my suggestions.

We should not expose custom texture accuracy to users who have not touched an INI. We don't expose custom in Graphics > Hacks for a reason (with hundreds of values that do nothing, the placebo potential is enormous), and if users can't edit a setting in the GUI, they shouldn't be able to see it in the GUI. I'd be fine with a tooltip that sneakily reveals the value under the hood though. I'd also be fine with a custom indicator appearing if a user has messed with INI values though. That sounds annoying to implement to me, but that's an option you could explore.

In standard Dolphin, if there is an ini custom value, the option is disabled entirely for the running game I'm okay with removing the custom button and keeping it that way. The reason I made the button is because it allows a temporary change while the game is running, which doesn't save. I can see how it'd be confusing, especially as it checks the ini set by the system.

I can easily limit the custom button to checking if the user's game ini has been changed, if other people want that. The button is already hidden if no custom value is set anywhere.

As another option, you could have an entirely separate Game Properties control that exposes more than the standard Graphics settings. So basically, leave graphics > hacks alone, and make an entirely new control in the game properties that allows switching between the presents or adjusting the actual value directly. That would reduce UX impact but also give more control for power users wanting to tweak things without dipping into INIs (which is what the game properties is for, more or less.)

With the way I'm coding it, I'm trying to keep the graphics widgets identical. The general game properties widget could have a custom value option, but I'm not sure it's really needed. My main goal here is to make something sensible that goes through the Config system. When everything is in the config system making large adjustments becomes quite easy.

I'm not committed to a particular design, so any discussion is quite helpful here.

@MayImilae
Copy link
Contributor

MayImilae commented Dec 24, 2024

Radio boxes with more than two items are generally avoided in modern designs, since it's visually noisy. Today, radio boxes may appear as just two items, but most of the time radio boxes are just avoided outright and other tools such as toggles and comboboxes are used instead.

I'm not explicitly attached to the slider though, but I'd like to avoid more than two radio box items. How about a combobox?

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 24, 2024

I could do a ComboBox, but would have to expand ConfigChoice to take custom int values. I'll do that if everyone is okay with a ComboBox.

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 24, 2024

cache

May be a little hard to read, but that will be displayed whenever there's a custom value. The box will be disabled too.

Does this look okay?

@mbc07
Copy link
Member

mbc07 commented Dec 24, 2024

I'm personally against replacing the slider with a combo box on this case, it would just introduce more clicks for no reason (IMO). What I think could work is actually introducing a label with the current value somewhere close to the slider, there's already plenty examples of that in Dolphin (Dolby Pro Logic II decoder quality, CPU/RAM overrides, HDR and Gamma sliders in Color Correction settings, etc). I also agree we shouldn't allow arbitrary values to be set directly on the GUI, this should remain INI-only.

Make the label show the values for the Safe/Medium/Fast steps the existing slider already have, and in case a custom value is set via INI, position the slider to the leftmost/rightmost position (depending of the custom value) and show the custom value in the label, with a (custom) or something to sinalize that. If the user then moves the slider in that state, override the setting back to the predefined safe/medium/fast steps and drop the (custom) postfix from the label.

(I'd try to make a mockup but it's Christmas Eve already, could do this later, though)

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 25, 2024

I don't think we need to show the values,

My only goal is to get it working with game properties widget and the slider is rather convoluted for that. The slider goes 0 - 512 - 128 and that requires custom code outside the Config system to process.

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