Skip to content

Conversation

Catta1997
Copy link
Contributor

@Catta1997 Catta1997 commented Jun 11, 2024

Start working on fix render issue of Settings UI where the toggle not update (state change but not graphically)

  • KeyMapView
  • GraphicView
  • BypassView
  • MiscView
  • Check if ALL toggle are saved correctly

@Catta1997
Copy link
Contributor Author

Registrazione.schermo.2024-06-11.alle.23.20.54.mov

@Catta1997 Catta1997 changed the title Update AppSettingsView.swift Fix Setting render issus Jun 11, 2024
@Catta1997
Copy link
Contributor Author

Catta1997 commented Jun 11, 2024

Should be all, not totally sure if all settings is saved correctly. I checked MetalHUD and DiscordRP and both are correct

@Catta1997 Catta1997 marked this pull request as ready for review June 11, 2024 22:12
Removed no necessary onChange()
@mitchellmebane
Copy link
Contributor

mitchellmebane commented Jun 23, 2024

I built this branch and all the controls I tested work properly.

Clicking the "Fix window display issues" checkbox on the Graphics tab doesn't enable the dropdown next to it, but I've never used that, so I don't know how it's supposed to work. I can't seem to enable that dropdown in the old nightly 636, either.

@mitchellmebane
Copy link
Contributor

After looking through the code, I see that the dropdown only gets enabled when "Fix window display issues" is checked and "Resolution" is set to "App Default". It's working properly in this branch. 👍

@Depal1 Depal1 changed the title Fix Setting render issus Fix Setting render issues Jun 29, 2024
@Depal1 Depal1 changed the title Fix Setting render issues Fix Settings render issues Jun 29, 2024
@TheMoonThatRises
Copy link
Member

I think we should wait for an update from Apple, as this is most likely a bug and not intended behaviour. This does not occur on previous versions.

@Catta1997
Copy link
Contributor Author

I think we should wait for an update from Apple, as this is most likely a bug and not intended behaviour. This does not occur on previous versions.

oh ok, btw this is a thing in other part of the project (Settings/InstallSettings.swift, PlayCover/Views/Settings/UpdateSettings.swift and PlayCover/Views/Settings/UninstallSettings.swift).

@mitchellmebane
Copy link
Contributor

mitchellmebane commented Jul 4, 2024

I may be way off base here, but is the issue the multi-level object nesting? E.g., in the AppSettingsView tab views, the views are binding to an instance of AppSettings, but the controls are using an attribute of the nested AppSettingsData.

Picker("", selection: $settings.settings.iosDeviceModel)

All the docs and examples of SwiftUI data binding I've seen either use a directly published property, or use a property of a published class. I haven't seen anything which uses a property in a struct nested in a published class.

@TheMoonThatRises
Copy link
Member

I may be way off base here, but is the issue the multi-level object nesting? E.g., in the AppSettingsView tab views, the views are binding to an instance of AppSettings, but the controls are using an attribute of the nested AppSettingsData.

Picker("", selection: $settings.settings.iosDeviceModel)

All the docs and examples of SwiftUI data binding I've seen either use a directly published property, or use a property of a published class. I haven't seen anything which uses a property in a struct nested in a published class.

I believe that this is the issue. @Catta1997 if you want to try fixing it by removing the nesting of classes and structures, do try. If not, I can merge this as a temporary fix.

@Catta1997
Copy link
Contributor Author

Ok, I will try

@Catta1997
Copy link
Contributor Author

But since I don know a lot SwiftUI I think you can merge this temporary fix 'cause I don't know if I will be able to fix it in the other way

@TheMoonThatRises TheMoonThatRises merged commit d591f49 into PlayCover:develop Jul 7, 2024
@Catta1997 Catta1997 deleted the settings-fix branch August 1, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants