SFML / imgui-sfml

Dear ImGui backend for use with SFML
MIT License
1.13k stars 169 forks source link

Update to use AddKeyEvent, supporting the full key range from v1.87 #197

Closed oprypin closed 2 years ago

oprypin commented 2 years ago

Dear ImGui v1.87 has expanded its support for keyboard and gamepad buttons, and replaced the interface for backends to report the keys. The old way keeps working OK but is deprecated and has big limitations, thus already making "imgui-demo" run with some missing functionality when it's run through "imgui-sfml".

This change implements the new approach. So, (almost) all the keyboard keys and possible gamepad events will be properly reported to ImGui and possible to query. Fixes #196. Fixes #174.

I implemented this by following what changes the SDL backend has undergone and trying to make equivalent changes here.


Here you can observe the missing events: One can just run the default "imgui-demo" and in the current state of imgui-sfml it is missing a lot of information in the "Keyboard, Gamepad & Navigation State". After this change it matches the SDL backend's behavior.

before after
https://user-images.githubusercontent.com/371383/158024702-86a606c2-aea7-4718-a494-6e5ec8b06510.mp4 https://user-images.githubusercontent.com/371383/158024705-83840c9a-26ad-41ad-86a5-073bf89f51fd.mp4

This is backwards-compatible in the sense that I'm pretty sure all user code will continue working as is. In particular, both of these keep working the same:

ImGui::IsKeyDown(sf::Keyboard::Z) == ImGui::IsKeyDown(ImGui::GetKeyIndex(ImGuiKey_Z))

In the table below, "before" means without this change and "after" means with this change.

  ImGui 1.86 before ImGui 1.87 before ImGui 1.87 after
ImGui::IsKeyDown(sf::Keyboard::Z) Works Works Works
ImGui::IsKeyDown(ImGui::GetKeyIndex(ImGuiKey_Z)) Works Works Works
ImGui::IsKeyDown(ImGuiKey_Z) Always false Works Works

But Z was one of the keys that was possible to make ImGui aware of. Q was not one of such keys, hence:

  ImGui 1.86 before ImGui 1.87 before ImGui 1.87 after
ImGui::IsKeyDown(sf::Keyboard::Q) Works Works Works
ImGui::IsKeyDown(ImGui::GetKeyIndex(ImGuiKey_Q)) Invalid code Always false Works
ImGui::IsKeyDown(ImGuiKey_Q) Invalid code Always false Works

From a different point of view, this is not backwards-compatible because it requires ImGui 1.87 to compile. If the tables above had a column "ImGui 1.86 after", that would all be "imgui-sfml can't compile".


I also change the deadzone for sticks from 5% to 15%, because that seems much closer to what the SDL backend does.


Additions to the API are also up for discussion.

 IMGUI_SFML_API void SetJoytickDPadThreshold(float threshold);
 IMGUI_SFML_API void SetJoytickLStickThreshold(float threshold);
+IMGUI_SFML_API void SetJoytickRStickThreshold(float threshold);
+IMGUI_SFML_API void SetJoytickLTriggerThreshold(float threshold);
+IMGUI_SFML_API void SetJoytickRTriggerThreshold(float threshold);

In particular: Should we keep the typo "Joytick" consistent? Should we perhaps merge the L + R pairs of these functions into one function?

eliasdaler commented 2 years ago

Hello. Wow, this is a fantastic PR, thanks a lot!

I'll take a look at it soon as I'm currently very busy with real life stuff... (from what I've seen, it looks correct, just needs some manual testing).

Which OSes did you test this on?

As for the "Joytick" - feel free to rename it to "Joystick". I have no idea how no one noticed that one before. API/ABI break is okay in our case (since Dear ImGui and SFML don't have make any ABI promises)

Should we perhaps merge the L + R pairs of these functions into one function?

Two functions are better than one, imo.

oprypin commented 2 years ago

Which OSes did you test this on?

Only on Arch Linux (KDE, Xorg, NVIDIA)

how no one noticed that one before

I definitely noticed, but didn't say anything

eliasdaler commented 2 years ago

Only on Arch Linux (KDE, Xorg, NVIDIA)

Okay, I see. I'll test it on Ubuntu and Windows 10 a bit later!

Please rebase your branch onto the latest master, I'll approve the Gitlab Actions runs (CI was broken)

eliasdaler commented 2 years ago

Please sync with master again - there are some merge conflicts now. :)

eliasdaler commented 2 years ago

Tested on Ubuntu 20.04 and Windows 10 - works fine. The one failing job is CI's issues, not caused by this branch. Merging... thank you!