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

PC Facade Improvements: Javadoc, launch args and usage requests. #1395

Merged
merged 1 commit into from
Nov 25, 2014
Merged

PC Facade Improvements: Javadoc, launch args and usage requests. #1395

merged 1 commit into from
Nov 25, 2014

Conversation

emanuele3d
Copy link
Contributor

  • Increased class-level javadoc
  • Renamed launch argument variables for readability and consistency.
  • Now handling command-line usage requests (i.e. --help flag)

These changes were originally part of now obsolete PR #1108 and have been updated to reflect and integrate the current code. Further changes to the PC Facade had been suggested in PR #1108 but I will leave them for a separate PR.

@emanuele3d
Copy link
Contributor Author

Question: how do I test the PC Facade? That is, how do I run Terasology from command line?

@Cervator Cervator merged commit d1bfc3b into MovingBlocks:develop Nov 25, 2014
@Cervator
Copy link
Member

Thanks for the doc! Very nice :-)

I added a tiny bit to the end of the first sentence to make it clear that the PC Facade main class is for running as a PC app, as opposed to an applet or something like the AWT facade. Probably makes sense already, but anyway.

To run the game "gradlew run" is the usual go-to, but that includes compiling the code, running from source, etc. To test the command line you usually do:

gradlew distApp distModules

Which will prepare a distribution of the game in facades/PC/build/distributions/app so you can cd there and go "Terasology -homedir" to get a nice local dir for data and logs. You could of course also launch via the jar with a slightly longer line.

I tested -loadlastgame real quick, but probably should've checked out the new --help - as it actually doesn't seem to print anything for me, huh. Was able to confirm local compilation was working and everything. System.out.println doesn't seem to the printing to anything visible in that spot for some reason.

@emanuele3d
Copy link
Contributor Author

Looks good to me. Thanks for the testing/running info. I'll check and hopefully fix the problem of the --help output.

@emanuele3d emanuele3d deleted the PCFacade_NewLaunchArgumentHandling branch November 25, 2014 08:06
@emanuele3d
Copy link
Contributor Author

On second thought. Is there a page on the wiki where this info about how to run from command line is stored?

@emanuele3d
Copy link
Contributor Author

I can confirm the help doesn't get printed out.

@msteiger
Copy link
Member

It does work for me when run in eclipse with "-help", so the code is probably ok. Maybe the start script doesn't forward the command line parameters?

If that is the case, there are several possible workarounds:

  • Tweak the .bat file - difficult as it will be overwritten by gradle distZip
  • Use the TerasologyLauncher - doesn't make much sense for this
  • Use launch4j to create a nice exe with proper cmd-line handling and icons and stuff
  • Use the new launcher by @shartte (created for TS launcher) that does similar things (but better) and bundle it with TS. This probably justifies a new github issue.

Currently, the launchers work on Windows only :sad:, but that's a start.

EDIT: maybe related?
http://forums.gradle.org/gradle/topics/customize_the_generated_start_scripts_add_command_line_parameters

@emanuele3d
Copy link
Contributor Author

With "start script" do you mean the code that eventually gets converted into Terasology.exe? Where's that stuff?

@Cervator
Copy link
Member

I don't think we have anywhere that details how to launch the game beyond the basics mentioned above. It is sort of a rarely needed topic :-)

The config file that leads to Terasology.exe is https://github.com/MovingBlocks/Terasology/blob/develop/facades/PC/launchScripts/Launcher.xml but it has been used all of .. twice? Ever. We might indeed want to just wait for @shartte's update

In the meantime it is still weird the new help argument doesn't work. We've added other new arguments since the last time the .exe was generated. It might be something about how System.out / System.err is handled? I couldn't get either to output while testing

@emanuele3d
Copy link
Contributor Author

Haven't found anything so far. System.out has a setOut() method to redirect the stream, i.e. to a file. But I don't see a way to query System.out to find if that's the case.

Investigating launch4j I stumbled upon this and it is perhaps relevant: http://stackoverflow.com/questions/24694275/how-can-i-make-exe-file-to-write-text-on-the-command-line

But I am not familiar with launch4j so I still don't have a clue really.

@emanuele3d
Copy link
Contributor Author

Hmmmm... this is perhaps the problem?

http://sourceforge.net/p/launch4j/feature-requests/52/

@Cervator
Copy link
Member

Considering there was a user there who offered a fix in 2011 ... we probably won't see that fixed any time soon :-) Until we fix it via our own little utility anyway.

Maybe related to #1339 - I forgot when that started happening.

I figure it works fine if you run via the jar? java -jar Terasology.jar -help or so

@emanuele3d
Copy link
Contributor Author

Ah! Thanks for suggesting this! It both works and it doesn't. In the sense --help, -help and -h all work. But /help, /h and /? are effectively ignored, as in

java -jar Terasology.jar

which, by the way, generates a stack overflow.

I will investigate why the / is causing trouble and add the fix to the next commit in #1396.

@emanuele3d
Copy link
Contributor Author

Hmmm. Interesting. I just added a println() statement to the function handlePrintUsageRequest() to see what's coming through from the command line. With any of the dash-prefixed help arguments, the correct string is printed back.

$ java -jar Terasology.jar --help  
ARG: --help

However, with any of the forward slash prefixed arguments:

$ java -jar Terasology.jar /help
ARG: C:/Program Files (x86)/Git/help

the slash gets replaced by Git's installation directory. Any idea why is that?

My guess is that the underlying code works though.

@Cervator
Copy link
Member

Huh! :D

Are you by chance running in a Git Bash terminal while testing that? Or Cygwin, or a straight up Linux shell of some sort? You got that $ prefix going on there

I added all kinds of println and couldn't get any of it to show up when running via .exe in a normal Windows command prompt.

java -jar Terasology.jar /help

On the other hand works perfectly (including my debuf text) :)

C:\Dev\Terasology\Git\integrate\Terasology\facades\PC\build\distributions\app>java -jar Terasology.jar /help
Main!
Handling!
Handling arg /help
Printing usage
Usage:

    terasology [--help|-help|/help|-h|/h|/?] [-homedir|-homedir=<path>] [-headless]

By default Terasology saves data such as game saves and logs into subfolders of a platform-specific "home directory".
Optionally, the user can override the default by using one of the following launch arguments:

    -homedir           Use the current directory as the home directory.
    -homedir=<path> Use the specified directory as the home directory.

It is also possible to start Terasology in headless mode (no graphics), i.e. to act as a server.
For this purpose use the -headless launch argument.

To automatically load the latest game on startup,
use the -loadlastgame launch argument.

By default Crash Reporting is enabled.
To disable this feature use the -noCrashReport launch argument.

Examples:

    Use the current directory as the home directory:
    terasology -homedir

    Use "myPath" as the home directory:
    terasology -homedir=myPath

    Start terasology in headless mode (no graphics):
    terasology -headless

    Load the latest game on startup and disable crash reporting
    terasology -loadlastgame -noCrashReport

    Don't start Terasology, just print this help:
    terasology -help

@msteiger
Copy link
Member

I guess the forward slash / is interpreted by msysgit and cygwin as file system root folder which is
C:/Program Files (x86)/Git for msysgit on Windows.

I encountered the same issue on other applications that run both on Windows and Linux. For example here:

https://github.com/lisitsyn/tapkee/blob/master/src/cli/main.cpp#L49-L55

In line with that snippet, I suggest checking the OS with System.getProperty("os.name") and then use either - or / rather than both. When in doubt, the - is probably the safer choice.

@msteiger
Copy link
Member

I just realized / remembered that there is also Apache Commons CLI, JCommander and args4j that do just that (and a lot more).

Not sure if it's worth adding another dependency, but such a library is probably small and could be useful. For example, args4j can "[..] generate HTML/XML documentation listing all options".

@emanuele3d
Copy link
Contributor Author

I like your suggestion of using "os.name" Martin, but it's getting a bit
"hacky" for something that appears to be due to the particular shell
environment I'm launching from. Thinking about the typical user launching
from a standard Windows shell or even a Linux shell my guess is that we
should leave it as it is for the time being. However, I will add a comment
in the source about git bash/msysgit replacing the forward slash with the
file system root folder.

On 26 November 2014 at 08:48, Martin Steiger notifications@github.com
wrote:

I guess the forward slash / is interpreted by msysgit and cygwin as file
system root folder
which is
C:/Program Files (x86)/Git for msysgit on Windows.

I encountered the same issue on other applications that run both on
Windows and Linux. For example here:

https://github.com/lisitsyn/tapkee/blob/master/src/cli/main.cpp#L49-L55

In line with that snippet, I suggest checking the OS with
System.getProperty("os.name") and then use either - or / rather than
both. When in doubt, the - is probably the safer choice.

Reply to this email directly or view it on GitHub
#1395 (comment)
.

@emanuele3d
Copy link
Contributor Author

Yes, I normally use Git Bash on windows. And yes, running the exe just
doesn't show any of the println(), I can confirm. I think this is due to
that issue with the launch4j. On the other hand I've tested "java -jar
Terasology.jar" with both dash-based and slash-based help argument and they
both work in a standard windows shell. So, it really appears to be a
launching problem rather than a code problem. I'll add a comment to the
code. I wonder if it's worth opening an issue about it as by my
understanding all this will become obsolete when a proper launcher is in
place?

On 26 November 2014 at 05:39, Rasmus Praestholm notifications@github.com
wrote:

Huh! :D

Are you by chance running in a Git Bash terminal while testing that? Or
Cygwin, or a straight up Linux shell of some sort? You got that $ prefix
going on there

I added all kinds of println and couldn't get any of it to show up when
running via .exe in a normal Windows command prompt.

java -jar Terasology.jar /help

On the other hand works perfectly :)

C:\Dev\Terasology\Git\integrate\Terasology\facades\PC\build\distributions\app>java -jar Terasology.jar /help
Main!
Handling!
Handling arg /help
Printing usage
Usage:

terasology [--help|-help|/help|-h|/h|/?] [-homedir|-homedir=<path>] [-headless]

By default Terasology saves data such as game saves and logs into subfolders of a platform-specific "home directory".
Optionally, the user can override the default by using one of the following launch arguments:

-homedir           Use the current directory as the home directory.
-homedir=<path> Use the specified directory as the home directory.

It is also possible to start Terasology in headless mode (no graphics), i.e. to act as a server.
For this purpose use the -headless launch argument.

To automatically load the latest game on startup,
use the -loadlastgame launch argument.

By default Crash Reporting is enabled.
To disable this feature use the -noCrashReport launch argument.

Examples:

Use the current directory as the home directory:
terasology -homedir

Use "myPath" as the home directory:
terasology -homedir=myPath

Start terasology in headless mode (no graphics):
terasology -headless

Load the latest game on startup and disable crash reporting
terasology -loadlastgame -noCrashReport

Don't start Terasology, just print this help:
terasology -help

Reply to this email directly or view it on GitHub
#1395 (comment)
.

@skaldarnar
Copy link
Member

I'll just jump into this discussion with a minor question/remark: do we follow some particular guideline for command line interfaces? The Wikipedia gives a broad overview of different styles in use, here's just what I personally would prefer:

  • -h | --help for *nix systems, i.e. short hand option with -, long option name prefixed with -- (double hyphen)
  • /h | /help | /? for Windows systems, i.e. both short and long argument are prefixed with /

I've never used/seen the /-version on Linux applications, but then probably nobody would try it in the first place. I'm not sure if it could become nasty later on when introducing more and more CLI arguments and parsing them, since option prefix and path separator would be the same character.

For the existing commands we could use the following abbreviations:

  • -s | --headless with -s derived from server
  • -d | --homedir

Edit: Some reference to a programmer's exchange question (which itself contains more reference links.... 😄 ) - http://programmers.stackexchange.com/questions/70357/command-line-options-style-posix-or-what

@emanuele3d
Copy link
Contributor Author

From the somewhat fluid status of Terasology's launching apparatus and the discussions had so far I'm starting to wonder if the PC facade will ever be meant to be used via command line. In this context, while launching arguments might be reasonable, usage help printout and abbreviations might not, as the launcher might provide those facilities either via GUI or, indeed, via command line.

That been said, we could nevertheless implement the functionality you mention @skaldarnar, if only to use the PC Facade as a depot of features until the launcher takes over those responsibilities. Any other opinions on the matter?

@skaldarnar
Copy link
Member

I think with one of the aforementioned CLI libraries the handling of long and abbreviated options comes for "free". The launcher should indeed offer nicer toggles, checkboxes and co. for the different arguments, but since both are not coupled strongly having (meaningful) options(-names) is good to have.

None of this has to be addressed in this PR though, we should agree on some guideline and put it into the Wiki for later additions/refactorings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants