djhackersdev / bemanitools

Runs recent Konami arcade games and emulates various arcade hardware.
The Unlicense
84 stars 16 forks source link

Add ViGEm iidxio client - [merged] #169

Closed icex2 closed 1 year ago

icex2 commented 3 years ago

Merges vigem-iidxio -> master

Summary

Adds a ViGEm client application daemon that exposes BT5's iidxio API as virtual XBOX controllers.

Description

This allows you to use any hardware that is implementing the BT5's iidxio API with any application/game that supports xinput game controllers.

Related Issue

@GRIM.657 also started to work on this at the same time. This MR is incorporating any additional changes of their MR, e.g. compatibility with LR2, more options for TT input handling.

How Has This Been Tested?

Windows 10 x64, system settings, game controllers test menu

Checklist

icex2 commented 3 years ago

added 5 commits

Compare with previous version

icex2 commented 3 years ago

added 6 commits

Compare with previous version

icex2 commented 3 years ago

marked the checklist item Followed the developer (style) guidelines. as completed

icex2 commented 3 years ago

General note: Even this is not compatible with Infinitas, I would still like to get this merged as I would consider it a valid solution nevertheless. Someone else can have a look at whatever emulates direct input, e.g. vjoy, and open another MR.

@GRIM.657 would be cool if you can have a look at my implementation and also give it a try on whatever setup(s) you tested yours. Feedback highly appreciated.

icex2 commented 3 years ago

In GitLab by @GRIM.657 on Dec 23, 2020, 03:30

Hey, I got a chance to test this today. Fyi, I’m using an IO-2-R with Windows 10 Home and a custom PC build (i7-6700T, RX560).

When I set the lights mode to 1 or 2, the behavior is the same, once the TT is spun the neons turn on and remain on all the time. Once the TT-as-a-button input is copied over, mode 2 will probably be easy to fix because the neon illumination state and the TT-as-a-button state can be the same (this is how I did it). I’m not sure why mode 1 is behaving the same as mode 2.

In Relative mode it seems like the stick position is constantly 0, I don't get any response. You can check the adjustments I made to the math in my MR for Relative mode to work, and it’s still a little rough around the edges, but it’s functional.

There is no input on controller 3. P1 keys + start, and P2 keys + start both show up on each pad, but the third pad shows no response for the cab buttons like Effect, VEFX, Test, Service. At a cursory glance at the code I’m not sure why this is happening.

The LED ticker functionality is really cool! One thing that might be helpful is converting the user’s config input to uppercase for them, or adding a note about using uppercase letters. I tried using lowercase letters but got gibberish on the ticker.

icex2 commented 3 years ago

Hey, I forgot to add that this is not entirely ready, yet, and I still have to port a few things from your branch. Also, I haven't tested everything, yet, either.

Nevertheless, your input is valuable nevertheless as it points out things that I still need to look into + some testing details.

I will rename this MR to make this clear now and let you know once this is actually ready for testing. Sorry for not making this clear.

icex2 commented 3 years ago

marked this merge request as draft

icex2 commented 3 years ago

added 46 commits

Compare with previous version

icex2 commented 3 years ago

added 1 commit

Compare with previous version

icex2 commented 3 years ago

added 4 commits

Compare with previous version

icex2 commented 3 years ago

marked this merge request as ready

icex2 commented 3 years ago

added 2 commits

Compare with previous version

icex2 commented 3 years ago

MR is now ready to be reviewed. I should have incorporated everything from your branch @GRIM.657. All logic is ported 1:1, so everything should behave identical to your branch. One exception though, I figured that quickly reversing the TT direction did not translate well to the button inputs. I added a bit of code there (see separate commit) which takes care of that, imo.

Please test and let me know how everything works/feels.

@xyen your feedback is appreciated as well, of course.

icex2 commented 3 years ago

marked the checklist item Incorporate any additional features from @GRIM.657's branch, MR !67 as completed

icex2 commented 3 years ago

In GitLab by @xyen on Jan 1, 2021, 04:17

Commented on doc/vigem/README.md line 12

3 pads

icex2 commented 3 years ago

In GitLab by @xyen on Jan 1, 2021, 04:17

Commented on doc/vigem/vigem-iidxio.md line 5

Probably add a note that it doesn't work with games that require dinput (infinitas)

icex2 commented 3 years ago

In GitLab by @xyen on Jan 1, 2021, 04:17

Commented on src/main/iidxio-ezusb/iidxio.c line 84

gate this behind a config flag

icex2 commented 3 years ago

In GitLab by @xyen on Jan 1, 2021, 04:17

Commented on src/main/vigem-iidxio/config.c line 1

given the complexity of the config file, I'd generate a default one and provide it.

icex2 commented 3 years ago

Why? Since this is an issue/bug on real hardware that can trigger odd software behavior depending on how you process the inputs on startup, I don't see a reason having to gate this behind a flag.

icex2 commented 3 years ago

changed this line in version 8 of the diff

icex2 commented 3 years ago

added 2 commits

Compare with previous version

icex2 commented 3 years ago

added 19 commits

Compare with previous version

icex2 commented 3 years ago

In GitLab by @GRIM.657 on Jan 3, 2021, 04:02

Commented on src/main/vigem-iidxio/cab-light-sequencer.c line 55

I still noticed the neons flickering occasionally, I tested with a multiplier of * 4 here and that worked nicely. An even higher value might be better.

icex2 commented 3 years ago

In GitLab by @GRIM.657 on Jan 3, 2021, 04:13

Thanks for incorporating the additional features! I tested using the same setup as before, it works great, verified all buttons, lights modes, and TT input modes. I think the TT button input responsiveness in LR2 feels improved compared to mine, that’s a good addition. I’m going to go ahead and close my MR.

There’s one oddity I found. If I use test+service to exit the program, and then I relaunch it, it reports test+service are held and exits again. Relaunching it a 2nd time works normally. I tried calling _all_lights_off_shutdown() at line 327 before the main loop starts to flush the input state, and this seems to help. I wonder if it actually makes more sense to incorporate something like _all_lights_off_shutdown in the iidxio-ezusb(2) init/fini functions, since this is a quirk of that hardware and might not apply to BIO2 or other iidxio implementations. I think that's outside the scope of this MR, but might be something to consider.

icex2 commented 3 years ago

In GitLab by @xyen on Jan 5, 2021, 18:41

Commented on src/main/iidxio-ezusb/iidxio.c line 84

The bug is specific to using it with certain iidxio right? If using this with other iidxio, they may behave differently, and not require the flushing, hence why I feel like it should be gated.

Edit: nvm, idk why I thought this code was in vigem-iidxio

icex2 commented 3 years ago

In GitLab by @GRIM.657 on Jan 5, 2021, 22:06

Commented on src/main/iidxio-ezusb/iidxio.c line 84

From my comment above: "I wonder if it actually makes more sense to incorporate something like _all_lights_off_shutdown in the iidxio-ezusb(2) init/fini functions, since this is a quirk of that hardware and might not apply to BIO2 or other iidxio implementations. I think that's outside the scope of this MR, but might be something to consider."

This would actually have several advantages. It keeps hardware-specific quirks to the section of code directly related to that hardware. It removes the need for duplicated code between vigem-iidxio and iidxiotest. And, it would result in a clean I/O state when quitting out of IIDX AC titles too, instead of just these utilities.

icex2 commented 3 years ago

See my other comment below.

icex2 commented 3 years ago

Yup, I agree and 4 works better than 2.

icex2 commented 3 years ago

changed this line in version 10 of the diff

icex2 commented 3 years ago

added 2 commits

Compare with previous version

icex2 commented 3 years ago

added 2 commits

Compare with previous version

icex2 commented 3 years ago

I agree with you @GRIM.657, that's a good point to move this to the hardware specific iidxio implementations. I have moved some more HW specific stuff when turning off the lights to flush any remaining data properly on iidx_io_fini of iidxio-bio2 and iidxio-ezusb (both tested).

icex2 commented 3 years ago

In GitLab by @xyen on Jan 6, 2021, 19:48

approved this merge request

icex2 commented 3 years ago

This turned out really nice, thanks a lot for your help @GRIM.657. Your input regarding code and feedback was very valuable, highly appreciated.

icex2 commented 3 years ago

added 22 commits

Compare with previous version

icex2 commented 3 years ago

resolved all threads