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

PCSX2-WX: Add restore defaults button to GS panel #2003

Merged
merged 1 commit into from Jul 12, 2017

Conversation

Projects
None yet
4 participants
@ssakash
Member

ssakash commented Jul 5, 2017

Summary of changes:

  • Added Restore Defaults button to the GS panel of Emulation settings dialog.

Initially, I was planning on adding it for all the dialogs but it seems pointless to add it to Gamefixes panel as you can just uncheck the Enable Manual Gamefixes and be done with it and the GS Window panel seems to be under a bit of a work right now ( #1950 & #1918), wouldn't want to cause any conflicts with them, so I'm doing it only for the GS panel at the moment.

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Jul 10, 2017

Contributor

I love screenshot when you update the GUI. Where did you put the button ?

Contributor

gregory38 commented Jul 10, 2017

I love screenshot when you update the GUI. Where did you put the button ?

@ssakash

This comment has been minimized.

Show comment
Hide comment
@ssakash

ssakash Jul 10, 2017

Member

Sorry, you know how much I love to post screenshots. ;)

picture

I've placed it in the bottom right of the right most panel. (similar to the placement of Restore defaults in the other panels for consistency).

Member

ssakash commented Jul 10, 2017

Sorry, you know how much I love to post screenshots. ;)

picture

I've placed it in the bottom right of the right most panel. (similar to the placement of Restore defaults in the other panels for consistency).

@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Jul 10, 2017

Member

Looks good, behaves properly as well.
Is it also added on linux btw?

Member

lightningterror commented Jul 10, 2017

Looks good, behaves properly as well.
Is it also added on linux btw?

@ssakash

This comment has been minimized.

Show comment
Hide comment
@ssakash

ssakash Jul 11, 2017

Member

Is it also added on linux btw?

Yes, wxWidgets code is common for all platforms.

Member

ssakash commented Jul 11, 2017

Is it also added on linux btw?

Yes, wxWidgets code is common for all platforms.

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Jul 11, 2017

Contributor

It would be nice to have a single button in the bottom bar (on the same level as ok/cancel/apply). Instead to have a per panel button. Besides it will save room for small screen. I don't know if it is difficult to code.

Contributor

gregory38 commented Jul 11, 2017

It would be nice to have a single button in the bottom bar (on the same level as ok/cancel/apply). Instead to have a per panel button. Besides it will save room for small screen. I don't know if it is difficult to code.

@ssakash

This comment has been minimized.

Show comment
Hide comment
@ssakash

ssakash Jul 11, 2017

Member

It would be nice to have a single button in the bottom bar (on the same level as ok/cancel/apply).

I don't see how it's better, it looks sort of awkward to me to have it at the same level as OK/Cancel/Apply. The bottom is already crowded with those buttons plus the preset slider and screenshot icon.

I just placed the Restore Defaults similar to it's position in other tabs. (EE/IOP, VUs, Speedhacks)

Member

ssakash commented Jul 11, 2017

It would be nice to have a single button in the bottom bar (on the same level as ok/cancel/apply).

I don't see how it's better, it looks sort of awkward to me to have it at the same level as OK/Cancel/Apply. The bottom is already crowded with those buttons plus the preset slider and screenshot icon.

I just placed the Restore Defaults similar to it's position in other tabs. (EE/IOP, VUs, Speedhacks)

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Jul 11, 2017

Contributor

I just placed the Restore Defaults similar to it's position in other tabs. (EE/IOP, VUs, Speedhacks)

Yes I know that it is the same issue for others tabs. It wasn't directly related to this PR.

It is strange to have a default button at various location which depends on the current tab. Here it is fine but it takes too much room on the speedhack panel (people still have low height display). Besides, maybe it would be better to have a single restore default button that restore all default instead of the current tab option (except if the use case is to only "default" a single tab).

it looks sort of awkward to me to have it at the same level as OK/Cancel/Apply.

Why ? I mean all 4 buttons control the settings. You can validate the settings, you can discard the settings. It feels logical to be able to reset the settings too. The fact that you need to duplicate it in all tabs is a proof that it doesn't belong to a tab.

Contributor

gregory38 commented Jul 11, 2017

I just placed the Restore Defaults similar to it's position in other tabs. (EE/IOP, VUs, Speedhacks)

Yes I know that it is the same issue for others tabs. It wasn't directly related to this PR.

It is strange to have a default button at various location which depends on the current tab. Here it is fine but it takes too much room on the speedhack panel (people still have low height display). Besides, maybe it would be better to have a single restore default button that restore all default instead of the current tab option (except if the use case is to only "default" a single tab).

it looks sort of awkward to me to have it at the same level as OK/Cancel/Apply.

Why ? I mean all 4 buttons control the settings. You can validate the settings, you can discard the settings. It feels logical to be able to reset the settings too. The fact that you need to duplicate it in all tabs is a proof that it doesn't belong to a tab.

@ssakash

This comment has been minimized.

Show comment
Hide comment
@ssakash

ssakash Jul 12, 2017

Member

Besides, maybe it would be better to have a single restore default button that restore all default instead of the current tab option

(except if the use case is to only "default" a single tab).

Yeah, thanks for repudiating your own suggestion by providing a counter-argument along with it. :P

Let's assume a certain scenario with your global restore defaults idea,

  • Person A has his manually tuned speedhacks necessary for his game but now he encounters a new rounding/clamping bug, he tries playing with the rounding/clamping modes and now wants to switch back to the defaults. What would he do? He can't possibly use the global restore defaults option as it would also ruin his personal combination of speedhacks.

Why ? I mean all 4 buttons control the settings. You can validate the settings, you can discard the settings. It feels logical to be able to reset the settings too.

Probably because OK/Cancel/Apply applies to all the tabs and not just for the current tab like Restore Defaults? It doesn't make much sense to have them at the same level.

Also it's not used enough to place it near the global OK/Cancel/Apply buttons which are frequently used, that's my reasoning on why I'd prefer to not have together but your mileage may vary, it's a rather subjective topic.

The fact that you need to duplicate it in all tabs is a proof that it doesn't belong to a tab.

Well, there's no other way, It needs to be duplicated in every single tab if we need a button which restores only the current tab's options to defaults.

Member

ssakash commented Jul 12, 2017

Besides, maybe it would be better to have a single restore default button that restore all default instead of the current tab option

(except if the use case is to only "default" a single tab).

Yeah, thanks for repudiating your own suggestion by providing a counter-argument along with it. :P

Let's assume a certain scenario with your global restore defaults idea,

  • Person A has his manually tuned speedhacks necessary for his game but now he encounters a new rounding/clamping bug, he tries playing with the rounding/clamping modes and now wants to switch back to the defaults. What would he do? He can't possibly use the global restore defaults option as it would also ruin his personal combination of speedhacks.

Why ? I mean all 4 buttons control the settings. You can validate the settings, you can discard the settings. It feels logical to be able to reset the settings too.

Probably because OK/Cancel/Apply applies to all the tabs and not just for the current tab like Restore Defaults? It doesn't make much sense to have them at the same level.

Also it's not used enough to place it near the global OK/Cancel/Apply buttons which are frequently used, that's my reasoning on why I'd prefer to not have together but your mileage may vary, it's a rather subjective topic.

The fact that you need to duplicate it in all tabs is a proof that it doesn't belong to a tab.

Well, there's no other way, It needs to be duplicated in every single tab if we need a button which restores only the current tab's options to defaults.

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Jul 12, 2017

Contributor

Not really a counter argument. Even if you put the button in the bar, you can still code it to reset only the current tab.

The fact is that this button consume too much space in the speedhack panel. So far the solutions are

  • keeps the button in the middle and render crap in small screen
  • puts the button in a different location

Technically it would be the same story for the gamefix panel... But I guess it won't have it.

Anyway, I'm not against your PR.

Contributor

gregory38 commented Jul 12, 2017

Not really a counter argument. Even if you put the button in the bar, you can still code it to reset only the current tab.

The fact is that this button consume too much space in the speedhack panel. So far the solutions are

  • keeps the button in the middle and render crap in small screen
  • puts the button in a different location

Technically it would be the same story for the gamefix panel... But I guess it won't have it.

Anyway, I'm not against your PR.

@gregory38 gregory38 added the Reviewed label Jul 12, 2017

@gregory38 gregory38 merged commit 9e13b7d into PCSX2:master Jul 12, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ssakash ssakash deleted the ssakash:wx_restore_default branch Jul 13, 2017

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