mamedev / mame

MAME
https://www.mamedev.org/
Other
7.76k stars 1.95k forks source link

Fix one x way joystick rotary step sometimes rotates character2 steps #12541

Open zorro2055 opened 2 days ago

zorro2055 commented 2 days ago

Hello,

For an X-way rotary joystick that presses a virtual Dial or Positional Inc or Dec button for a certain amount of time, the virtual button must be "held" down long enough to guarantee it will be detected at a frame. This means it has to be held down for a time just slightly longer than a frame. This means that one rotary step could register twice if the timing is right across two frames.

This PR adds an "X-Way Joystick" toggle option in the Analog Input menu. When off (default), behavior does not change at all. When on, to avoid the above issue, if an inc or dec press is detected, mame will ignore any press on the next frame. So, this guarantees that one rotary step would always only rotate the character one step. It specifically has to be toggled on for a joystick designed for that to take advantage of it. This does mean the character would only be able to rotate at max, half the frame rate. So if the frame rate is 60fps, the character would rotate at 30fps which is still plenty fast.

I made it so this toggle only shows up for Dial and Positional Analog devices (games that use this type of controller use one or the other of those devices). This option does not show up for other Analog devices such as Pedals, etc.

Picture of option in Ikari Warriors analog menu: image

This is the x-way (12-way in this case) rotary joystick that uses this input scheme and will benefit from this PR: https://www.ultimarc.com/arcade-controls/joystick-accessories/ikari-12-way-rotary-upgrade-for-servostik-j-stik/

There are about a dozen games that use this type of rotary joystick. The most famous one is Ikari Warriors. https://www.reddit.com/media?url=https%3A%2F%2Fi.redd.it%2Fqo4x3p5dm4ry.jpg - note the octagonal joystick controllers https://www.youtube.com/watch?v=yQYoiZcqURg

Let me know if this looks good to merge. Or if there are any questions or comments. Thanks!

dinkc64 commented 2 days ago

Nice one, been wanting such a feature since forever!

happppp commented 2 days ago

There is no universal solution for this, the number of animation steps per IPT_DIAL or similar controller distance traveled depends on game, it's not always once per frame.

dinkc64 commented 1 day ago

happppp, maybe some sort of digital / quadrature-encoder-ish style input type can be made to handle this?

zorro2055 commented 1 day ago

Thanks for the feedback @happppp and @dinkc64 .

Hmm, for me, from my testing, this PR fixes the issue referred to in the title for all the games that use the rotary joystick that I know of. Most of them are Analog type Positional, only 3 are type Dial. See what analog settings I used at the end of this message. With these settings, one rotational step on the rotary joystick would sometimes rotate the character 2 steps if the X-Way Joystick option I added was off. If that option was turned on, the character would never rotate 2 steps if the rotary joystick was rotated one step. My rotary joystick is configured to press the "virtual" Positional or Dial Increment and Deincrement Buttons for 20 ms. Longer than one frame, but less than 2 frames.

@happppp Are there other IPT_DIAL games that have multiple animation steps (between frames I guess)? Or do input polling multiple times per frame? That would give me something to look at. I'm all for making this PR work for scenarios I am not aware of as well. Would adding another option such as "Ignore Number of Input Polls" or "Time to ignore next button press" in the analog menu help?

@dinkc64 i'll wait for @happppp response to your suggestion to get a better understanding before asking about it.

I look forward to your answers to help clarify things.

Analog Settings for Games I tested that support an X-way rotary joystick For the following games with Positional Analog type, I used the settings of: Positional Increment / Deincrement Speed: 1 Positional Sensitivity: 100 Bermuda Triangle DownTown Gondomania Guerilla War Heavy Barrel Ikari Warriors Victory Road Ikari III Midnight Resistance SAR - Search and Rescue T.A.N.K Time Soldiers

Then I used these individual settings for the Dial games that support a rotary joystick: Caliber 50 Dial Increment / Deincrement Speed: 4 Dial Sensitivity: 100

Touchdown Fever Dial Increment / Deincrement Speed: 2 Dial Sensitivity: 100

Touchdown Fever 2 Dial Increment / Deincrement Speed: 2 Dial Sensitivity: 100

happppp commented 1 day ago

Ah right, inc/dec speed is the number of incs/dec steps per frame for IPT_DIAL, so yeah I suppose it will work better than I first thought (user will need to find the correct speed setting per game). I'm ok with the concept and my disbelief is gone, and it is not feature creep.

Is "X-Way Joystick" meaningful? Something like "One-shot Increment/Decrement" or "Single Step Increment/Decrement" makes more sense. Is it user friendly to limit it to IPT_POSITIONAL and IPT_DIAL? It can in theory be used for anything that does not autocenter (aka center speed of 0), so basically any analog input. Source changes: I didn't look much. Keep in mind to only use tabs for left indentation, use spaces in if it's in the middle of a line instead of "bool (tab) (tab) (tab) something;"

zorro2055 commented 17 hours ago

Thanks @happppp , good to hear!

Ok, so basically, more generalize it. That makes sense. I'm more partial to "Single Step Increment/Decrement". Here are some other possible suggestions. I would not call these final suggestions, but more fuel to try to come up with the best description.

The most complete, but too long description would probably be "Prevent double detection of increment/decrement fixed time pulse of 1.x frames duration. Changes max input detection rate to game frames per second divided by 2.". Other descriptions to provide possible inspiration: "Prevent Extra Increment/Decrement Detection" "Prevent Double Increment/Decrement Detection" "Ignore Next Frame Increment/Decrement Detection"

If none of those inspire another description, I'm fine with going with "Single Step Increment/Decrement". Just let me know what you think.

I don't know the other analog types as well, but looking at the code now, I see what you are saying. Some of the types can be set to auto centering, but centering can also be set to 0. Ok, I will remove the if statements so that this option is visible for all analog device types. And that this option is always applied for the device if the option is set to yes.

Will do on the source changes. I'll be removing the "internal" tabs and replacing with spaces.

So, I can push the updated changes once there is agreement on the menu option name. I'll change the xwayjoystick variable based on that final menu name option as well.

rb6502 commented 17 hours ago

With regard to the tabs/spaces, if you build MAME with TOOLS=1 it'll make a utility called "srcclean" that you can run on a file to clean everything up.

cuavas commented 17 hours ago

I agree that there’s a problem, but I don’t think this is the solution. The issue is that the handling of “sensitivity” for positional controls is completely broken.

Moving “N steps per frame” isn’t useful behaviour for positional controls. You really want to be able to configure it to move “one step every N frames”. If you hold the button/direction, it should move continuously, but at a reasonable speed.

It may be possible to just apply different scaling for positional inputs when the source is digital.

We already apply scaling to the sensitivity when an absolute control is used for a relative axis input. See the code here – we divide by eight to get more sane scaling: https://github.com/mamedev/mame/blob/mame0267/src/emu/ioport.cpp#L3832

I think that function really needs to be updated to treat sensitivity for positional inputs differently when an absolute control is used for a wrapping positional input, or a digital control is used for any positional input.

happppp commented 16 hours ago

Those other setting descriptions you suggested? IYAM they don't strike my fancy.

Tweaking analog sensitivity won't get these type of controllers to work, the one posted by zorro: https://www.ultimarc.com/arcade-controls/joystick-accessories/ikari-12-way-rotary-upgrade-for-servostik-j-stik/

cuavas commented 15 hours ago

Those other setting descriptions you suggested? IYAM they don't strike my fancy.

Tweaking analog sensitivity won't get these type of controllers to work, the one posted by zorro: https://www.ultimarc.com/arcade-controls/joystick-accessories/ikari-12-way-rotary-upgrade-for-servostik-j-stik/

Those will never work well because they don’t show as an analog axis from the PC’s point of view. MAME doesn’t respond to button presses and releases, it polls the state of buttons on each emulated frame update.

MAME will miss “clicks” in various situations, for example:

You’ll also currently get bad results if the emulated system is running fast enough that it does more than one frame update between the simulated press and release.

MAME’s approach of polling once per frame is fundamentally incompatible with things that are physically relative axis controls but work by simulating button pushes.

Note that this PR only handles the the case where the emulated system is running fast enough that there are multiple frame updates between the simulated press and release. It doesn’t deal with the press and release happening between frame updates, or spinning the control too fast to see every pressed and released interval.

And all that doesn’t change the fact that the current behaviour is basically useless for positional controls. You never want “N positions per frame” when holding down a key/button assigned to one of these inputs – you want it to move one position when you press the key/button, and then continue to more at a reasonable rate if you continue to hold the key/button (and one position per frame is never a reasonable rate).

zorro2055 commented 9 hours ago

@rb6502 Thanks, I'll use that.

@happppp Sounds good. I'll use "Single Step Increment/Decrement". I'll replace the xwayjoystick variable with singlestepincdec and the enumerated constant ANALOG_ITEM_XWAYJOYSTICK with ANALOG_ITEM_SINGLESTEPINCDEC. And I'll fix the comments accordingly. I should have these changes pushed by the end of this weekend if not before.

@cuavas The way this PR is right now does work fine for me right now. The 12-way rotary joystick that I have that happppp just mentioned simulates a virtual inc or dec button press (depending on which direction rotated) for a fixed time. Right now, I have that fixed time set to 20ms. The joystick snaps 30 degrees to trigger this "virtual" button press. That is longer than a frame, but less than two frames. So Mame is guaranteed to detect it. I have added the option in this PR to essentially keep double detection from happening. Any press that happens a frame after a press was just recorded is ignored if this option is on.

It's true that the game has to run at a consistent native frame rate, in this case, about 60fps, for this to work properly. Since all the games that use a rotary joystick which I tested are older, this is not an issue. For newer games that run slower, this would not be an issue if the game runs slower if the fixed time the "button" is held down is still slightly longer than one frame. If the frame rate is inconsistent, then this option would not work as well.

I can see your issue with Positional if real physical buttons are used to try to rotate the character in Ikari Warriors or similar game. I can only think of one new option (separate and not related to this PR) that might help with that, but would take some effort to determine if it could work and implement.

Basically add another on/off option to the analog menu called something like "Interpret Sensitivity as Frames per Inc/Dec". When that is turned on, then if a button is first pressed, the game inc or dec something, then if the button is continually pressed, it waits that many frames before applying the key inc/dec again....