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] Pass additional game information to the pre-post launch script using environment variables #3748

Merged

Conversation

casasfernando
Copy link
Contributor

This PR goal is to pass additional game information (title and executable path) to the pre-post launch script using environment variables. This would allow to use the same script with different games, taking actions based on the game title value or executable path.
Tries to fulfill the alternative solution proposed in #3691.


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)

…st launch script using environment variables
@flavioislima
Copy link
Member

flavioislima commented May 12, 2024

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

@casasfernando
Copy link
Contributor Author

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

@casasfernando
Copy link
Contributor Author

recheck

@OlegAckbar
Copy link

I've just tried it and I'm pretty much content with the result. Now my scripts can create folders for recorded videos based on GAMEINFO_TITLE environment variable. Hope it will get merged so I won't have to compile Heroic every time.
изображение

@imLinguin
Copy link
Member

While I like the idea I see some issues that may occur.

  • The game title may be set to ??? for games from Amazon when their API gets a stroke
  • The gameInfo.install.executable is valid only for Epic, as other providers handle game launching based on the manifest file from the install directory.

I propose to expose variables like we did for wrappers. Making it clear that they come from Heroic (having a HEROIC_ prefix for example).
Additionaly maybe add variables that inform about the runner and game id.

@casasfernando
Copy link
Contributor Author

casasfernando commented May 12, 2024

Thanks for your comment @imLinguin.
It is true that I wrote this patch mainly for my use case (sideloaded Windows games running with Wine) so I didn't think about the points above.

  • I like the idea of changing the variables name and prefix them with HEROIC_ so if this the way to go I will change it. Just let me know.
  • gameInfo.install.executable is not only valid for Epic, as mentioned above, sideloaded games also have this property set as well. Not sure if on every case though.
  • I thought about adding variables for other information like the game id, the runner or the prefix path but I wanted to keep the PR as small as possible while at the same time maximizing the value it adds. The runner and the prefix path for example will require to pass the GameSettings object to the function (right now only GameInfo is available but correct me if I'm wrong) so it will require a slightly bigger change while I wasn't sure it worth it. The prefix path can be obtained from the executable path and right now I can't think of a good use for the game id (again my use case of Heroic is limited to running sideloaded Windows games). In any case I thought this was a good start and more variables could be added as needed, in the future.

Last but not least the variables are passed as strings so it should be fine if they are null/empty or have unexpected values. If the script is going to use them I would like to think that is up to it for validating the data but I would be very interested into hearing others thoughts on these.

PS: I'm not very familiar with Heroic's code and features and this is my first contribution so I wanted to keep it simple as possible.

@casasfernando
Copy link
Contributor Author

casasfernando commented May 12, 2024

I've checked the gameInfo object again and this is the information available in my case. I will highlight the properties that I think can be useful for pre-post launch script, but please let me know if there are other as well for other use cases and I can add them to the variables list:

  • runner
  • app_name
  • title
  • install:
    • executable
    • platform
    • is_dlc
  • folder_name
  • art_cover
  • is_installed
  • art_square
  • canRunOffline
  • browserUrl
  • customUserAgent
  • launchFullScreen

Thanks

EDIT: highlighted runner and app_name

@flavioislima flavioislima added the pr:ready-for-review Feature-complete, ready for the grind! :P label May 14, 2024
@casasfernando
Copy link
Contributor Author

@imLinguin I just pushed two more commits, one to add the HEROIC_ prefix to the variables and another to add a variable for the runner. Not sure which gameInfo property will match the game id. Maybe the app_name?

@imLinguin
Copy link
Member

Yes the app_name is a game id.

@casasfernando
Copy link
Contributor Author

Yes the app_name is a game id.

Thanks for clarifying. Should I add this on a new commit? And if yes, should be named HEROIC_GAMEINFO_APP_NAME or HEROIC_GAMEINFO_ID instead?
I think it should be the former so it's easier/faster to know where the value is coming from within Heroic.

@casasfernando
Copy link
Contributor Author

recheck

@arielj
Copy link
Collaborator

arielj commented May 17, 2024

Yes the app_name is a game id.

Thanks for clarifying. Should I add this on a new commit? And if yes, should be named HEROIC_GAMEINFO_APP_NAME or HEROIC_GAMEINFO_ID instead? I think it should be the former so it's easier/faster to know where the value is coming from within Heroic.

I would use HEROIC_APP_NAME.

Maybe we can also send the prefix for games that have a prefix? (HEROIC_GAME_PREFIX for example?) I imagine it could be useful if someone needs to reference it after a game is closed (like if you want to copy some file or something)

@casasfernando
Copy link
Contributor Author

Thanks @arielj for the input. I like the idea of adding the prefix info as well.
Will prepare a new commit for the suggestions above and will also add a variable to indicate if the script is running before or after the game. This will further facilitate using one script for both stages if the user wishes to do that.

src/backend/launcher.ts Outdated Show resolved Hide resolved
src/backend/launcher.ts Outdated Show resolved Hide resolved
@casasfernando
Copy link
Contributor Author

@arielj any chance this will be merged before the next release? Thanks

@casasfernando casasfernando requested a review from arielj May 26, 2024 13:54
@arielj arielj merged commit 00cc06a into Heroic-Games-Launcher:main May 27, 2024
9 checks passed
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators May 27, 2024
@casasfernando casasfernando deleted the pre-post-script-enhancement branch May 27, 2024 22:56
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.

6 participants