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

Feature Request: Command-line Arguments when Launching Games via Protocol #4182

Closed
Sewer56 opened this issue Dec 16, 2024 · 11 comments · Fixed by #4186
Closed

Feature Request: Command-line Arguments when Launching Games via Protocol #4182

Sewer56 opened this issue Dec 16, 2024 · 11 comments · Fixed by #4186
Labels
feature-request New feature needs to be implemented.

Comments

@Sewer56
Copy link

Sewer56 commented Dec 16, 2024

Hey there, Heroic team! 👋

Some of you are familiar already; but for the others. I'm one of the folks working on
the Nexus Mods app (a Cross Platform, GPLv3 Mod Manager) funded by Nexus Mods.

And, a while back we've added support for GOG games by integrating with Heroic;
in the hopes to make modding on Linux more accessible to a typical user.

That said, thank you for all your work.

What's missing?

It seems there's currently no way to pass command line arguments when launching
games through Heroic.

Looking at src/backend/protocol.ts#L104,
which I believe is the entry point, it seems the heroic protocol doesn't support this yet.

Why we need this

When we launch games from within the App, we actually request the store / user's installation method to start the process
whenever possible. People hitting 'launch game' via the App actually request Heroic itself to launch it (heroic://launch/).

Launching via Heroic ensures the end user's preferences for things like WINEPREFIX, Wine/Proton version etc. are respected.

Some games however may need specific command line arguments to work properly with mods.
Some good examples:

  • Mount & Blade II: Bannerlord: We need to specify the mods to be loaded via commandline args; and whether to boot singleplayer/multiplayer.
    • If the user has BLSE installed, we need to pass /forcenetcore to be able to boot the game at all!!
  • Cyberpunk 2077: If the user is using REDmod (CDPR's 1st party mod loader/tool), we need to pass the set of mods to 'deploy' (compile) to their CLI.

What we're thinking

It would be helpful if it were possible to pass additional launch arguments when starting games through Heroic's protocol.

Currently the protocol uses the following form:

heroic://launch/{storeType}/{gameId}

It could be extended to something like:

heroic://launch/{storeType}/{gameId}/{commandLineArgs}

With the commandLineArgs section being optional.
Or alternatively:

heroic://launch/{storeType}/{gameId}?args={commandLineArgs}

We're happy to help out with this if you need any support from us - just let us know!
And of course, thanks for your time 🤞

@Sewer56 Sewer56 added the feature-request New feature needs to be implemented. label Dec 16, 2024
@CommandMC
Copy link
Collaborator

CommandMC commented Dec 19, 2024

Hey there, I've just opened a PR to resolve this issue. Please check it out, feel free to ask (either here or on Discord) if there are any questions.

I should note that if you want to pass multiple arguments, you will need multiple arg parameters. Splitting arguments automatically on whitespace is purposefully not done (to make it possible to specify /paths/with spaces/ as one argument for example)

@Sewer56
Copy link
Author

Sewer56 commented Dec 19, 2024

Acknowledged. Thank you.
Currently at the end of the day (on a yearly party), and will be travelling tomorrow.

I may have a quick look afterwards around Saturday or late Friday. But it sounds good to me 👍

@CommandMC
Copy link
Collaborator

Oh, no rush at all! Have a good one

@Sewer56
Copy link
Author

Sewer56 commented Jan 8, 2025

Hello again.
Did everyone enjoy their New Year?

Since it was Christmas and all, I decided in the end to wait a while to report back. I figured the folks here most likely develop the launcher at the own time out of fun and passion; in the same way, a bunch of us (myself included) do mod stuff, after hours.

I didn't want anyone to feel pressured (as there's a slightly bigger entity behind me), so I decided to wait till everyone's had their fun and are well rested.

In any case, I had a quick go with the PR yesterday afternoon; here's some quick impressions.


Works Like a Charm

Passing the actual parameters works like a charm, pretty much solid there.

Integration against the PR took around 10 minutes, I had to change the signature of an API
on my end and add a specialised endpoint:

if (install.Store == GameStore.GOG 
    && install.LocatorResultMetadata is HeroicGOGLocatorResultMetadata gogLocatorResultMetadata)
{
    // Assemble the new format Heroic launch string.
    // TODO: Support non-GOG stores, currently GameFinder doesn't detect these, so we have
    // no way to write the code with alternate runner.
    var idAsNeutralString = gogLocatorResultMetadata.Id.ToString(CultureInfo.InvariantCulture);
    var baseString = new StringBuilder($"heroic://launch?&appName={idAsNeutralString}&runner=gog");
    foreach (var argument in arguments)
    {
        baseString.Append("&arg=");
        baseString.Append(argument);
    }
    
    // Note(sewer):
    // We must not canonicalize else Uri injects a backslash after `launch` in string, breaking the command.
    var uri = new Uri(baseString.ToString(), new UriCreationOptions()
    {
        DangerousDisablePathAndQueryCanonicalization = true,
    });
    // runs `uri` with `xdg-open`
    var process = await _osInterop.OpenUrl(uri, fireAndForget: false, cancellationToken: cancellationToken, logOutput: true);
}

C# for reference. This was fairly simple.

In C# I had to specifically set DangerousDisablePathAndQueryCanonicalization, when formatting the protocol as an Uri (which is what the existing code expected) on my end, a backslash would be injected like this heroic://launch/?&. Threw me off for a short bit (another 20 minutes or so), but aside from that, easy peasy.

image

Heroic GUI Spawn

Launching through the protocol spawns the GUI, with no proper way to currently suppress this (I also reviewed the code in the PR, as another set of eyes).

It's a very minor, low priority convenience thing but I imagine some end users down the road may want to suppress the GUI from popping up in front. I haven't looked how difficult it would be to do currently.

Say you're on a tiling WM, clicking 'launch' in an external application may bring up 2 windows instead of 1, which is a problem in the case that a launched game does not go fullscreen. Only a very minor thing.

AppImage Didn't Register heroic: protocol.

To test, I actually had to manually add a .desktop file on an Arch-based (specifically CachyOS) system:

[Desktop Entry]
Name=Heroic Games Launcher
Exec=/home/sewer/Downloads/Heroic-2.15.2-linux-x86_64.AppImage %U
Terminal=false
Type=Application
Icon=heroic
StartupWMClass=Heroic
Comment=Open Source GOG and Epic Games launcher
MimeType=x-scheme-handler/heroic;
Categories=Game;

The AppImage from CI wasn't producing one for me with Failed to register protocol with OS in log.

I haven't investigated why but if it was trying to write to usr/share/applications it'd probably be permissions as I was running as regular user. I would have expected it to throw something in .local/share/applications though.

I mention this as an end user pulling an AppImage from GitHub Releases may encounter this; then the protocol would fail to kick in if used. This also seems to appear commonly in logs uploaded to the web, but I haven't seen an issue for this. Let me know if this is known or not; might be worth opening an issue for.


In any case, all is good 👍

On a slightly unfortunate note; I only noticed something yesterday.

There is actually another requirement, the ability to run an arbitrary binary via the WINEPREFIX belonging to the game. I forgot to state this in the original post, but it applies to both the original examples I gave. A binary path could be expressed either as a relative path or an absolute one relative to game folder root; I have no preference.

I noticed stdout mentioning umu (formerly ULWGL), so assuming that's what's being used under the hood, we'd be dealing with invoking umu-run or the equivalent of that via API.

Please remember, no rush there. In my case, the higher up powers that be really, really want focus on Stardew Valley for a game focused Beta-ish release which doesn't require this; but I didn't want to make anyone here wait; out of mutual respect for the effort everyone here puts in. Afterwards is Cyberpunk and Bannerlord, I'm pretty sure I could find a good excuse to PR to Heroic if needed :p.

If you folks also would want me on the Discord, or something of the kind, let me know.

@CommandMC
Copy link
Collaborator

Heroic GUI Spawn

Ah, I see what has gone wrong here. I did actually remove GUI hiding code in that PR, thinking it was unused.
Heroic itself doesn't actually ever use the protocol, instead starting itself with the protocol string (/path/to/heroic/binary heroic://...). It then also includes the --no-gui parameter to disable the GUI from popping up on startup. I assume we want to make that the default if started via protocol

AppImage Didn't Register heroic: protocol.

That's (I believe) one of the reasons Heroic doesn't use the protocol.
We don't really do anything special to register it:

if (process.env.CI !== 'e2e' && !app.isDefaultProtocolClient('heroic')) {
if (app.setAsDefaultProtocolClient('heroic')) {
logInfo('Registered protocol with OS.', LogPrefix.Backend)
} else {
logWarning('Failed to register protocol with OS.', LogPrefix.Backend)
}
} else {
logWarning('Protocol already registered.', LogPrefix.Backend)
}

app.setAsDefaultProtocolClient is an Electron function that should handle everything for us (but evidently it's not working in this case).

I'll look into why that's not working. For now, honestly, most people don't use the AppImage (at least as their main Heroic installation), so it's gonna work fine for most users.

There is actually another requirement, the ability to run an arbitrary binary via the WINEPREFIX belonging to the game. I forgot to state this in the original post, but it applies to both the original examples I gave

I see, that shouldn't be too hard (good thing someone made the new protocol format easily expandable). Might end up doing that in another PR though

@Sewer56
Copy link
Author

Sewer56 commented Jan 8, 2025

app.setAsDefaultProtocolClient is an Electron function that should handle everything for us (but evidently it's not working in this case).

Yeah, chances are user will get it from their package manager, but there's always that edge case that'll show up.

Worst case scenario, we can always just manually write it to .local/share/applications XDG_DATA_HOME/applications.
What I have above can be used as template, and then you'd just string substitute the path to the AppImage.

You normally can get the path to the non-mounted AppImage file via the APPIMAGE Environment Variable.
Whether a build is an AppImage build could either be set at build time in-script (usually better approach), or (as a much hackier, questionable, but might still be okay way) by just checking if at least 2 of above environment variables exist.

@Etaash-mathamsetty
Copy link
Member

Etaash-mathamsetty commented Jan 9, 2025

Heroic GUI Spawn

Ah, I see what has gone wrong here. I did actually remove GUI hiding code in that PR, thinking it was unused. Heroic itself doesn't actually ever use the protocol, instead starting itself with the protocol string (/path/to/heroic/binary heroic://...). It then also includes the --no-gui parameter to disable the GUI from popping up on startup. I assume we want to make that the default if started via protocol

AppImage Didn't Register heroic: protocol.

That's (I believe) one of the reasons Heroic doesn't use the protocol. We don't really do anything special to register it:

if (process.env.CI !== 'e2e' && !app.isDefaultProtocolClient('heroic')) {
if (app.setAsDefaultProtocolClient('heroic')) {
logInfo('Registered protocol with OS.', LogPrefix.Backend)
} else {
logWarning('Failed to register protocol with OS.', LogPrefix.Backend)
}
} else {
logWarning('Protocol already registered.', LogPrefix.Backend)
}

app.setAsDefaultProtocolClient is an Electron function that should handle everything for us (but evidently it's not working in this case).

I'll look into why that's not working. For now, honestly, most people don't use the AppImage (at least as their main Heroic installation), so it's gonna work fine for most users.

There is actually another requirement, the ability to run an arbitrary binary via the WINEPREFIX belonging to the game. I forgot to state this in the original post, but it applies to both the original examples I gave

I see, that shouldn't be too hard (good thing someone made the new protocol format easily expandable). Might end up doing that in another PR though

I don't think it's possible to register an app image as a url handler because the url handler executable is meant to be something more permanent (and not something that can be easily removed like an app image)

@Sewer56
Copy link
Author

Sewer56 commented Jan 9, 2025

I don't think it's possible to register an app image as a url handler because the url handler executable is meant to be something more permanent (and not something that can be easily removed like an app image)

Many years ago, I shipped (and still do ship) a piece of software with a similar problem on Windows. A user can place it anywhere, move it at any time, and (in some rare cases) even have multiple copies.

In my case, I simply opted to update the path to the handler on every boot, if necessary. The main problem really is cleaning up after the user removes the AppImage; but that's also true with config files in any piece of software in general. Removing a handler would just be part of any existing cleanup logic.

Note: Delayed response by 1h because GitHub was having site issues stopping me from commenting.

@Etaash-mathamsetty
Copy link
Member

I don't think it's possible to register an app image as a url handler because the url handler executable is meant to be something more permanent (and not something that can be easily removed like an app image)

Many years ago, I shipped (and still do ship) a piece of software with a similar problem on Windows. A user can place it anywhere, move it at any time, and (in some rare cases) even have multiple copies.

In my case, I simply opted to update the path to the handler on every boot, if necessary. The main problem really is cleaning up after the user removes the AppImage; but that's also true with config files in any piece of software in general. Removing a handler would just be part of any existing cleanup logic.

Note: Delayed response by 1h because GitHub was having site issues stopping me from commenting.

Windows works differently than Linux :p

@Sewer56
Copy link
Author

Sewer56 commented Jan 9, 2025

Windows works differently than Linux :p

The problem is the same however:

  • I have a consistent location to register a protocol handler to.
    • (Register) For current user only.
    • Application itself is also (usually) in a user specific directory, i.e. a child of $HOME. (👈 this doesn't really matter, but just for context)
  • I have a binary that can be moved at any time.

People will always have different opinions for everything, but in my personal opinion at least, when a user installs an application like Heroic they should expect it to 'just work'.

Should a user be one of the rare few that decides to install the AppImage, be it because:

  • They want a binary directly from upstream
  • Their distro shipped package is outdated.
  • For testing.

Or for any other reason, their experience as a user should be comparable. If (as an alternative example), someone made a browser extension that allows you to start downloads from GOG within a browser, and Heroic had a protocol call to support this, it should be expected that this works out of the box after install, regardless of the user's install method.

It is unreasonable to ask that users manually have to write .desktop files, or have to read some subtext in a not easy to find location telling them how to fix it. Having additional steps and barriers to accessibility only makes users more frustrated and wider adoption of the Linux desktop harder to achieve.


There are obviously some minor caveats, e.g. what if a user has both package manager installed and AppImage installed Heroic, but there isn't much a tiny bit of code can't fix.

@Etaash-mathamsetty
Copy link
Member

I guess the actual issue then is that electron doesn't know how to create the desktop file, and i think it's actually generated beforehand by electron-builder, so this should be fixed electron side in that case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature needs to be implemented.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants