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

[Feat - General] Sideload Apps #1864

Merged
merged 100 commits into from Oct 25, 2022
Merged

[Feat - General] Sideload Apps #1864

merged 100 commits into from Oct 25, 2022

Conversation

flavioislima
Copy link
Member

@flavioislima flavioislima commented Oct 4, 2022

This adds the possibility of adding a game outside Epic/GOG. That can use the same configuration system we have and support the same features as the previous.
It can be a Game or any type of software.

OBS.: this also serves as POC for a functional-based approach instead of OOP like we have for Epic and GOG. This decision leads to some code duplication and could be improved if we decided to use functional approach on the other runners as well in the future.

Implements #1506

Checkliist

  • Add backend methods to add a game to a sideload library similar to current ones
  • Add an Install dialog that can be used to add a new App/Game
  • Handle GamePage and settings for sideloaded app
  • Add a way to edit the game
  • Handle Uninstall / Add shortcut / Add to Steam for sideloaded apps
  • Run Windows games on Linux using Wine/Proton
  • Run Native Linux binaries
  • Run Native windows Games
  • Run native macOS games
  • Add new filter for sideloaded games
  • Run windows games on macOS using crossover
  • App should have a log
  • App should show with correct name on Discord Rich Presense
  • Changed dev command from yarn dev to yarn start
  • Changed defaults for Install DXVK and VKD3D to be true.

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)

Screenshots:

image
image
image

@flavioislima
Copy link
Member Author

Ok, fixed the UI bugs:
image
image

And also the issue with the install platform. I swear I have tested installing games using 'Windows' on legendary and it worked but now I can reproduce the issue.

The error you showed related to the icon I cannot reproduce it. Can you maybe try to remove node_modules and run yarn again?

The progress issue I am investigating.

@flavioislima
Copy link
Member Author

flavioislima commented Oct 21, 2022

Ok, fixed the missing progress issue.
Somehow I removed the onOutput option from callRunner so the progress was not being sent to the frontend.

That was a really good catch, thanks for that.

@arielj
Copy link
Collaborator

arielj commented Oct 22, 2022

I can't reproduce the console issue when opening the add game modal, that can be ignored 👍

One thing that will be a problem for sure, we should remove invalid folder name characters for the prefix, you can put / * ? ! anything there
image

Maybe it's a good idea to also put a limit in the app's name length? now it can be anything, can create issues in the UI a weirdly long name

@arielj
Copy link
Collaborator

arielj commented Oct 22, 2022

Hmmm I can get the devtools console error when I change from Windows to Linux in the platform selector

@arielj
Copy link
Collaborator

arielj commented Oct 22, 2022

If you delete the app's title, the prefix has no folder (which makes sense) but you can still execute an installer with the Run Installer First button. You can even delete the complete prefix path and the Run Installer First button is still clickable.

If the title is empty and you change to Linux and then back to Windows, the prefix now uses a sideload folder.

@arielj
Copy link
Collaborator

arielj commented Oct 22, 2022

In the list view, the Store column shows sideload but the filter at the top is Other, it should be Other too.

@arielj
Copy link
Collaborator

arielj commented Oct 22, 2022

Maybe this is something for a future iteration, but we can run an installer with a given prefix folder and then change the prefix. I can imagine users changing the prefix by mistake like maybe changing the title after running the installer to fix a typo or something, for people not knowing what a prefix is it can be not clear that it's a problem.

I can maybe we can default to some autogenerated folder name for the prefix (hidden to the user) and allow an advanced option to set a prefix folder for the people that knows what it is, that way we show that only for users that know what it is.

@flavioislima
Copy link
Member Author

@ariel those were great catches, will work on then.
About the prefix, yes, that can be complicated. We can either disable the input after running a installer there. Or I don't know. It is a really complicated issue to solve because we have users of all levels.
Maybe adding some help texts.
We can maybe also do those in steps. Like enabling/disabling fields one by one. That is a more complicated approach but could make it more stable.
I believe it is better to test first with real users on the beta and then we can improve it.

@flavioislima
Copy link
Member Author

@LiamDawe @arielj
Did these changes and fixes that you guys have commented on here and on #1936
image

I think in the future we need to show a Dialog on Startup for new Installs with some instructions, that would make things clear.
And we should on that for next release imo.

src/backend/sideload/games.ts Outdated Show resolved Hide resolved
src/backend/launcher.ts Outdated Show resolved Hide resolved
src/backend/launcher.ts Show resolved Hide resolved
@flavioislima flavioislima dismissed Nocccer’s stale review October 24, 2022 20:14

already did the changes

@flavioislima flavioislima merged commit 1bae2c9 into beta Oct 25, 2022
@flavioislima flavioislima deleted the feat/sideload_apps branch October 25, 2022 08:25
@CommandMC CommandMC mentioned this pull request Oct 26, 2022
4 tasks
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.

None yet

8 participants