JaydenMaalouf / JoystickPlugin

Native Input Joystick Plugin for Unreal Engine 4/5
MIT License
30 stars 6 forks source link

JoystickDeviceManager initialized too late #19

Closed brifsttar closed 2 years ago

brifsttar commented 2 years ago

Hey again @JaydenMaalouf,

Another bug came up today, this one leading to crash on startup. It's fairly easy to reproduce.

This should lead to a crash. At least it does for me and a colleague. I've tracked down the issue to the fact is the JoystickDeviceManager doesn't yet exist at the time of the JoystickCount call. The manager actually takes a couple of seconds to spawn.

It's not an issue in-editor, or if your game/app doesn't use SDL right after launch. But it can become an issue in certain use cases (e.g. your default controller registers for SDL events).

However, I don't really know how to fix this. The manager is created by CreateInputDevice, which is inherited from IInputDeviceModule; so it's not like we can just call that earlier. I've tried changing the plugin's LoadingPhase, but it doesn't seem to impact this call. I don't really have any idea of the call chain leading up to CreateInputDevice being called, so I don't know why it occurs so late in the startup process.

For now the workaround is just to delay all calls to the plugin by a few seconds, which works, but isn't really robust (or nice). IJoystickPlugin::IsAvailable() could maybe check if the manager exists, but even though that would prevent crashes, it wouldn't solve everything as call made before availability would just get ignored, without the user even knowing it.

I can investigate this more, but I wanted to get it past you first.

Bertrand

JaydenMaalouf commented 2 years ago

Interesting find!

I’ll look into this one on Monday.

Might also introduce a delegate for when the input device is ready - instead of having to delay loop until is ready.

JaydenMaalouf commented 2 years ago

Haven't had much time for this project currently. Hoping to find some time in the next few days.

Juris3D commented 2 years ago

This variation of plugin is very promising, I hope it will be continued :) Best wishes.

JaydenMaalouf commented 2 years ago

This variation of plugin is very promising, I hope it will be continued :) Best wishes.

Absolutely. I am progressing the plugin as I work on my own project. New features coming soon - including ability to configure mappings with offsets and range map directly in the settings (saved into the input ini).

Look forward to getting this stable and ready for proper use!

JaydenMaalouf commented 2 years ago

@Juris3D @brifsttar I've just released a beta version that should solve the issues described here https://github.com/JaydenMaalouf/JoystickPlugin/releases/tag/v1.0.0-beta.14 This version fundamentally changes the way SDL is initialised in the plugin - making use of Subsystems within Unreal instead of a basic C++ class.

As part of this change, I added a delegate that you can hook into at anytime - like so: image Do note, the SDL is readied before Begin Play is called - just needed something for the screenshot haha Please let me know if this solves your issues

brifsttar commented 2 years ago

Hey @JaydenMaalouf, Thank you so much for this update! I'll try to test this (and #18) soon, but all my gear is at work, and with the Covid situation right now I can't go there too often. I'll keep you updated once everything is tested.

brifsttar commented 2 years ago

Hey @JaydenMaalouf,

I've started integrating the changes to my project, and I have some feedback/questions.

First, the DeviceId property of FJoystickInfo lost it BlueprintReadonly specifier, which is needed for FFB. I've fixed that on my fork and could do a PR for it if needed.

https://github.com/JaydenMaalouf/JoystickPlugin/blob/bbb7c0e23e961fd45ce75bd491661429f99d966d/Source/JoystickPlugin/Public/Data/JoystickInfo.h#L16-L17

Second, I used to call RegisterForJoystickEvents from the old interface, which apparently no longer exists. I think it was just needed to get the low-level OnStateChanged event (don't remember the exact name), but it isn't needed for device-specific events, right? I still had that RegisterForJoystickEvents lying around, from back in the days before you fixed the issue about device-specific events being on the wrong update loop, and I had to rely on the low-level state-change event. So I'm just wondering if I can safely get rid of the RegisterForJoystickEvents, or if there's a new one to call somewhere. And also, does this mean the low level OnStateChanged no longer exists? I don't need it, so that's just for my general understanding.

Finally, about the new Ready delegate, I'm wondering how to actually use it. For instance, if I wanted to print the joystick count after each BeginPlay, and I have multiple BeginPlay during runtime (e.g., multiple levels), would the delegate fire each time? From my understanding, it seems like it wouldn't: it only fires once ready. So the delegate itself is very useful for the first BeginPlay, but if it doesn't fire for subsequent ones, then it becomes an issue.

Maybe having a IsReady function could solve this. That way, on the BeginPlay, we can query it: if the subsystem is ready, we can immediately do our stuff. If not, we bind to the delegate and wait for it to fire.

JaydenMaalouf commented 2 years ago

@brifsttar

First, the DeviceId property of FJoystickInfo lost it BlueprintReadonly specifier, which is needed for FFB

That's not a problem at all. Not sure why I dropped it tbh. Will readd tomorrow once the build system is transferred to GitHub (currently moving from Azure DevOps)

Second, I used to call RegisterForJoystickEvents from the old interface, which apparently no longer exists

RegisterForJoystickEvents has been removed entirely as the Inputs are now properly exposed to the Engine via the Unreal Input Manager. Joysticks are automatically registered by the Subsystem on startup or if they're plugged in at runtime.

OnStateChanged has also been removed as we rely on the Unreal Input system for Action/Axis Mapping Events to give us the input values.

Finally, about the new Ready delegate, I'm wondering how to actually use it.

You're correct in your thinking that this will only fire once. It actually fires before the Level is even loaded as the Subsystem is initialised on application startup. This means in-editor the delegate is fired before the Editor is visible and in-game it's fired before the Level has loaded.

The subsystem lasts the lifetime of the game, so SDL is only ever initialised once. Thus, level transitions will not cause a refire.

Maybe having a IsReady function could solve this

I actually intended on adding this but completely forgot to do it haha

JaydenMaalouf commented 2 years ago

Almost a year on and this issue has been resolved! Release can be found here: https://github.com/JaydenMaalouf/JoystickPlugin/releases/tag/1.0.0