respect JAVA_HOME env in linux/mac launch scripts

[janAkali]

Forge should use java version set by JAVA_HOME first and if it's not set - only then use plain "java" invocation.

Also fixed warnings reported by shellcheck utility:

• SC2164 "cd command can fail.." => "cd ... || exit"
• SC2155 "local foo=..." => "local foo; foo="
• SC2086 added double quotes where word-splitting is not needed

[Hanmac]

do you have a mac to test the scripts?

can you update the forge.command script too? #5548 it still misses the version checks from the forge.sh

[janAkali]

do you have a mac to test the scripts?

No, I don't have a mac to test scripts, but the changes I made are POSIX-compliant. It should work the same with every default shell on any OS (dash, bash, zsh, etc..).

can you update the forge.command

AFAIK there's no difference between ".sh" and ".command" file extensions in MacOS. (see https://apple.stackexchange.com/questions/24262/what-is-the-difference-between-command-tool-and-sh-file-extensions) Having to support two identical launch scripts seems unnecessary. Maybe it'd be better to remove them? (question is addressed to maintainers)

[Hanmac]

do you have a mac to test the scripts?

No, I don't have a mac to test scripts, but the changes I made are POSIX-compliant. It should work the same with every default shell on any OS (dash, bash, zsh, etc..).

can you update the forge.command

AFAIK there's no difference between ".sh" and ".command" file extensions in MacOS. (see https://apple.stackexchange.com/questions/24262/what-is-the-difference-between-command-tool-and-sh-file-extensions) Having to support two identical launch scripts seems unnecessary. Maybe it'd be better to remove them? (question is addressed to maintainers)

I think MacOS reacts differently with the ".command" file extension.

so either copy the .sh to the .command file extension too, or make the .command call the .sh if able

[janAkali]

Oh I missed that you was talking about forge.command that launches the desktop client. In my previous message I meant that we can remove two ".command" scripts: forge-gui-mobile-dev/src/main/config/forge-adventure.command and forge-adventure/src/main/config/forge-adventure-editor.command because they have "*_mac.sh" files beside them that are more up to date.

But with "forge.command" it seems like it's possible to copy "forge.sh" script with minimal changes. First, the she-bang (macos uses zsh shell instead of bash):

#!/usr/bin/zsh

I've tested this script both in bash and zsh (on Linux machine) and there's only one line that would work differently in zsh:

elif [[ -n $(type -p java) ]]

type in zsh returns a string "foo not found" even if it can't find the path, but we can fix it to check the exit code instead:

elif (type -p java)

or replace it with whence that's also builtin and doesn't return anything on fail:

elif [[ -n $(whence java) ]]

[Hanmac]

it seems we can still force MacOS to use sh for the *.command file, or would that be ignored?

https://github.com/Card-Forge/forge/blob/78bff0a8f96d6e7490e0a2297a7ad29d0b4c7dc0/forge-gui-desktop/src/main/config/forge.command#L1-L3

[github-actions[bot]]

This PR has not been updated in a while nad has been marked on stale. Stale PRs will be auto closed

[Hanmac]

@tehdiplomat @Agetian @kevlahnota can someone check this MR?

[kevlahnota]

@Hanmac I think it's fine however once my PR is merged along with Java 17 update, the -illegal-access permit shouldn't work and should be removed and we need to use add opens paramater to cleanup the code.

[Agetian]

Agreed with @kevlahnota :+1: