VPX should abort when invalid options are passed on the command line

[dekay]

VPX happily runs when invalid options are passed on the command line. I don't think I even got a warning, not that I'd have seen it anyway because of the hundreds of INFO messages that print to the terminal when a table starts up. I think it would be better to just abort instead if an unknown option is passed.

I found this on the standalone branch running Linux but I expect the same thing would happen when running from the Windows terminal.

To Reproduce

1. From the command line, pass an invalid parameter like so (note the typo in "Truee")

./VPinballX_GL -DisableTrueeFullscreen -Play <tablename>

2. VPX will happily run with the default full screen display without an indication that an option I passed was not valid

Expected behavior VPX should have printed a message that the command line option wasn't recognized and aborted.

Versions & Settings

• VPX: v10.8.0-1347-3d585e7-99-g75084d31 (standalone branch)
• Arch Linux

[francisdb]

And preferably also non-0 exit codes in case of errors And startup failing if the rom can not be found

[toxieainc]

The rom part is difficult as this is done via VBS<->VPinMAME. So it would have to be done in the core.vbs somehow and then fed back into VPX.

[francisdb]

Thanks. Any reason to have the vpinmame interfacing in vbs instead of vpinball?

[nailbuster]

@toxieainc

This change will break every default vpx launch in Popper. It needs to be reversed or backward compatible. It stops loading with error dialog about the vpx-folder (1st parameter)

Heres a sample of everyones launch script today in Popper:

vpinballx.exe "C:\vPinball\visualpinball" -DisableTrueFullScreen -minimized -play "C:\vPinball\visualpinball\Tables\Leprechaun King (Orbital 2020).vpx"

Its been like this for 5+ years. I assume its the first parameter for exe vpxdirectory its not liking?

another problem with this: i think its another issue# in bulid. if you make a bat file and just do this:

vpinballx64.exe -play "D:\visual pinball\Tables\Leprechaun King (Orbital 2020).vpx"

you get an error that vpinballx64 is an invalid parameter.

both above examples work fine in older builds...

[jsm174]

Its been like this for 5+ years. I assume its the first parameter for exe vpxdirectory its not liking?

My 2 cents, I personally like this new change. Allowing unneeded or invalid params to pass through is just bad practice.

C:\vPinball\visualpinball may have been ignored for the past 5 years, but to make a proper command line parser allow for this one use case, idk..

[nailbuster]

Its been like this for 5+ years. I assume its the first parameter for exe vpxdirectory its not liking?

My 2 cents, I personally like this new change. Allowing unneeded or invalid params to pass through is just bad practice.

C:\vPinball\visualpinball may have been ignored for the past 5 years, but to make a proper command line parser allow for this one use case, idk..

one use case? You believe it makes sense to break 10,000+ users popper/vpx setup for this one 'feature'? A low-requirement feature. I guess we can just tell users to modify their launch script for the next 12 months and 1,000 posts about it.

[jsm174]

@nailbuster - if 10.8 was already officially released than of course wouldn't want to break 10,000+ users. Again it was just a 2 cents...

[LynnInDenver]

I would recommend this be like the "ignore certain errors and run anyway" setting with VPinMAME. Basically set it up so that the default is "error out", and then give us the option to turn off the error condition alert versus changing the front end loading script.

I do get that VPX should always have basically errored out in invalid parameters, but we're now stuck with years of a legacy, and recent other events with other software in the community shows how long issues can linger when a change is made to how things are done.

[LynnInDenver]

Also, it might be a good idea for VPX to actually tell us which specific parameter caused the error, and why. Giving a list of valid parameters is good, but a specific error of "parameter %EXPLOIT% is not valid", "argument of parameter %EMUDIRECTORY% is missing", "command string position of parameter %FSMODE% incorrect" or "mode %DEPREC% is no longer supported" would be better.

[nailbuster]

IMO, the best solution would be to treat/check only parameters that start with the '-' or '/'. All others parameters that don't have '-' like filename and this emufolder parameter or even the custom values -c1 "something" should be skipped for validity.

note to verify: I assume -c1 ... -c9 are still valid parameters.

[LynnInDenver]

IMO, the best solution would be to treat/check only parameters that start with the '-' or '/'. All others parameters that don't have '-' like filename and this emufolder parameter or even the custom values -c1 "something" should be skipped for validity.

note to verify: I assume -c1 ... -c9 are still valid parameters.

Again, I can see what's going on... the whole request to "MUST ABORT ON BAD PARAMETERS" is that the best security practice is considered to be "if there's something wrong, fail to stay safe". The idea of a setting to not abort would be to ignore malformed parameters anyway but still run, which is less safe. Of course, we're talking software that leans heavily on the COM system of Windows.

[toxieainc]

IMO, the best solution would be to treat/check only parameters that start with the '-' or '/'. All others parameters that don't have '-' like filename and this emufolder parameter or even the custom values -c1 "something" should be skipped for validity.

This was also my idea. This way we could (hopefully) not break other legacy stuff.

[toxieainc]

Thanks. Any reason to have the vpinmame interfacing in vbs instead of vpinball?

Its by design. Getting the state of lamps, solenoids, motors, etc, is then processed by script/author code. Of course one could do an additional indirection VPM->VPX->VBS, but there is no real major usecase that would justify such a breaking change.