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: Implement auto-start for pre- and post-game launch #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cozyGalvinism
Copy link

@cozyGalvinism cozyGalvinism commented Jan 27, 2023

This PR implements the autostart tab with options to run a script

  • on the host before the game launches
  • in the Wine prefix before the game launches (only on Unix)
  • on the host after the game has successfully launched and the addons were loaded
  • in the Wine prefix after the game has successfully launched and the addons were loaded

The Wine options are hidden on non-Unix operating systems.

This PR relies on this PR on FFXIVQuickLauncher and cannot run without it.

The pre-game launch scripts additionally have the option to be waited on before the game launches.

An example use-case for this PR would be running either ACT or IINACT, either before or after the game has started.

An additional idea would be for a post-game script which executes after the game process has exited for cleaning up.

A screenshot of the tab:

image

@tatsumara
Copy link

i would really love this, just commenting to maybe bump it a little (:

@goaaats
Copy link
Member

goaaats commented Mar 26, 2023

I don't have the capacity to review XLCore stuff anymore, so I'll probably have to hand this stuff off. Maybe @BitsOfAByte wants to take it?

@Blooym
Copy link
Contributor

Blooym commented Mar 26, 2023

I don't have the capacity to review XLCore stuff anymore, so I'll probably have to hand this stuff off. Maybe @BitsOfAByte wants to take it?

I should have some time to review & test this within a week or two

@cozyGalvinism
Copy link
Author

I probably need to either move the Run function to its own interface (though having 2 interfaces with one function each sounds stupid and looks stupid too) or I need to rename it to make it more clear what it does (as opposed to Start). I am open for input and ideas for that.

Copy link
Contributor

@Blooym Blooym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be beneficial to rephrase pre-game & post-game to pre-launch and post-launch for clarity as post-game sounds more like a script that would run on exit.

Additionally, I believe scripts should handle running things inside of wine instead of us doing it for them, although I'm not sure how others feel about this and I'm happy with the idea if it would make things easier for others.

Otherwise LGTM

@cozyGalvinism
Copy link
Author

It would be beneficial to rephrase pre-game & post-game to pre-launch and post-launch for clarity as post-game sounds more like a script that would run on exit.

Agreed, will do that after work today.

Additionally, I believe scripts should handle running things inside of wine instead of us doing it for them, although I'm not sure how others feel about this and I'm happy with the idea if it would make things easier for others.

I don't know about this. Since I hang around in the IINACT Discord a lot to help people getting it to run (before it turned into a plugin), I experienced first hand just how many steps and hurdles there are for people run something inside the correct prefix and the correct Wine version. Running it over XL alleviates those issues a little, though it takes away some control.

Example: If you want to use imgoverlay, you would set IMGOVERLAY=1 for the game process but not for the additional tools. If XL runs scripts directly in Wine, the only way to modify the environment variables so far is to set them prior to launching XL, so you wouldn't be able to do this and both IINACT (as example here) and XL would get IMGOVERLAY=1.

One compromise I could think of is to expose environment variables "evaluated" from XL, like setting XL_WINE to the configured Wine path of XL, XL_WINEPREFIX to the prefix etc specifically for the scripts or maybe even directly setting WINEPATH and WINEPREFIX for them.

@Blooym
Copy link
Contributor

Blooym commented Apr 3, 2023

One compromise I could think of is to expose environment variables "evaluated" from XL, like setting XL_WINE to the configured Wine path of XL, XL_WINEPREFIX to the prefix etc specifically for the scripts or maybe even directly setting WINEPATH and WINEPREFIX for them.

I like the idea of scripts having access to certain environment variables when being run by XL as it saves us from having to run anything directly in-prefix. If we go down this route I don't think it's unreasonable to directly set WINEPREFIX and WINEPATH for the script - could even expose additional variables using this method (like config paths) later on.

@sersorrel
Copy link
Contributor

sersorrel commented Apr 3, 2023

Running things inside the prefix programmatically is super frustrating. If you're doing anything nontrivial, you have to (aiui) get things like the environment variables to configure DXVK and esync/fsync correct and guess the right path to the wine binary xlcore is going to use, and the documentation for that is basically "a single pin in #linux-and-deck from 2022".

If I just want to e.g. "run the wine-discord-ipc-bridge exe before the game starts", I don't want to have to write a script to do that, I want to click a button, select "wine" and "before startup" in a dropdown, and double-click the exe...

@Blooym
Copy link
Contributor

Blooym commented Apr 4, 2023

and the documentation for that is basically "a single pin in #linux-and-deck from 2022".

This is a problem way beyond just this feature, I do admit we need to do better on documentation for XLCore.

If I just want to e.g. "run the wine-discord-ipc-bridge exe before the game starts", I don't want to have to write a script to do that, I want to click a button, select "wine" and "before startup" in a dropdown, and double-click the exe...

Exposing environment variables to the scripts that provide wine information would make running things easy as it wouldn't require guessing prefix locations and it allows for more control over the auto launch experience, at most it'd be 2-3 lines in a script if you don't need to run anything else.

@cozyGalvinism
Copy link
Author

If I just want to e.g. "run the wine-discord-ipc-bridge exe before the game starts", I don't want to have to write a script to do that, I want to click a button, select "wine" and "before startup" in a dropdown, and double-click the exe...

I know it's not what you want, but you will always be able to do more in a script than with a UI. If I now add a table for environment variables, the next thing is going to be specifying arguments, then a delay before/after launch, then being able to run multiple programs one after another but some others can start at the same time. And maybe even relations, so one software dies when another is shut down... Goes on and on.

All of these things, you can already do with just a single script and with so much more control over what runs where and how. It is not ideal from a usability standpoint, but it is for simplicity. Also for sanity, because spawning processes like these with a build-up like from a UI make it increasingly more difficult to debug.

@tatsumara
Copy link

i agree, as much as a UI for every little thing would be nice i'd say it's very out of the scope of this launcher. a script might be a little user unfriendly, but it's easy enough to just write a good script once and share it around, so i think this is a really good compromise.

@sersorrel
Copy link
Contributor

Also for sanity, because spawning processes like these with a build-up like from a UI make it increasingly more difficult to debug.

you neglect the impact on my mental health from having to write more bash :p

but yeah, fair. if you can only set one path to a script, though, why make it configurable at all? why not just run ~/.xlcore/pre-launch and ~/.xlcore/post-launch, if they exist? you don't need the "wait for script" checkbox either, you can just unconditionally wait for the script to exit (potentially with some ui that appears after a while saying "your pre-launch script is taking a while, did you forget an &" or something, if you want to go to the effort) and the script can just background anything that doesn't need waiting for.

@marzent
Copy link
Contributor

marzent commented Apr 4, 2023

I probably need to either move the Run function to its own interface (though having 2 interfaces with one function each sounds stupid and looks stupid too) or I need to rename it to make it more clear what it does (as opposed to Start). I am open for input and ideas for that.

Alternatively this could also be implemented as IRunnableAddon similar to https://github.com/goatcorp/FFXIVQuickLauncher/blob/master/src/XIVLauncher.Common/Addon/Implementations/GenericAddon.cs, just for unix-like systems (especially since the consensus seems to pivot towards using a script)

@Blooym Blooym added the enhancement New feature or request label Jun 16, 2023
@Blooym Blooym linked an issue Oct 3, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Auto-Launch Wine Application With FFXIV In Prefix
6 participants