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

[Fix] AppSettings type mismatches #2718

Merged
merged 1 commit into from
Nov 6, 2023
Merged

[Fix] AppSettings type mismatches #2718

merged 1 commit into from
Nov 6, 2023

Conversation

CommandMC
Copy link
Collaborator

@CommandMC CommandMC commented May 19, 2023

  • userInfo's structure was completely different to how it should be like, and it seems like it wasn't used anywhere, so removing it felt like the best option
  • The @ts-expect-error around the object was moved, this way it won't hide errors like this in the future
  • Some other smaller issues were corrected

This is also one of the many steps we'll have to take to untangle our circular dependency issues, as this LegendaryUser import currently leads to this import chain:

src/backend/config.ts
-> src/backend/storeManagers/legendary/user.ts
-> src/backend/utils.ts
-> src/backend/storeManagers/legendary/library.ts
-> src/backend/launcher.ts
-> src/backend/tools.ts
...

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@CommandMC CommandMC added the pr:ready-for-review Feature-complete, ready for the grind! :P label May 19, 2023
@CommandMC CommandMC requested a review from a team May 19, 2023 19:29
@CommandMC CommandMC self-assigned this May 19, 2023
@CommandMC CommandMC requested review from arielj, flavioislima, Nocccer and imLinguin and removed request for a team May 19, 2023 19:29
Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks, good, I think we are not using this info anywhere anymore.
One thing is that I don't remember why, but on the customWinePaths, when using just the array instead of null, was somehow breaking the settings on windows. But this might be fixed.

@CommandMC
Copy link
Collaborator Author

I can't find any usages of customWinePaths in places that aren't gated by an if (!isWindows), so I'm guessing this is fixed. Still, I'll give this another test on Windows in a few days

@flavioislima
Copy link
Member

@CommandMC any reason this one has not merged since it was approved in May?

@CommandMC
Copy link
Collaborator Author

I wanted to test this on Windows one more time, but then never got around to it. I'll fix up the conflicts & re-test soon:tm:

- `userInfo`'s structure was completely different to how it should be
  like, and it seems like it wasn't used anywhere, so removing it felt
  like the best option
- The `@ts-expect-error` around the object was moved, this way it won't
  hide errors like this in the future
- Some other smaller issues were corrected
@CommandMC CommandMC force-pushed the fix/settings-defaults branch from afbe2ba to fb04c89 Compare November 6, 2023 13:26
@CommandMC CommandMC merged commit bc3c2dd into main Nov 6, 2023
13 checks passed
@CommandMC CommandMC deleted the fix/settings-defaults branch November 6, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants