-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added measures to eliminate when occasionally, two character rotations steps occur when rotary joystick is twisted one position. #1652
Conversation
I think if you're looking to alter how inputs are polled you'd best do this at the input driver / frontend level instead of here. The input drivers already behave differently between platforms as it is. I was under the impression that you could only read a polled input once before having to poll again for most input drivers though I haven't explicitly tried this. Would this be the case where a game driver is reading the input last polled twice in a frame? @grant2258 has worked on the inputs here in the past, locally and the input drivers. Maybe he would offer up his opinion on this request. |
There are some overall issues here, a simplistic explanation is there are two main types of rotary encoder: absolute and incremental. I will assume from your explanation your are going for the incremental this would have side effects on an absolute encoder output. The output of an absolute encoder indicates the current shaft position, making it an angle transducer. . The output of an incremental encoder provides information about the motion of the shaft, which typically is processed elsewhere into information such as position, speed and distance. Now there is another issue your going by the emulation input poll rate when the game reads the polls itself it could be it doesnt read every frame on a particular game or it`s read as absolute rather than incremental. The encoder will read the amount of travel in incremental for mame dials are treated as analog so the analog menu is available. If you are using the dial as a button (digital) you would need to use the joy/key speed and use sensitivity for the real analog reads. I would need to check the code path for the dials to see its they are read abs or rel. Then enters another issue if your dial is reading rel or abs in RA itself. Its something that can be fixed in the analog menu though. If the defaults arent good for dial feel free to change them. The logical part of me says this isint a the right solution its more likely just a fix for a specific game perhaps? At saying that I dont really use the core so don't feel strongly about it to say yes or no though. Its worth remembering even buttons have bounce time after letting go of them. If its moving too fast it more than likely the sensitivity and (if using digital key press is too high or sensitivity) as mame tries to make a digital button press count up rather than just on using there two settings. |
Hi, thanks for your responses. Ok, I'll try to respond to some of the comments / questions now. And I'll circle back on others later depending on your feedback from these answers, etc....
Yes, I am going for incremental, types IPT_DIAL and IPT_DIAL_V in the code. A keyboard or joystick button is assigned to rotate clockwise and another button is assigned to rotate counterclockwise. These are named "Dial" in the input selection menu. I would guess that the types IPT_PADDLE and IPT_PADDLE_V are absolute, but did not check that. My pull request only applies to IPT_DIAL and IPT_DIAL_V.
Ah, any easy way to tell if this is happening? The games I have tested so far do not seem to do that, but I have not looked for that specifically....
Yeah, at least for the couple of these types of games that I tried, I needed to set joy/key speed at 16 and sensitivity to 100%. That basically turns the character exactly one step.
There are not a lot of games that have this particular stepped rotary action (usually, the steps are 30 or 45 degrees), so a fix for just those games might be possible. The games I am aware of that use this are (there could be more, but I would not think a lot more if there are): Per my option 3, if code is not added only for specific games, having additional options in the Analog Controls menu to disable this feature for other games that use IPT_DIAL and IPT_DIAL_V games that have much smaller steps than 30 or 45 degrees like the above games do could be a way to avoid unintended effects with those other games. Just for reference to see the style of game these larger step rotary joysticks were used for, here is a picture of the Ikari Warriors actual arcade cabinet. You can see the joysticks which actually twist in steps of 30 or 45 degrees (I can't remember which) to turn the character 45 degrees: And here is video of Ikari Warriors gameplay: I have this specific arcade controller that is a 12 way rotary joystick: Most of these games have similar type gameplay / concept of use as Ikari Warriors. With this particular joystick, there is button "debounce" to control how long one rotary step (twist) emulates a keyboard or joystick button being pressed down. With a Windows program that changes the setting the the joystick's firmware on its PCB, I can change the length of time that the "button" is "pressed". Since I am running Retropie in linux which has udev which has events, not polling, I was able to determine the time the "button" was held down for each. That info is:
I have mine set at Standard which holds the "button" down for typically a little more than one full frame (at 60 fps).
If it makes sense to put this more in the input drivers, I can look at that. It might be something like a new function is added that calls the polling function in the driver, but adds the ability to monitor the last time the joystick was twisted to filter out the character rotating two steps from one step of the rotary joystick. Hopefully, that was not too much info! |
This seems like a more appropriate direction in my opinion, also would benefit any core not just ours. |
Ok, from that info, here is my potential plan then. I'll wait for comments over the next day or two to see if y'all largely agree with this so I'm being the most efficient time wise. Some things might change a little as I have not dug deep yet into everything.
3a. To avoid disturbing other games that use very small steps for IPT_DIAL and IPT_DIAL_V, I do basically option 3 that I mentioned before. I add two new options to the Analog Controls menu, useRotaryJoystick (calls new function from libretro if set to yes), and Lockout time (how long to not allow new rotary input). So, if this is the current PORT_ANALOGX line and definition for SNK games from a certain time including Ikari Warriors:
then this would be that line updated to PORT_ANALOGJOY and the PORT_ANALOGJOY definition added.
Doing it that way means I do have to touch the 241 different PORT_ANALOG and PORT_ANALOGX lines, just the 10 to 15ish lines of games that use the large step rotary joystick. I would modify this PR to do items 2-3b after item 1 is completed with some testing of item 1 with item 2 to verify item 1 is working properly. This is obviously going to take some time to do. I would think I would have item 1 done in 1-3 weeks. Let me know if this is a plan that largely makes sense. |
Per driver edits here would be difficult to maintain, since backporting from newer mame would not have this. A thought on future proof design.... It seems to me that your goal is to adjust the polling rate to ignore a reading from these input types. I think it might be easier to create an option in the frontend solely. This reminds me that "polling behavior" is already an option under retroarch input options. Have you experimented with early vs late polling? If this doesn't help the situation, it maybe a place to add a new option, "delayed polling" with a secondary option to set a specific delay. |
I dont really agree with the direction of this on a logical basis then again I dont have any objections either way as long as your device works. Your handling of buttons should treating it as a rotary switch counter rather than a analogue dial device. Think of it im terms rotary encoder/rotary switch scenario(digital vs analog). If the device has an analogue mode consider using that as a dial device. A button press for example is around 100ms between pressing and reaction/movement time for a human and that is being generous. No way can a human react in a time frame of 4ms so there really is little need for that kind of accuracy. A lot of people try using frameskip for this kind of thing but the reality is your taking out the human reaction time. There really is no need for the pole at all. If you just catch the input as buttons in a 12 way counter with negative and positive direction the reaction time of devices 4ms wouldnt matter because it cant be physically done in that time scale. Just treat it as normal two button counter up and down rather than a dial. Since mame has a key speed this can be adjust to work already via the analog menu settings. @mahoneyt944 I cant see how this can be put in the front end. You would need 12 inputs for position tracking and its seems to be a negative/positive two output on the device in question. RA also would need 12 inputs for a 12 way rotary switch and way to map it as well. There is also no clear way to map this on a RA as we would need two more inputs for this device as far as the front end goes. A better solution would be a position(number switch) with sensitivity settings on how fast the button reacts for the sake of ra it would take digital buttons and analogue in mouse and joystick helpers. Since all these inputs are available you this could also be done on a core level. If Ra is doing it though there would also need to be a options for the separate input mapping for each input available for a real device. It would certainly need some thought if done through RA to be honest. I cant see anything beyond arcade emulators needing this though and each core would need updated to handle the new position code anyway. @zorro2055 this is mainly between you and @mahoneyt944 inputs discussions can get heated easily and it just my perspective on how I would deal with it if i was to do it. So just take its as something to consider or ignore rather than any kind of request as im not affiliated with libretro or any of its projects, I just put commits in now and then. Someone that isusing the core will have there own perspective on how things should be done. If someone want something done a way you dont agree with, they probably should be doing it themselves in the way they want it,if it differs from the way you want to do it that is. I think the changes that would be required on RA would probably get a strong no go but I could be wrong. |
A core level solution would be okay, but if it requires hundreds of drivers to be modified, that maybe problematic in respect to future backports and such, so there would need to be care given in this regard. |
Thanks for comments so far. I was thinking and testing some of them. I should have enough time tomorrow to read through all of them and respond. |
Okay, first, I'll just clarify how the current Ultimarc 12 way rotary device works. When the joystick is twisted, it snaps a 12-way rotary switch from one position to the next. So the joy stick basically snaps from one position to the next position every 30 degrees. With every twist to the next position, the PCB that the 12 way switch is attached to simulates a button press that lasts a specific time, either about 4, 8, 20 or 40 milliseconds depending on the firmware setting. I have mine set at 20 milliseconds (Standard setting). There is a unique simulated button press for clockwise and counter clockwise rotation. The simulated button press for clockwise and counterclockwise can be mapped to any keyboard, joystick, or mouse button. This is no option to have this 12 way rotary joystick report as a mouse axis (ie rotary encoder). This is the only aftermarket device that I have found that simulates the control style for Ikari Warriors and similar games. Until that is when I saw this rotary joystick: So yes, I'm interested in getting my current Ultimarc 12-way rotary to work with the best accuracy with this PR or similar. Understood @grant2258 that this conversation is basically between @mahoneyt944 and me, but I think it makes sense to answer your questions where I can just so also @mahoneyt944 has the most information to help us resolve this. @mahoneyt944 i did try the late polling option in RA and did again and it does not help. I think that is because if all polling points are later, then it is basically the same situation from a time point of view for potentially having one rotary step rotate the character two steps. So if regular polling looks like this with the dashed lines being one rotary step (20 milliseconds long per my Standard setting) and P standing for polling times:
As you can see, adding a setting for potentially more delay would only shift the second option further to the left and still have the same problem. That is why it seems the only option (with the control scheme that the ultimarc 12-way rotary joystick uses) is to ignore the next poll only if a rotary step was detected in the previous poll only per the algorithm of the PR (implemented by effectively locking out any more character rotation for 24 milliseconds after detecting a rotary step). Also, it does seem that the frontend setting applies to all games, but maybe it is possible to only target specific games. The way MAME is currently setup, it does seem to push for using the clockwise and counterclockwise button mappings in Dial which is probably why the Ultimarc 12-way rotary joystick is designed for that. You can see that for the PORT_ANALOGX pre-sets these clockwise and counterclockwise buttons right off the bat (the KEYCODE_Z and KEYCODE_X in the preset below from one line that I grepped):
@grant2258 i agree that the ideal device would probably have a dedicated mapped button for every angled position of a 12-way rotary with that mapped button basically always "virtually" held down when the rotary joystick was in that particular position. Both this control scheme and the emulated mouse axis (rotary encoder) control scheme eliminates the need to basically skip polling for a frame when a rotary step is detected as this PR's algorithm is doing. Obviously, no hardware exists that supports this 12 virtual button control scheme at the moment. Although, perhaps it will show up in the future at some point. Also, this 12 virtual button control scheme might be the best, if hardware for it existed, because as @mahoneyt944 pointed out, rotary encoders can be a little squirrelly. Anyway, so what to do now with the currently existing 12-way ultimarc rotary joystick? Potential option 1 - If most of this PR effectively goes into RA / libretro per my proposed Step 1 (as originally suggested by @mahoneyt944 ), but it goes into a separate new C file so that makes reverse porting from later versions of MAME easier? As long as the original polling function name stays the same during reverse porting, then the new polling function in the new separate C file calling the original polling function, should not be affected. I would have to verify this could be done of course. I only really looked at the udev_input.c driver before in RA. I'd have to look at it more to see how that whole driver system in RA works before I could verify this would definitely work.... Potential option 2 - keep it in the core mame2003-plus. This is basically what this PR currently does right now. For either of these options, again, to keep this algorithm change from affecting all analog ports (only needed with games that use the analog ports for large angle steps, such as Ikari Warriors, Time Soldiers, etc), I believe I would need to change to a new preset analog type for 10-15 games per my step 3a and 3b in an earlier comment. I would have to verify that I could easily have the other present analog types PORT_ANALOG and PORT_ANALOGX automatically set the 2 new analog menu items to values to not use this algorithm so that other games do not use this algorithm. So, if option 1 is preferred, I would first research that it could be implemented as described and would report what I found here. I would also research and make sure that new analog menu items could have a default setting easily assigned in one function if that preset does not set that new analog menu item. If I reported here that these two things could be implemented, then I would think that option 1 is possible. If option 2 is preferred, then I would research and make sure that new analog menu items could have a default setting easily assigned in one function if that preset does not set that new analog menu item. If I reported here that this one thing could be implemented, then I would think that option 2 is possible. Well, hopefully that was clear enough. Let me know what you think. |
@zorro2055 I think it's worth looking into how and if the frontend could handle this polling, Option 1 as you mentioned. I suppose for option 2, we could create a new analog type which would mirror mame2003-plus-libretro/src/inptport.h Lines 218 to 227 in 3523655
|
@mahoneyt944 sounds good. I'll start looking at the code to see if option 1 is possible like we are thinking and will report back. FYI, from a quick check of the latest version of MAME, it did appear to have the same issues. So, if everything is as we think it might be, maybe I would submit to fix there issue also in the latest version of MAME which could be back ported to here. And then this could be updated to reference that. But there are a lot of twists and turns on that path which might lead to a dead end, so I'll continue with our tentative plan that you just outlined as discussed. |
@mahoneyt944 , ok, here is the research I have done so far on option 1. It seems possible, and fairly clean, but let me know what you think. First, here are some of the relevant lines of code relating to implementing option 1. There are definitely multiple steps. First, the function pointer "poll_cb" is set to point to the polling function.
The type, "retro_input_poll_t" for "poll_cb" is defined in libretro.h (located in mame2003-plus and RA):
"poll_cb" is actually set by the function "retro_set_input_poll" declared in libretro.h:
This function is defined in mame2003-plus:
The "retro_set_input_poll" function is called in RA and selects "core_input_state_poll_maybe", defined in RA, as the polling function: https://github.com/libretro/RetroArch/blob/f091b5a9e9475255e5efaded5f95c9750fdfe15e/runloop.c#L4569 Then things are set up for "poll_cb" to be called every frame it looks like:
So, here is roughly how I think a second poll function could be set for the games that used the 12-way rotary joystick in the arcades, and hopefully not get in the way of backporting features from later version of MAME. First, two new files would need to be created in Retroarch (names are just placeholders): To select the new polling function if needed, more code would be added to the function definition shown above in mame2003-plus for "retro_set_input_poll". It would check to see a new setting in the Analog Menu, called say "Use CW/CCW Rotary Joystick". If this setting is false, then the function sets poll_cb to the old poll function. The new poll function calls the old poll function, then filters the polled data that it needs to (types IPT_DIAL and IPT_DIAL_V). I glossed over some details, but hopefully you get the idea. Feel free to ask questions if anything is not clear. If this concept seems okay to you, I would start coding it in to the point where I could test it out. If there are things that might make this better, please let me know that as well. This option 1 would most likely also need an new port_analog type as well for the games that used a rotary joystick. I would work on that after I got code in place for the concept details I went through above. @grant2258 This would not happen right now, but I was thinking about your concept of using 12 virtual buttons for the 12-way rotary joystick. I think that could be reduced to 6 virtual buttons in total. Then hitting each button is a position 60degs apart from each other. To select the 30deg positions between the 6 buttons, 2 buttons next to each other would be pressed. That would represent the "position" between these two buttons. Maybe someone will make hardware that supports that someday.... |
I don't see any immediate issues with your proposal, it maybe worth verifying that the analog menu option is initialized prior to polling if it's to be referenced when the core initializes. |
Great, sounds good! Good point, I assumed the analog menu option is initialized prior to poll with some of the functions I saw. But now I just followed the relevant functions, and this does seems to be the case. In RA in function runloop_event_init_core, Then it appears 10 lines down from function runloop_event_init_core, the polling function is selected going down multiple functions from function runloop_event_load_core: So hopefully that is right. If all goes well, I should have this done in a couple of weeks. If I have questions along the way, I'll ask here. When I'm done, I assume I'll post a PR for Retroarch and update the code in this PR. If you are interested in seeing code along the way, let me know. A good half way point for that would be when I have all the code in RA and minimal code in mame2003-plus just to test the code in so far.... |
Hello @mahoneyt944 , this is taking longer than anticipated, but I am making progress. Calling function definitions in the main executable (RA) from the library (mame2003-plus) is not done often, so it took some research and testing. Now I have a question about which way to go. Let me know what you think.: In both scenarios, I have added the two new files, filtered_poll.h and filtered_poll.c, to RA. I also copied over filtered_poll.h to mame2003-plus. Also, in both scenarios, one line is added in Makefile.common in RA to add filtered.o to the OBJ list to compile. Scenario 1: NO modification to existing RA source files at all, some modification to the RA build files. For operating systems that support the ELF format, like linux, I was able to export the two symbols of the functions defined in filtered_poll.c for RA (the main executable). I did this by:
For Windows based systems, I typedef'd the two functions, then from mame2003-plus, was able to "probe" the main executable for the function definition pointers of the two functions defined in RA, then assign these function pointers to local typedef'd variables to call those functions. I was able to support both methods by using #ifdef, #else, #endif statements. I was able to get this to work in linux and in Windows 11 using MinGW64. I do prefer this method, but the only thing that worries me is since this method does not seem very common, does it work on all the other platforms that RA supports? Is there a semi-easy way to check that without having all the hardware? Scenario 2: Define a third function that is called from RA and defined in mame2003-plus. This function can pass the function pointers of the two functions in filtered_poll.c to mame2003-plus. From there mame2003-plus can call the functions. This scheme is used by RA currently multiple times. For instance, RA passes the polling function pointer to mame2003-plus by using retro_set_input_poll: So this method has been proven to already work on all RA's platforms, but this way does have to add a call to a third function that would be declared in filtered_poll.h and a #include <filtered_poll.h> line to one of the current RA sources files, probably runloop.c. So let me know what you think. Below is the code for my early filtered_poll.h, filtered_poll.c and modified retro_set_input_poll definition in mame2003.c for reference that demonstrates Scenario 1:
filtered_poll.c
modified retro_set_input_poll in mame2003.c
|
Actually, after grepping RA, and a little more googling, it does look like the methods in Scenario 1 are already used in RA, so that seems to prove they would work across all RA build targets. On the linux side, switch to using typedefs now as well like with windows, but with dlopen and dlsym function calls. This is already used in dylib.c: On the windows side, this method with GetModuleHandle and GetProcAddress already appear to be used in dylib.c: So, it seems I could go forward with Scenario 1, but switch to using typedefs, dlopen and dlsym for linux. Let me know if you see any issues with that, or if you are okay with me proceeding forward with Scenario 1. |
Further info I discovered on the windows side of Scenario 1: From these lines in lylib.c in RA, it looks like GetModuleHandle is not available in UWP, which seems to be what XBox uses? Perhaps if that is case, it could be compiled statically for UWP? |
Sounds alright to me. @arcadez2003 might be able to answer the Xbox case for you. Not sure on that. |
Thanks @mahoneyt944 . If @arcadez2003 has any feedback, great. But I did do some googling and I found these instructions to compile RA for UWP / Xbox 1: link It does look like the core is statically built into RA. Two package files are created, but one of them looks to only contain Visual C libraries. So, since I just discovered that I have a old PS Vita which still works (and also is statically linked when compiling RA for it), I will check to see if my code compiles and runs on that. If yes, then I'll check to see if it compiles for the Xbox 1. If yes, then, I would feel pretty confident that it would work on the Xbox 1, but I would not be able to test that. I'll go ahead on this, and report back with my results. If you see any issues with this though, let me know. |
Ok, I was able to successfully compile and run RetroArch with the mame2003-plus core per the code I gave above on PS Vita. This was with my added filtered_poll.h, filtered_poll.c and modified mame2003.c file. The dlsym and dlopen function calls that I was going to use instead for linux (non-windows) were not found, so I assume that library does not exist for PS Vita. But useing the code as I had it above (exporting the two functions from RA, then the linker links the declared functions from the core) worked. So I guess I'll stick with that. Also, using the compile instructions from the link I posted before to compile a UWP for Xbox 1, Series S, or Series worked as well. I do not have the hardware to test running it. That targets x64. Maybe only UWP for ARM doesn't support the GetModuleHandle function call. So, my plan is to continue on with more of the functionality per discussion in previous posts on here from a couple of weeks ago. Also, I'm thinking just in case there are some RA platforms out there that would not compile well with this (which I have not seen yet), I would wrap #ifdef (NO_FILTERED_POLL), #else, and #endifs around the code I add. That way, worse case, someone could define this variable in a Makefile to not include the added filtered poll code. It would just be the original poll function then. Let me know if there are any comments or questions on this, otherwise, I plan to just continue on. |
…Used to prevent single rotary step rotating character twice
…ctFromSingle12wayRotate Sync this branch with changes that have occurred in the upstream master.
…nput, or saved poll data request filter) to be used to lockout analog rotation button events for a certain time to prevent one analog rotation click (simulated button) to rotate character more than once.
a7801f5
to
cbcc653
Compare
Hi @mahoneyt944 , its been a while as it definitely took longer than expected and other things came up, but I believe it is done (it appears to work as intended with my 12-way rotary joystick) and ready to merge! It definitely uses more code than I expected. This pull request now reflects this latest code. See the corresponding RetroArch pull request that is needed for this to work that I request shortly. First, some key points about implementation on the mame2003-plus and RetroArch side) after how this ended up (hopefully, it all looks good to you):
That is all I can think of at the moment. Let me know if you have any questions of course. Hopefully, it is ready to merge off the bat. I'll post a link to the RetroArch pull request in another comment once I post that which should be shortly. |
Link to RetroArch pull request that is needed to for this pull request to work: |
Good news @mahoneyt944 ! I just saw your comment about you should be able to let this through. Yes, I did try to think it out as best as I could. |
Is this pretty much to fix issues with this code below reading twice per frame? Would it be adequate to just ignore additional reads if it was already read in that frame? As in 1 read per frame? mame2003-plus-libretro/src/drivers/snk.c Lines 371 to 429 in f03f865
|
Ah, the PR I submitted for that function fixed an issue when the 12-way rotates to 0xf0 which is not a valid position to get around a joystick error protection in Guerilla War according to the comments. My patch just held that position of 0xf0 for one frame, then went onto the next number so the character appears to rotate once rather than no times when the 12-way joystick rotates to 0xf0. I never looked at where it got the input at that time. Looking now, it appears that it:
The locally saved within mame2003-plus absolute value and previous absolute value look to be stored by the update_analog_port function (was between the functions I put in to check processing time in the my retroarch patch currently proposed) when it fetches the saved poll data from retroarch. So it seems the work of fully processing the raw analog input data is split into two stages, so I don’t think only one call could be made. Of course, there does seem to be a lot going on, so maybe there is a way to cut down code somewhere, but it would definitely take some investigation trying to figure out all the interactions. Hopefully, that somewhat answers your question. More details on the retroarch -> mame2003-plus absolute values input and mame2003-plus -> previous absolute + scaled relative value snk game input.retroarch→mame2003-plus absolute values update_analog_port function: mame2003-plus-libretro/src/inptport.c Line 1520 in f03f865
locally stored previous analog input absolute value (input_analog_previous_value[]) within update_analog_port function: mame2003-plus-libretro/src/inptport.c Line 887 in f03f865
seq_pressed (in update_analog_port) drills down to eventually call input_cb to get saved poll data from retroarch for deincrement port (“button”): mame2003-plus-libretro/src/inptport.c Line 909 in f03f865
seq_pressed (in update_analog_port) drills down to eventually call input_cb to get saved poll data from retroarch for increment port (“button”): mame2003-plus-libretro/src/inptport.c Line 913 in f03f865
after processing at end of update_analog_port, locally store current analog input absolute value (input_analog_current_value[]): mame2003-plus-libretro/src/inptport.c Line 1067 in f03f865
mame2003-plus → prevous absolute + scaled relative value snk game input readinputport function in snk_rot12 function: mame2003-plus-libretro/src/drivers/snk.c Line 386 in f03f865
scale_analog_port function (sensitivity scaling and time since last frame scaling happens here) in readinputport mame2003-plus-libretro/src/inptport.c Line 1536 in f03f865
input_analog_current_value[] and input_analog_previous_value[] processed here within scale_analog_port function: mame2003-plus-libretro/src/inptport.c Line 1080 in f03f865
which leads to setting the local variable input_port_value[] to the previous absolute + scaled relative: mame2003-plus-libretro/src/inptport.c Lines 1097 to 1098 in f03f865
This input_port_value[] is returned to snk_rot12 function in readinputport function: mame2003-plus-libretro/src/inptport.c Line 1539 in f03f865
|
And I think I misunderstood your first question. This PR here keeps one rotation step of a 12 way rotary joystick from occasionally being detected over two frames. This is because one rotation step of the rotary joystick is simulated as a button press. This "button" must be "pressed" for slightly longer than a frame to guarantee it is detected. But that means one rotation step will be detected over two frames rotating the character two steps instead of one. Hmm, a memory (static variable) could be built into that function to ignore input if input was received on the last frame. So that would work I think. Although, the function does not know about time implicitly or what frame it is. So that would have to be built in. Edit: actually, i believe this function just runs every frame; my PR from before took advantage of that. So time does not have to be kept track of. However, some of the games using 12 or 8 way rotary joysticks use a different driver, not snk.c. Such as snk68.c and probably others. Also, notice even in snk.c, there are other rot functions such as snk_rot8 and snk_rotx. Those might be used by some of the games. So if this method were implemented, all relevant rot functions in all the relevant drivers would have to be found and modified.... |
This can be used to grab the frame at anytime mame2003-plus-libretro/src/cpuexec.c Line 1124 in f03f865
You can also use it to manipulate time to a degree, like I did here to hide crosshairs after 15 seconds of inactivity. mame2003-plus-libretro/src/drawgfx.c Line 3539 in f03f865
I suppose you could do something similar, flag when a frame hits in a static variable and lock out for recurring hits on that frame. Or buffer a number of frames. |
Ah, good to know about those two items. Okay, that could work, and I imagine is a lot faster. If we assume the game always runs at its native frame rate, then just tracking frames works the best. If the game ran slower than its native frame rate (not good for the game in general), then tracking time is better to not lockout rotation for too long. This could also be implemented in just the update_analog_port for simplicity. Basically, if the analog port is a IPT_DIAL or IPT_DIAL_V type, and a static variable says rotation happened last frame, then don't run these lines: mame2003-plus-libretro/src/inptport.c Line 909 in f03f865
mame2003-plus-libretro/src/inptport.c Line 913 in f03f865
This can be turned on and off with the X-Way Joystick setting I put in the Analog Controls menu as well. If we assume things will always run at the games native frame rate, then a time setting that I added in Analog Controls is not needed. Are we exploring this because it is faster? Obviously, the current retroarch and mame2003-plus PRs I have proposed are slower, but are more general. Or will it depend on how the discussion with the retroarch PR goes? |
I don't have authority over what the frontend accepts or rejects. So this could be another solution, core side only, if they decide against your PR at least. Good to have options. I know slowing anything down for the sake of one core probably isn't their first choice...but not sure what they will do |
Got it. Thanks @mahoneyt944 , that makes sense. |
@zorro2055 this seems like a lot of overhead for a dozen or so games and likely only this core would be using this filtered polling.... As grant also pointed out this could effect other input types absolute vs relative too with how they are overlaid here. The latency is also a concern, although minimal is worth noting..Maybe there is a more simplistic approach or perhaps this might be a case where you want to branch this to your personal repo and use that core for these games only? I don't like to see your efforts go to waste, as I can tell a lot went into this, but I don't think we're ready to adopt this as it is today. |
Hi @mahoneyt944 , thanks for the feedback. It sounds like you talked with Libroretroadmin and grant. Yeah, I did like the concept of it having the filtered polling function accessible with any core, but it did get more complex than perhaps initially imagined. If it is unlikely any other core would use filtered polling, then I am fine with getting rid of that. That would get rid of a lot of code in the just mame2003-plus PR. The filtered_poll files would go. The large function in mame2003.c would go as well as other stuff. The #ifdef NO_FILTERED_POLL statements would be removed. It was a lot of work, but sometimes its hard to see the results before the work is done. With the results in hand, this new decision can be made. On the positive side, I definitely learned some stuff while doing it! I would like to still put something in as this would support anyone that bought that 12-way rotary joystick. Plus then I don't have to keep a separate patched version. I can rework this PR to just compensate for the one rotation of the joystick causing the character to rotate twice sometimes issue, no general "filtered polling". I was thinking about your using the frame count comment, and I think that always makes sense. No matter what the frame rate is, the "hold time" of the virtual dial button that a rotation of the joystick triggers must be a little longer than a frame, but not longer than two frames. So just using frame count works. So I could implement that. Then, I just need the one added setting for each player, "X-Way Joystick", in Analog Controls instead of two. This is by default off so the behavior of the dial buttons is not changed. Everything acts the same as before this PR with this setting. Only when X-Way Joystick is set to on is a pressed dial button locked out for one frame after it is pressed. So this would be set to off (default setting) if a person were using a keyboard or perhaps other devices with these 15 or so games. If a person had the rotary joystick, they would set this to on for the particular game. Here is a picture of that setting circled. Ignore the crossed out Lockout setting which I would remove: Pseudo code for what I would do so this only affects the dial buttons (when X-Way Joystick setting is set to ON) in the area I mentioned on April 5th is:
Does that basic plan sound good? I'm not sure how you mean by affecting the other absolute and relative types, but I think being able to turn this on and off with X-Way Joystick setting addresses that? Let me know if it doesn't though. Or maybe clarify what that means.... |
I think that sounds much simpler overall, @grant2258 what do you think? |
Sounds good to me, the only discussion on my part was on here. It could be possible that the input offsets might need checked on other games since the structure size changed. Ill need to check the code it probably sets things to default if its different havent checked, even if it does its as simple as deleting your config files. |
Sounds good! It will probably take me a week or two to make this update. For the defaults, I do know that when I went between testing this PR as is compiling with and without NO_FILTERED_POLL set (thus removing and adding the X-Way Joystick and Lockout settings), all input settings that I looked at reset. I had to set new dial keys, and Analog settings each time I did that including setting X-Way Joystick to Yes when NO_FILTERED_POLL was not enabled. But yeah, that behavior can definitely be double checked once I update this PR. |
…d and not filter the polling within Mame2003-Plus only
…ctFromSingle12wayRotate
That looks good to me, are you ready to merge? |
Great, yes, I'm ready to merge. I assume once this is merged, it is okay to port this to mame2003? |
Yes, you're welcome to port it there too if you want. Though you might want to let it simmer here for a little in case there's any bugs to work out. |
Ah, that makes sense. I'll do that. Thanks. |
@zorro2055 Seems ok to me, since this is for IPT_DIAL / IPT_DIAL_V. It might be sensible to omitt the the x-way options from showing in the mame menu for all other analog types. Since they can't be used with those types anyhow. Such as lightgun etc |
@mahoneyt944 Good to hear it is working for you. Ah, I see what you mean about showing up with the lightgun and others. I guess there are two options for this, the second which you mentioned:
Let me know what you think! Note: I just quickly checked about option 2a and 2b, so it might be more complex then I think, but hopefully not. |
Being that adding options isn't a common practice, 2a might be ok... though I haven't looked at the code to know if changing the entries size will alter the config sizes when they save ( which might not be ideal either). I'm surprised there isn't a common parameter to hide the visibility of an entry without resizing the config? It's not a major deal to leave it visible, as the toggle isn't functional for the other types but it definitely fills up the menu considerably. More of a visual thing. |
…s steps occur when rotary joystick is twisted one position. (libretro#1652)
Got it. Yeah, I skimmed all of the usrintrf.c file for some sort of disable feature and searched all of mame2003-plus for the words disable, grey and gray. Nothing came up. And it looks like only these first two variables are used for the menu, no obvious third variable for status such as enable or disable. So, I'm thinking it is pretty unlikely that feature exists. menu_item below is the name such as "X-Way Joystick". And menu_subitem is the setting such as UI_on or UI_off. The other variables do not seem to relate... label and setting are temporary variables which are then assigned to menu_item and menu_subitem respectively mame2003-plus-libretro/src/usrintrf.c Lines 1947 to 1955 in 97dfe65
From a UI point of view, it would be nice to get rid of those X-Way Joystick entries in the analog devices where they will not register a change. Let me try to put together a working 2a concept. I realized it will be slightly more complex because multiple analog devices can be used (and thus listed in this menu) for one game. For instance, Hard Drivin has analog petals and wheel, three separate analog devices. That will make it a little more complex, but I think it can be done. It will probably take me a week to get it together. Right, the saving thing could be an issue. But it might be ok since different games can have different amounts of analog devices, so a different number of lines in the menu. So length differences between games may not trigger a reset unless the number of analog menu line items changes in a particular game. We'll see! |
Hi @mahoneyt944 , So, it seems to be working pretty well. Should I open a new PR for that code for you to review? Or should I add that code to the branch that was for this PR and update that branch in github? Thanks. |
Yes open a new pr. Nice work |
Hi, this pull request will likely require discussion and maybe a change before being merged.
Explanation of issue:
Libretro appears to use a poll based system to get input from joysticks, keyboards, etc. This means once every frame, an input is polled at that moment to see what it state is. So, for an X-way rotary joystick single twist to guarantee that it is detected, it has to simulate a button press that lasts at least one entire frame. However, when the button "press" is held that long, one since joystick twist can sometimes be detected twice, once at the beginning of a frame and once at the end of a frame.
Here is a text graphic to kind of show this ( continuous dashed line is when one twist "presses" a button for a certain time, and P is when libretro is polling for input ):
Solution to this issue in this pull request:
If a twist is detected, rotate character one position as usual. Then, however, block any more character twists for 24 milliseconds after the initial twist is detected. 24 milliseconds is 1.44 frames @ 60fps and 1.2 frames @ 50fps. So the next twist that could be detected would happen two frames after the previous detection.
Cons with this method:
Alternative / possible code additions to minimize / eliminate some of the cons:
Option 1 - In games such as Blasteroids, the presets for the sensitivity (under Analog Controls under the Config Menu) could be doubled to allow the ship to spin at its original speed.
Option 2 - Change libretro to use events to gather input rather than polling. Then theoretically, multiple button presses could event be detected between frames. udev on Linux supports events, I'm not sure if all other systems supported by libretro do. However, this option must likely would need a huge overall of libretro, so it seems not worth pursuing.
Option 3 - In the Analog Controls under the Config Menu, two options could be added. The first option could be called possibly "Use rotary joystick". Setting this option to "yes" would turn on this pull request. The second option could be "Lockout time (ms)" which could be set to the number of milliseconds before detecting twists during input polling.
Option 4 - Something else?
Let me know what y'all think. If option 3 is favored, it would take time for me to search through the code to determine how to do that.
Once this PR, or some version of it, is hopefully approved, I can submit it to mame2003 as well where I originally developed it.
Thanks!
Thank you wanting to make a contribution to this project!
Please note that by contributing code or other intellectual to this project you are allowing the project to make unlimited use of your contribution. As with the rest of the project, new contributions will be made available freely under the classic MAME Non-Commercial License.
This license can be viewed at https://raw.githubusercontent.com/libretro/mame2003-plus-libretro/master/LICENSE.md.