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

[Tech] Refactor Game Managers #2578

Merged
merged 26 commits into from Apr 6, 2023
Merged

Conversation

BrettCleary
Copy link
Collaborator

@BrettCleary BrettCleary commented Mar 28, 2023

This PR refactors the legendary, gog, and sideload store managers into a consistent module level interface.

This reduces complexity and decouples the store managers from code that consumes it through abstraction and allows for further extensibility of game managers to other stores in the future.

This also removes the overly complex object oriented singleton approach which only serves to remove the need to pass appName to each function. A module level functional approach is simpler, easier to test, and accomplishes the same objectives of code reuse and consistent interface.

Sideload Game info has been added into GameInfo which makes sense because the frontend shouldn't be concerned with implementation level backend details like what store/runner the game is using

Fixes a bug with the default install path setting

Fixes a bug where successive downloads would remain queued and not install

Tested on Windows

(Launch Proton tested on SteamDeck)

Epic Games

  • Login (alternative)
  • Logout
  • Install
  • Import Games
  • Launch Native
  • Launch Proton
  • Move Game
  • Change Install Path
  • Uninstall

GOG

  • Login
  • Logout
  • Install
  • Import
  • Launch Native
  • Launch Proton
  • Move Game
  • Change Install Path
  • Uninstall

Sideload

  • Add Game
  • Launch Game
  • Remove Game

Further Work

Refactor gog/user.ts and legendary/user.ts to use the same module level functional type

  • I would prefer to do this on a separate branch

Fix electron module mocking for tests

  • Electron should be automocked with only some of the methods overridden instead of exposing a new interface that only mocks some of the electron methods
  • This might be best handled on a separate branch as well

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)

* init

* refactor gog, legendary, hyperplay games.ts

* update submodule to latest main commits

* refactor game manager games module

* update types, refactor library manager

* fix diskWrite merge

* fix more types

* rm bin exec override

* move store managers into storeManagers folder

* fix data types for hp games, add listUpdateableGames

* refactor library refresh

* refactor runWineCommand and hasGame

* rm hasUpdate, fix hp store install version

* rm refreshInstalled from library interface

* rm unused updateAllGames

* fix stop, add unzipping status, add inject process name

* change unzipping to extracting, add process names, fix restricted char bugs

* fix legendary and gog download status

* fix gog install, uninstall

* fix default install path setting

* fix proton/wine

* fix gog and legendary import

* update gameInfo and use gameInfo to inject HP Games

* move storeManager maps and fxns out of main

* fix legendary moveInstall and changeInstallPath

* refresh gog when app is ready

* fix changeGameInstallPath

* fix uninstall when gog game is already moved

* fix remaining merge conflicts

* add one conditional method call
@BrettCleary BrettCleary marked this pull request as ready for review March 28, 2023 20:16
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.

Thats a lot of stuff but in general looks good.
Tested basically everything on a new install and current install and everything seems to work just fine.

I think this approach is fine and finally, we are moving out of OOP and all boilerplate it was bringing to the code.

Thanks for that

@flavioislima
Copy link
Member

I am merging this one to avoid more conflicts in the future.

@flavioislima flavioislima merged commit 48d2787 into main Apr 6, 2023
7 checks passed
@flavioislima flavioislima deleted the tech/refactor_game_managers branch April 6, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants