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

Added definitions for ImageButton with explicit ID #266

Merged
merged 9 commits into from
Dec 7, 2023

Conversation

Sinan-Karakaya
Copy link
Contributor

Hello!

I've added support for the new definitions of ImGui::ImageButton that support explicitely setting the id of the widget.
This feature was added in ImGui v1.89 and wasn't implemented in ImGui-SFML as far as I was aware.

Please give any feedback if something is wrong. Thanks :)

@ChrisThrasher
Copy link
Member

This will require we raise the minimum ImGui requirement from 1.87 to 1.89. @eliasdaler Thoughts on raising that requirement?

@Sinan-Karakaya
Copy link
Contributor Author

This will require we raise the minimum ImGui requirement from 1.87 to 1.89. @eliasdaler Thoughts on raising that requirement?

For what's it's worth, it seems like the old definitions are still available in 1.89, although they are now marked as deprecated.

@ChrisThrasher
Copy link
Member

Search the repo for "1.87". You should find another reference to that version in a build script.

@ChrisThrasher
Copy link
Member

Are there certain package managers still shipping <1.89 that we would break support with if we raised this requirement?

@Sinan-Karakaya
Copy link
Contributor Author

Search the repo for "1.87". You should find another reference to that version in a build script.

Found it and fixed it in CMakeLists.txt

Are there certain package managers still shipping <1.89 that we would break support with if we raised this requirement?

I would have to check, I mostly use FetchContent with ImGui-SFML so I'm not sure about that

CMakeLists.txt Outdated Show resolved Hide resolved
@ChrisThrasher
Copy link
Member

Search the repo for "1.87". You should find another reference to that version in a build script.

Search again. There are more references to 1.87.

@Sinan-Karakaya
Copy link
Contributor Author

Search the repo for "1.87". You should find another reference to that version in a build script.

Search again. There are more references to 1.87.

You're right my bad, should be fine now (🤞)

@ChrisThrasher
Copy link
Member

Thanks for cleaning that up.

I'm going to give Elias time to weigh in one whether he likes increasing this minimum requirement.

@eliasdaler
Copy link
Contributor

I'm going to give Elias time to weigh in one whether he likes increasing this minimum requirement.

Let's do it. 1.89 was released more than a year ago and Dear ImGui is a fast moving library which many users are expected to keep up with since it rarely breaks its API.

@eliasdaler
Copy link
Contributor

eliasdaler commented Dec 7, 2023

Shouldn't overloads without const char* id be removed? It seems like you can't call ImGui::ImageButton without passing a label/id since 1.89.

imgui-SFML.cpp Outdated Show resolved Hide resolved
@Sinan-Karakaya
Copy link
Contributor Author

Shouldn't overloads without const char* id be removed? It seems like you can't call ImGui::ImageButton without passing a label/id since 1.89.

They can be kept, as those definitions are still available in 1.89. The only difference is that they are now marked as deprecated.

@eliasdaler
Copy link
Contributor

Shouldn't overloads without const char* id be removed? It seems like you can't call ImGui::ImageButton without passing a label/id since 1.89.

They can be kept, as those definitions are still available in 1.89. The only difference is that they are now marked as deprecated.

I see. Let's just drop these overloads - it's better than copy-paste and we'll need to get rid of them eventually.

@Sinan-Karakaya
Copy link
Contributor Author

Shouldn't overloads without const char* id be removed? It seems like you can't call ImGui::ImageButton without passing a label/id since 1.89.

They can be kept, as those definitions are still available in 1.89. The only difference is that they are now marked as deprecated.

I see. Let's just drop these overloads - it's better than copy-paste and we'll need to get rid of them eventually.

Just dropped them in last commit

@eliasdaler
Copy link
Contributor

Much better.
@ChrisThrasher, please look at it again and feel free to merge.

imgui-SFML.cpp Outdated Show resolved Hide resolved
imgui-SFML.cpp Outdated Show resolved Hide resolved
imgui-SFML.cpp Outdated Show resolved Hide resolved
@ChrisThrasher ChrisThrasher merged commit 125081b into SFML:2.6.x Dec 7, 2023
25 checks passed
@Sinan-Karakaya Sinan-Karakaya deleted the fix/image-button-id branch December 7, 2023 20:00
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.

3 participants