search

dosbox-staging / dosbox-staging

DOSBox Staging is a modern continuation of DOSBox with advanced features and current development practices.
https://www.dosbox-staging.org/
Other
1.23k stars 150 forks source link

Regression when setting a `cycles` value containing a `%` sign in `[autoexec]` #3564

Closed johnnovak closed 2 months ago

johnnovak commented 3 months ago

This a regression we introduced in 0.81.0, it was working fine in 0.80.1 and earlier. Setting the same value in the [cpu] section works fine; it can only be triggered by setting it in the [autoexec].

  1. Create a dosbox.conf with the following content:
[autoexec]
cycles = auto 20000 80% limit 400000

  1. Start with dosbox --noprimaryconf

  2. Observe that the % sign was eaten by a grue and we get 80 cycles.

image
weirddan455 commented 3 months ago

First bad commit is bab4e9bc5da5adc11d3b54e4bfaeff87671e18d2 by @MeAreJeenius

It looks like the thing that was making it work before was a pretty ugly hack in a core function for reading batch files and it was intentionally removed.

johnnovak commented 3 months ago

First bad commit is bab4e9b by @MeAreJeenius

It looks like the thing that was making it work before was a pretty ugly hack in a core function for reading batch files and it was intentionally removed.

Yeah, we should be careful with such cleanups and removing stuff we don't fully understand. Often weird looking things are there for a reason.

Would you be able to take a look @MeAreJeenius ?

Actually, I will test other config settings in the autoexec that use percentages. Maybe we have a deeper problem here and the old hack was inadequate anyway.

johnnovak commented 3 months ago

@weirddan455 @MeAreJeenius I take it all back, with @MeAreJeenius 's change what we have now is the accurate MS-DOS behaviour.

Tested this on real MS-DOS, putting the same command into a batch file and the percentage disappears.

I'm strongly against reintroducing the hack, people will just need to learn not to set such things from the autoexec or batch files.

If someone is desperate to use percentage signs in batch files, %% works in both .bat files and the [autoexec] section. E.g.:

cycles = auto 20000 80%% limit 400000
viewport = relative 80%% 70%%
Grounded0 commented 3 months ago

Yeah %% is the correct MS-DOS string processing. We're here to emulate MS-DOS and trying to be backwards compatible with our string processing is only going to harm us in some other way like batch files for MS-DOS failing to run as intended.

FreeCOM 0.84-pre7 (god what a version numbering) allowed for lax % interpretation while FreeCOM 0.85 and newer only allows %% interpretation so everyone has been tightening up their ship in regards to this.

johnnovak commented 2 months ago

Closing as we emulate the MS-DOS behaviour accurately.