Overhaul command-line parameter functionality. #66
Conversation
|
Oh wow, this seems great. |
Yes, this fixes several of those cases. All of the ones I tested did work, but I'm not sure what some of them are supposed to do or how they are supposed to be used. Please do test and let me know if any of them don't work. :) |
|
Here are some builds you may use for testing: ubuntu-18.04, full
self-hosted_debian-11_aarch64, full
macOS-10.15, full |
| QString scriptsSwitch = QString("--").append(SCRIPTS_SWITCH); | ||
| _defaultScriptsLocation.setPath(getCmdOption(argc, constArgv, scriptsSwitch.toStdString().c_str())); | ||
| if (parser.isSet("scripts")) { | ||
| _defaultScriptsLocation.setPath(parser.value("scripts")); // Might need to be done in "main.cpp". |
There was a problem hiding this comment.
I'm going to leave this comment in until someone has confirmed that this parameter works.
| @@ -705,6 +709,8 @@ private slots: | |||
| QPointer<LogDialog> _logDialog; | |||
| QPointer<EntityScriptServerLogDialog> _entityScriptServerLogDialog; | |||
| QDir _defaultScriptsLocation; | |||
| // If above is only set by parameter, below is unnecessary. | |||
There was a problem hiding this comment.
I can try to check if it is set anywhere else, but it would take considerable time for me to be confident.
| ); | ||
| QCommandLineOption serverContentPathOption( | ||
| "serverContentPath", | ||
| "Path to find server content.", // What content?? |
There was a problem hiding this comment.
I think this description is too vague. Can someone write a more clear description?
There was a problem hiding this comment.
Hm. I am pretty sure it refers to the domain server "content". That content is what you actually see in world. Assets and their position. It doesn't contain configuration files.
There was a problem hiding this comment.
How about "path to directory containing domain assets"?
| "forceCrashReporting", | ||
| "Force crash reporting to initialize." | ||
| ); | ||
| // The documented "--disable-lod" does not seem to exist. |
There was a problem hiding this comment.
Should I remove these now or should the documentation be updated first?
There was a problem hiding this comment.
If it actually doesn't exist, I think we should remove it.
When someone implements that, it should be trivial to add this back in.
Note that currently the "LOD" system is turned off (in a fairly hacky way if I remember correctly).
| "clockSkew", | ||
| "Forces client instance's clock to skew for demonstration purposes.", | ||
| "integer" | ||
| ); // This should probably be removed. |
There was a problem hiding this comment.
I will leave this comment unless someone thinks we might want this parameter for something.
There was a problem hiding this comment.
This sounds useful for debugging actually.
Some applications run into issues when the system clock isn't reliable. For example there is a bug in Windows that has never been fixed where the clock does something like a "hiccup". On VRChat for example that breaks outbound voice data.
See discussion on original pull request here: vircadia/vircadia-native-core#1428
Also see this for possible future work: vircadia/vircadia-native-core#1461
As before, I do not know how to test all of the parameters but I did test the ones I could.