Fix support for Windows and macOS #44
Conversation
| chmod +x ${{ github.action_path }}/steam_deploy.sh | ||
| ${{ github.action_path }}/steam_deploy.sh | ||
| chmod +x $GITHUB_ACTION_PATH/steam_deploy.sh | ||
| $GITHUB_ACTION_PATH/steam_deploy.sh |
There was a problem hiding this comment.
This is necessary for the path to be Linux-style when running on Windows.
| manifest_path=$(pwd)/manifest.vdf | ||
| contentroot=$(pwd)/$rootPath | ||
| if [[ "$OSTYPE" == "darwin"* ]]; then | ||
| steamdir="$HOME/Library/Application Support/Steam" |
There was a problem hiding this comment.
Honestly, seems like a bug in Steam that steamcmd clobbers your main Steam app authentication on macOS.
| steam_username: ${{ inputs.username }} | ||
| steam_password: ${{ inputs.password }} |
There was a problem hiding this comment.
Why the breaking change?
The action is called steam-deploy. Do the variables really need steam_ in them?
If not absolutely needed I'd prefer to not break for existing users.
There was a problem hiding this comment.
I didn't think it was a breaking change, since these values come from the inputs which haven't changed their name. The reason is it appeared somewhere in the Windows bash environment $username was getting set to empty string, because this is a variable name some shells automatically set to the current user or otherwise reserve.
There was a problem hiding this comment.
I see. Thanks for the explanation.
There was a problem hiding this comment.
Generally speaking we should be converting all these "env" to "with" variables. That way they'll be prepended with INPUT_ which effectively solves this category of problem. But out of scope for this issue.
| steam_username: ${{ inputs.username }} | ||
| steam_password: ${{ inputs.password }} |
There was a problem hiding this comment.
I see. Thanks for the explanation.
There was a problem hiding this comment.
Tested with my app and it failed: https://github.com/finol-digital/Card-Game-Simulator/actions/runs/3109277871/jobs/5040233833
I think for the linux runner, we really need the "Copying /home/runner/Steam/ssfn...".
@wilg Can you bring that back for linux and I'll test again?
Also, all the logs go to /home/runner/Steam on Linux, so I think we should use that path instead of $steamdir when displaying the errors, as most users have linux runners.
|
Oh interesting. I wonder why that is. I'll investigate and fix when I try to get Linux up and running on my end hopefully sometime soon. |
There was a problem hiding this comment.
Tested and confirmed that it is working for Linux again with elif [ "$RUNNER_OS" = "Linux" ]; then at https://github.com/finol-digital/Card-Game-Simulator/actions/runs/3123780055
@wilg Can you test with your mac and windows once more to confirm we didn't break there?
Then we should be good to merge this.
|
Yep, still working on Mac and Windows |
Should probably be squashed as I was losing my mind implementing this, but this should allow this action to work on Windows and Mac, at least in my testing. It shouldn't break anything on other platforms.
Basically just changing hardcoded paths, converting paths to Windows paths (ugh), and on Windows
$USERNAMEwas getting clobbered.I removed the hardcoded, duplicate copies to
/home/runner/Steam. I don't think they were needed, but I haven't tested this on Linux.