dosbox-staging icon indicating copy to clipboard operation
dosbox-staging copied to clipboard

Emulated joystick keyrepeat related regression

Open johnnovak opened this issue 2 years ago • 14 comments

  1. Download the repro pack CPCE.zip (it contains the free CPCE Amstrad CPC emulator).

  2. Unpack & start with dosbox --noprimaryconf.

  3. Press any key when the about dialog appears at startup.

  4. First issue: The blue CPC startup screen will appear, but a "down arrow" symbol will be being auto-typed, followed by multiple "right arrow" keys on key repeat. This will only stop if you press some keys (press enter twice).

  5. ~Second issue: Press the "A" key – this will type aX. Press Space then press the "S" key — this will type sZ. Every other key behaves correctly as far as I can tell; only the "A" and "S" keys are affected, which is super perplexing.~ (This is not a regression; my joy 1 buttons were mapped to the A and S keys and I forgot about it.)

The below screenshot illustrates the problem. Bisecting revealed the regression was introduced by the 731547af978e108674f2a482c8ced42fd64dc68a joystick handling related change.

@kcgen looks like you were involved with this:

  • https://github.com/dosbox-staging/dosbox-staging/issues/2474
  • https://github.com/Nopey/dosbox-staging/commit/6374091fec637696ee5fe8d5b13e051cc58fa527

@Nopey this was your change, if you could look at it and fix the regression that would be appreciated 😄

image

johnnovak avatar Sep 21 '23 09:09 johnnovak

Ha! Bisected to this. SDL is off the hook... this time 😅

731547af978e108674f2a482c8ced42fd64dc68a is the first bad commit
commit 731547af978e108674f2a482c8ced42fd64dc68a
Author: Magnus Larsen <[email protected]>
Date:   Mon May 22 06:00:37 2023 -0700

    Create and update joy binds even without physical

    This has been broken since my change (#154) in 2019.

    Emulated joysticks still have issues with key-repeat, but this is a
    seperate issue (was not caused by my 2019 PR)

    See issue #2474

 src/gui/sdl_mapper.cpp |  3 +--
 src/gui/sdlmain.cpp    | 14 ++++++--------
 2 files changed, 7 insertions(+), 10 deletions(-)

johnnovak avatar Sep 21 '23 10:09 johnnovak

The plot thickens: the second issue happens because the buttons of joy 1 were mapped to "A" and "S" in my config, respectively. The first issue is still a regression, though—that happens even with the default mappings, so the joy 1 buttons unmapped.

Updating the ticket, plus I think we should have another command line option to ignore any mapperfile as random mappings can also throw off testing and troubleshooting.

johnnovak avatar Sep 21 '23 10:09 johnnovak

another command line option to ignore any mapperfile

FYI:

  • just sharing, I know you're not replicating X:

-defaultmapper : Uses the default key mappings for DOSBox-X (do not try to load a mapper file such as mapper-dosbox-x.map).

  • #342

Torinde avatar Sep 21 '23 12:09 Torinde

another command line option to ignore any mapperfile

FYI:

* just sharing, I know you're not replicating X:

-defaultmapper : Uses the default key mappings for DOSBox-X (do not try to load a mapper file such as mapper-dosbox-x.map).

* [DOSBox merging joysticks #342](https://github.com/dosbox-staging/dosbox-staging/issues/342)

That's not a bad name. Although for consistency with noprimaryconf and nolocalconf I think nomapperconf would be perhaps better.

johnnovak avatar Sep 21 '23 12:09 johnnovak

I did some testing on this. https://github.com/dosbox-staging/dosbox-staging/commit/731547af978e108674f2a482c8ced42fd64dc68a is a bit of a red hearing. The commit does exactly what it says it does. It fixes emulated joysticks. I checkout out the parent commit https://github.com/dosbox-staging/dosbox-staging/commit/2143e19953a8eccd5deb97a92c5c423f526dce7c and it has the same bug when a real joystick is plugged in (tested with an Xbox One controller).

weirddan455 avatar Sep 22 '23 06:09 weirddan455

I did some testing on this. 731547a is a bit of a red hearing. The commit does exactly what it says it does. It fixes emulated joysticks. I checkout out the parent commit 2143e19 and it has the same bug when a real joystick is plugged in (tested with an Xbox One controller).

Well, but on macOS with that commit I get the weird keyrepeat behaviour, with the one before it's all good. I don't have any joysticks or controllers plugged in.

johnnovak avatar Sep 22 '23 08:09 johnnovak

Well, but on macOS with that commit I get the weird keyrepeat behaviour, with the one before it's all good. I don't have any joysticks or controllers plugged in.

Right and the same thing happens on Linux when I unplug my controller. https://github.com/dosbox-staging/dosbox-staging/commit/731547af978e108674f2a482c8ced42fd64dc68a simply emulates a joystick being plugged in so now it behaves the same way.

When I have a real joystick plugged in, I get the exact same behavior you mentioned both before and after that commit. Same arrow patterns and pressing a key on the keyboard or on the gamepad stops the repeat.

The arrows can be controlled by the left stick on my Xbox controller and once you move the stick, the repeat bug starts up again.

It's strange because I see no such behavior in this joytest program. The X and Y axis stay perfectly centered at 128/128 when I'm not touching the stick (and so does the "emulated" joystick when I unplug my controller). https://www.vogons.org/viewtopic.php?p=187168#p187168

weirddan455 avatar Sep 22 '23 08:09 weirddan455

When I have a real joystick plugged in, I get the exact same behavior you mentioned both before and after that commit. Same arrow patterns and pressing a key on the keyboard or on the gamepad stops the repeat.

Ok, so maybe a bug in the CPCE emulator? In any case, it seems like a minor issue (I can just press Enter twice at startup or after a CPC reset with Ctrl+F5).

Btw, the 16-bit real-mode version is also included in the pack (CPCE.EXE), and that one doesn't exhibit the problem. Just make sure to set cycles = 200000 before starting it, otherwise it will take forever at 3000 cycles (it requires a Pentium MMX or higher).

johnnovak avatar Sep 22 '23 09:09 johnnovak

The joystick regression fix, when no joystick is physically plugged, brought back the presence of a virtual-mappable joystick.. like a dummy ever-present joystick that DOS programs see. It lets non-joystick owners easily map their keyboard buttons to joystick events and play joystick-exclusive games.

.. but the presence of a joystick was a bit rare in the DOS days (perhaps 1 in 100 DOS PCs had a joystick. I threw away my Gravis due to drift, and all of my fellow DOS PC friends didn't have one).

Some software and games has bugs when a joystick is plugged in.. maybe this is one of them? (Unfortunately would need a real 15-pin joystick to test on real HW with your SB card).

As for testing the no-joystick scenario, can try:

[joystick]
joysticktype = hidden

This hides the joystick from DOS's awareness and I/O port eventing.

kcgen avatar Sep 22 '23 09:09 kcgen

Can test the prior behavior with:

[joystick]
joysticktype = hidden

This hides the joystick from DOS's awareness and I/O port eventing.

Indeed, that fixes it! Wasn't aware of this setting (I very rarely play games that require a joystick; I'm a keyboard + mouse guy).

johnnovak avatar Sep 22 '23 09:09 johnnovak

It seems this is not really a bug, more like a weird CPCE issue, plus there's a workaround. CPCE might as well behave the same way on a real computer when a joystick is plugged in for all we know (unfortunately, I can't test it as I don't have an analog joystick for my retro PCs). Happy to close the ticket if you guys agree @kcgen @weirddan455 .

johnnovak avatar Sep 22 '23 10:09 johnnovak

Yeah I don’t know how to determine if this is our bug or just a quirk of this software short of testing it on real hardware which I don’t have.

Thank you @kcgen for explaining the situation in better words. That was what I was trying to get at, although I was not aware of that config flag.

weirddan455 avatar Sep 22 '23 11:09 weirddan455

I think we just need confirmation that this isn't a regression versus SVN, which should also exhibit the same issue (it similarly supports an emulated joystick).

kcgen avatar Sep 22 '23 12:09 kcgen

Ok, so the problem happens with main with cycles = auto only. If I set cycles = max or any fixed cycles value manually, I can't trigger it.

I can't trigger it in SVN or in 0.80.1 either with any cycles settings.

It's a weird issue, but a rather low priority one. I'll just keep it open until someone has the appetite to dig deeper because something strange is going on here.

johnnovak avatar Oct 12 '23 08:10 johnnovak