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

Change the word "Enable" to "Apply" in widescreen patches and no-interlacing patches settings #11236

Merged
merged 3 commits into from
May 16, 2024

Conversation

karasuhebi
Copy link
Contributor

Description of Changes

Removed the word "Enable" from two different checkboxes.

Rationale behind Changes

The word enable is redundant here. It is implied that the patches are enabled when the box is checked. That's how the rest of the settings in this window work (except for Interlace Offset which, IMO, should be reworked as well to not include the word "Disable")

Suggested Testing Steps

…atches

The word enable is redundant here. It is implied that the patches are enabled when the box is checked. That's how the rest of the settings in this window work (except for Interlace Offset which, IMO, should be reworked as well to not include the word "Disable")
@stenzek
Copy link
Contributor

stenzek commented May 12, 2024

I'm not really sure I agree. The other options directly affect the behaviour of PCSX2 (e.g. anti-blur), whereas patches control whether a patch is enabled, which then affects the behaviour of the game.

I still kinda hate that we still have these "global" options in the first place, the intended method now is to enable the patches in Game Properties. But removing the option has other implications, see #10528 (comment)

@refractionpcsx2
Copy link
Member

tbh I think removing the Enable makes it more implicative that your game will be made widescreen. if anything I'd change it to "Allow", but at this point we're just bikeshedding what it should say :P

@karasuhebi
Copy link
Contributor Author

OK yes, now that you explain it that way (that it affects the behavior of the game, not the emulator) I do see your point.

My main gripe was honestly the fact that it had the word "Enable" so that when you ask someone to enable the option, you're saying "Enable the 'Enable Widescreen Patches' option" which sounds silly. I like ref's suggestion of changing it to "Allow" though. What do we think of that?

@stenzek
Copy link
Contributor

stenzek commented May 13, 2024

It's not really allowing it either - it's not like the default action is to deny, the default action is to not even look for patches to begin with.

Like I said above, the option really needs to be scrapped anyway. It also doesn't make sense for patches for different aspect ratios (I believe there are some now); which one loads?

@Berylskid
Copy link
Contributor

How about 'Apply'? Personally, it sounds closer than 'Allow', especially for patches.

@karasuhebi
Copy link
Contributor Author

I really like "Apply" honestly. stenzek would you say it would make sense to use "Apply" instead of "Enable" for now while we wait and see if these global options are to be removed at the end of the day? Or just leave it alone for now?

@stenzek
Copy link
Contributor

stenzek commented May 14, 2024

/shrug, whatever, apply is fine I guess. There's going to be resistance if we remove the option anyway.

@karasuhebi karasuhebi changed the title Remove the word "Enable" from widescreen patches and no-interlacing patches Chane the word "Enable" to "Apply" in widescreen patches and no-interlacing patches settings May 16, 2024
@karasuhebi
Copy link
Contributor Author

Change made.

@karasuhebi karasuhebi changed the title Chane the word "Enable" to "Apply" in widescreen patches and no-interlacing patches settings Change the word "Enable" to "Apply" in widescreen patches and no-interlacing patches settings May 16, 2024
@stenzek stenzek merged commit 1483e4f into PCSX2:master May 16, 2024
12 checks passed
@karasuhebi karasuhebi deleted the patch-1 branch May 16, 2024 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants