search

SFML / imgui-sfml

Dear ImGui backend for use with SFML
MIT License
1.15k stars 172 forks source link

Added definitions for ImageButton with explicit ID #266

Closed Sinan-Karakaya closed 11 months ago

Sinan-Karakaya commented 11 months ago

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 commented 11 months ago

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

Sinan-Karakaya commented 11 months ago

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 commented 11 months ago

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

ChrisThrasher commented 11 months ago

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

Sinan-Karakaya commented 11 months ago

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

ChrisThrasher commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

Thanks for cleaning that up.

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

eliasdaler commented 11 months ago

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 commented 11 months ago

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.

Sinan-Karakaya commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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