Skip to content

[Fix] Sideloaded games on Windows #3562

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

Merged
merged 3 commits into from
Mar 9, 2024
Merged

[Fix] Sideloaded games on Windows #3562

merged 3 commits into from
Mar 9, 2024

Conversation

CommandMC
Copy link
Collaborator

@CommandMC CommandMC commented Feb 12, 2024

With my changes to callRunner (to use PowerShell on Windows), sideloaded games actually broke on Windows (for two different reasons!)
This fixes both of those problems (details as always in commit messages)

To test:
Well, try to run sideloaded games on Windows. Won't work on 2.13.0, will work with this PR.


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)

In case the array is empty, we have to make sure to not pass the parameter at
all (otherwise PS will error out)

This is used by sideloaded games
Sideloaded games pass their executable path as the runner. Before, this was done
 a little incorrectly. Assume the selected executable is
`C:\Windows\System32\cmd.exe`. Before, the runner would then be:
{
  bin: `C:\Windows\System32\cmd.exe`
  dir: `C:\Windows\System32\`
}

callRunner then just `join`s together these paths, resulting in
`C:\Windows\System32\C:\Windows\System32\cmd.exe`. This is obviously wrong and
will not work.

Now, we correctly pass just the bin for `bin` (`cmd.exe` in our example). This
is also in-line with how regular runners work
@CommandMC CommandMC added the pr:ready-for-review Feature-complete, ready for the grind! :P label Feb 12, 2024
@CommandMC CommandMC requested a review from a team February 12, 2024 23:56
@CommandMC CommandMC self-assigned this Feb 12, 2024
@CommandMC CommandMC requested review from arielj, flavioislima, Etaash-mathamsetty, Nocccer and imLinguin and removed request for a team February 12, 2024 23:56
@CommandMC CommandMC changed the title [Fix] Sideloaded games on Windows [Fix] Sideloaded games Feb 13, 2024
@CommandMC CommandMC changed the title [Fix] Sideloaded games [Fix] Sideloaded games on Windows Feb 13, 2024
@CommandMC CommandMC added pr:testing This PR is in testing, don't merge. and removed pr:ready-for-review Feature-complete, ready for the grind! :P labels Feb 13, 2024
This was somewhat flawed before; callRunner relied on an implementation detail
of `splitPathAndName` (it adding "./" to the "bin"). The relevant code was now
moved to callRunner itself
@CommandMC CommandMC added pr:ready-for-review Feature-complete, ready for the grind! :P and removed pr:testing This PR is in testing, don't merge. labels Feb 13, 2024
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.

LGTM

@flavioislima flavioislima merged commit 34d861c into main Mar 9, 2024
@flavioislima flavioislima deleted the fix/sideload-windows branch March 9, 2024 20:45
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Mar 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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