Skip to content
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

Fix for imgui 1.91.5 (clean some deprecated API calls) #305

Open
wants to merge 2 commits into
base: 2.6.x
Choose a base branch
from

Conversation

Legulysse
Copy link
Contributor

@Legulysse Legulysse commented Nov 13, 2024

This PR contains fixes for imgui 1.90 when using the IMGUI_DISABLE_OBSOLETE_FUNCTIONS flag.
Associated discussions : #301

For this PR to be finalized, the minimum supported version will have to be updated from 1.89 to 1.90.
The supported versions with the flag enabled would range from 1.90 (November 2023) to 1.91.0 (July 2024)
EDIT: It appears that the minimum version supported is unchanged, no need to update it.

To make the change on the CI settings and docs, I guess I should wait for the imgui_version branch to be merged back into the 2.6.x branch ?

PS: if several tests are implemented for different imgui targets, it could be interesting to also target the -docking tags associated with each imgui version.
(EDIT: not sure if anything could break on those branches though).

@Legulysse
Copy link
Contributor Author

Oh, it seems that the changes are still compatible with 1.89.
I also just realized that the switch cases I modifed are no longer needed...
I will tweak that.

@Legulysse Legulysse marked this pull request as draft November 13, 2024 17:22
@ChrisThrasher
Copy link
Member

What do we gain by upgrading the minimum ImGui version for ImGui-SFML 2 besides the ability to remove more deprecated functions?

@Legulysse
Copy link
Contributor Author

Upgrading the minimum version with the associated fixes could allow users to build with the more strict compilation flag, but I realize that maybe we are not many to do that and it's not worth it.
In that case you can wait for deprecation redirectors to disappear from imgui sources before migrating the code and the minimum version, it seems like those breakages are generally trivial to fix.

Concerning this PR, it appears that the changes are still compatible with 1.89, and would allow building with the deprecation flag from 1.89 to 1.91.0.
I will have to test with a gamepad that navigation still works properly.

For more details about API deprecation, I just realized that an exhaustive list exists at the beginning of imgui.cpp (look for "API BREAKING CHANGES" near line 430).

@ChrisThrasher
Copy link
Member

Concerning this PR, it appears that the changes are still compatible with 1.89, and would allow building with the deprecation flag from 1.89 to 1.91.0.

If we can remove the use of interfaces that exist in 1.89 but were deprecated later, I'm cool with that.

@Legulysse
Copy link
Contributor Author

After doing some migration on my own projects, it appears that the latest imgui version (1.91.5) actually removed some of the interfaces I cleaned in this PR, so it will be required anyway.
I should be able to mark the PR as ready as soon as I can check builds with it.
I will also rename it to better match the intent.

@Legulysse Legulysse changed the title Fix for imgui 1.90 with IMGUI_DISABLE_OBSOLETE_FUNCTIONS Fix for imgui 1.91.5 (clean some deprecated API calls) Nov 18, 2024
@Legulysse Legulysse marked this pull request as ready for review November 18, 2024 00:58
@@ -586,36 +586,9 @@ void SetJoystickRTriggerThreshold(float threshold) {

void SetJoystickMapping(int key, unsigned int joystickButton) {
assert(s_currWindowCtx);
// This function now expects ImGuiKey_* values.
// For partial backwards compatibility, also expect some ImGuiNavInput_* values.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did ImGuiNavInput_ enumerators get removed?

assert(key >= ImGuiKey_NamedKey_BEGIN && key < ImGuiKey_NamedKey_END);
finalKey = static_cast<ImGuiKey>(key);
}
assert(key >= ImGuiKey_NamedKey_BEGIN && key < ImGuiKey_NamedKey_END);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please break this up into two separate asserts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants