Review command arguments, cleanup, remove unused options, rename badly named options.

[daleglass]

There's a bunch of options to interface that are confusing, probably unused for our purposes, and badly named. We should do a review after PR #1428.

Here's an approximate list of the issues:

• Some options are in camelCase, like --allowMultipleInstances. Some are hypenated, like --fast-heartbeat.
• Some options related to the same thing are confusingly named. Eg, --scripts, --overrideScriptsPath and --defaultScriptOverride.
• Some options use confusingly named variables for their implementation in the code. The --scripts option internally uses the variable _overrideDefaultScriptsLocation, which may look like it's related to --overrideScriptsPath or --defaultScriptOverride.
• Many are under-documented. --cache is "Set test cache".
• In some cases it's not clear whether the argument is a file or a directory, and how the path will be evaluated. Eg, --traceFile takes a filename, and places it in the documents directory.
• --avatarURL and --replace-avatar-url appear to be duplicated with a priority system. This might have fulfilled some purpose for HiFi.
• Some have undocumented units, eg --clockSkew.
• In some cases, interface exits without any visible output. Eg, starting a second instance by default simply exits silently. It should probably at least write something to the log.

This doesn't need to be all fixed in one go, it probably makes sense to split it into multiple PRs and handle it over time.

[Penguin-Guru]

I think it's probably not good to create files/directories in places that aren't explicitly stated with the parameter in the web docs, --help output, and any other such places (e.g. manual/info pages). This is very likely to confuse users who won't know where to look or what to look for. It's even more problematic in cases where the assumed path may not exist on the host system, since generic directories may be created without the user's knowledge or consent.

One of these parameters, probably --traceFile, created a bunch of files on my system that have ugly names (note that this is raw ls output). When I was testing the parameter, I couldn't figure out how it worked because I had no idea where these were being produced.

• ~/Documents/�7V��~
• ~/Documents/'�x��'$'\020''V'
• ~/Documents/'��'$'\034''(0'$'\177'
• ~/Documents/'�'$'\031''*^�U'

[daleglass]

That's weird, that sounds more like uninitialized memory than anything else really.

[Penguin-Guru]

GitHub errors out even after I renamed it to "1.txt" and "1.log". The file is too big for the web services I tried to upload it to. Here are the first 20 lines (beware of GitHub's line wrapping):

[
{"cat":"trace.startup","name":"main startup","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225431025194},
{"cat":"trace.startup","name":"main startup","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225431028756},
{"cat":"trace.startup","name":"app full ctor","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225431028761},
{"cat":"trace.app","name":"notify","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225431044727},
{"cat":"trace.app","name":"notify","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225431053863},
{"args":{"nv_payload":"0"},"cat":"trace.app","name":"createOrReuseProgramShader","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440459318},
{"cat":"trace.app","name":"createOrReuseProgramShader","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225440459452},
{"args":{"nv_payload":"0"},"cat":"trace.app","name":"createOrReuseProgramShader","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440459712},
{"cat":"trace.app","name":"createOrReuseProgramShader","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225440459727},
{"args":{"nv_payload":"0"},"cat":"trace.app","name":"createOrReuseProgramShader","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440459865},
{"cat":"trace.app","name":"createOrReuseProgramShader","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225440459873},
{"args":{"nv_payload":"0"},"cat":"trace.app","name":"createOrReuseProgramShader","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440460035},
{"cat":"trace.app","name":"createOrReuseProgramShader","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225440460045},
{"args":{"nv_payload":"0"},"cat":"trace.app","name":"createOrReuseProgramShader","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440460284},
{"cat":"trace.app","name":"createOrReuseProgramShader","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225440460292},
{"args":{"nv_payload":"0"},"cat":"trace.app","name":"createOrReuseProgramShader","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440460439},
{"cat":"trace.app","name":"createOrReuseProgramShader","ph":"E","pid":32525,"tid":139633016962560,"ts":1635225440460454},
{"args":{"nv_payload":"0"},"cat":"trace.resource.parse","name":"process2DTextureColorFromImage","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440623478},
{"args":{"nv_payload":"0"},"cat":"trace.resource.parse","name":"processSourceImage","ph":"B","pid":32525,"tid":139633016962560,"ts":1635225440623489},

[Penguin-Guru]

Maybe those were produced when no file name was specified with the old parsing.

[Penguin-Guru]

I hardly use the program so I can't comment much on what parameters should be. That said, I think --clockSkew is most likely not needed and I'm not sure what effect --no-launcher is supposed to have since running the interface never shows me a launcher anyway.

As for the script parameters, I understand that exactly one script is loaded by default when the program starts. It makes sense to have a parameter to specify a different script, and perhaps some way of preventing any script from auto-loading. It should be made clear that the parameter controlling this applies to the script (singular) that is automatically run with the program. Am I correct in understanding that --overrideScriptsPath (as it is currently named) sets (1) the working directory used to load scripts on demand from Vircadia's console and (2) the default directory that is displayed when the script management window is opened? If so, it might be good to clarify that in the documentation and possibly the --help output.

[Penguin-Guru]

As far as I know hyphenated parameter names are standard on most operating systems. Camel case is a slight nuisance because it requires shifting and behaves differently depending on whether the host system is case sensitive. It's also harder to remember whether acronyms and such are supposed to be fully capitalised, whereas the hyphenated format would always be lower case.

[Penguin-Guru]

Some of these parameters might be more useful as application settings. For example, the parameter to use the system cursor. There might be some debug benefit to that but I think anyone who just prefers using their system cursor in general would prefer not having to specify the parameter every time they run the program. There may already be an application setting for this, I haven't checked.

[Penguin-Guru]

Kalila knows about a problem with at least one of the parameters. I asked her multiple times to comment here but she still hasn't so I'm leaving this as a reminder.

[digisomni]

Parameters to do with a "launcher" aren't going to be used unless it's relating to the server console.

[daleglass]

@digisomni Ok, then you're happy with this PR? Because I'd like to see it merged, it'll help with a PR I plan to submit soon.

[Penguin-Guru]

She also said that the "launcher" was not the same as the application launcher. She said it referred to some feature that was never implemented...

[Penguin-Guru]

I'm just going to remove --no-launcher and --scripts and let you all figure out the rest.

On second thought, I still don't want to touch these without more information. I will add warnings to the --no-launcher description in the --help output. These will need to be changed after merge unless someone wants to commit to my PR. Also note that someone still needs to deal with vircadia/vircadia-dev-docs#5. I don't know who administers these repos.

[stale]

Hello! Is this still an issue?