Skip to content

[Tech] Skip empty arguments in sideload and frontend command launcher #3009

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

Conversation

rdbrschf
Copy link
Contributor

If the additional command line arguments are unset/set to an empty string, the wine command launcher will likewise pass an empty argument to the launched application.

While that is usually harmless, some applications try to parse this argument and (rightfully) complain and usually exit with an error.

Make sure that we only pass additional arguments if they are actually set, fixing that issue.

This has been encountered with Palia's launcher.


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)

@arielj arielj added this to the 2.9.2 milestone Aug 30, 2023
@rdbrschf rdbrschf marked this pull request as draft September 2, 2023 00:08
@rdbrschf
Copy link
Contributor Author

rdbrschf commented Sep 2, 2023

This needs a rework, as discussed with @CommandMC on Discord. Turning into a draft for now.

@arielj arielj removed this from the 2.9.2 milestone Sep 2, 2023
@rdbrschf rdbrschf force-pushed the bugfix/winecmd-empty-arguments branch 3 times, most recently from 6e03da6 to d563dc5 Compare October 3, 2023 20:25
@rdbrschf rdbrschf changed the title [Tech] Skip empty arguments in wine command launcher [Tech] Skip empty arguments in sideload and frontend command launcher Oct 3, 2023
@rdbrschf rdbrschf marked this pull request as ready for review October 3, 2023 20:52
@rdbrschf
Copy link
Contributor Author

rdbrschf commented Oct 3, 2023

v2: modified the correct location (sideloaded launch function), modified frontend update bypass code to not add spurious whitespace characters (although the actual runners should generally handle this gracefully via shlex.split()).

…nt if falsy (empty, null, ...)

If the additional command line arguments are unset/set to an empty
string, the sideload command launcher likewise used to pass an empty
argument to the launched application.

While that is usually harmless, some applications try to parse this
argument and (rightfully) complain and usually exit with an error.

Make sure that we only pass additional arguments if they are actually
set, fixing that issue.

This has been encountered with Palia's launcher.

While at it, also fix a log line by including additional arguments and a
typo in a comment.
@rdbrschf rdbrschf force-pushed the bugfix/winecmd-empty-arguments branch from d563dc5 to 40dafdf Compare April 13, 2024 00:06
@flavioislima
Copy link
Member

flavioislima commented Apr 13, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@rdbrschf
Copy link
Contributor Author

rdbrschf commented Apr 13, 2024

I have read the CLA Document and I hereby sign the CLA.

@rdbrschf
Copy link
Contributor Author

recheck

@rdbrschf
Copy link
Contributor Author

Reworked this.

Fixed the syntax nit by using another variable as proposed, dropped the second commit (obsolete, newer code passes update skipping mechanic in a different way to the legendary component).

@imLinguin imLinguin merged commit 59a2127 into Heroic-Games-Launcher:main Aug 4, 2024
9 checks passed
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Aug 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants