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

Add uiSliderSetRange function to API #381

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

szanni
Copy link

@szanni szanni commented May 31, 2018

Add a function to the slider API to modify the slider range after slider creation.
This is useful for e.g. media players that reset the range with every track -> without having to resort to deleting the existing slider and creating a new one with all signals etc.

THE WINDOWS CODE HAS NOT BEEN TESTED
I sadly don’t have a windows machine at hand right now, but looking at the MSDN docs this should work.

I did not adhere completely to the style doc either - with regards to the temp variable, which is declared within the scope of the if block. Less lines, more screen estate. If this is ok I’d be willing to adjust the style guide to add this exception. If not here is the same content adhering strictly to the style guide:
https://github.com/szanni/libui/tree/uiSliderSetRange-formatting

@mischnic
Copy link
Contributor

mischnic commented May 31, 2018

Works on Windows 10.

You should set the current slider value to be within the new range in uiSliderSetRange.

@mischnic
Copy link
Contributor

mischnic commented May 31, 2018

Check the appveyor CI, there is a build error on Visual Studio 2013:

page4.obj : error LNK2019: unresolved external symbol _snprintf referenced in function _onRangeSliderChanged [C:\projects\libui\build\test\tester.vcxproj]
C:\projects\libui\build\out\Release\test.exe : fatal error LNK1120: 1 unresolved externals [C:\projects\libui\build\test\tester.vcxproj]

(https://ci.appveyor.com/project/andlabs/libui/build/build%20%2372/job/t3rx2mtgcf7sap7v)

@szanni
Copy link
Author

szanni commented Jun 1, 2018

Classic Windows not supporting the C standard. Apparently VS only started supporting the snprintf function as of 2015: https://social.msdn.microsoft.com/forums/vstudio/en-US/2b339bdf-7ab1-4a08-bf7e-e9293801455b/c99-standard-snprintf-function-

What is the best course of action here? The CONTRIBUTING.md states C++11 compatibility, snprintf is C99 so should be included. I know Microsoft thinks otherwise.

Should I change it to sprintf, make the buffer 12 char and assert() an Int has a max of 32 bits?

Or forget about the testcases compiling prior to VS 2015? Not sure if that would be the right approach.

@andlabs
Copy link
Owner

andlabs commented Jun 1, 2018

Yeah, I ran into this omission once before in the tester. And yeah, the final version of C++11 does state snprintf() should be part of <cstdio>... Just copy what I did in that other place.

@joa-quim
Copy link

joa-quim commented Jun 1, 2018

VS2013 has _snprintf

@szanni szanni force-pushed the uiSliderSetRange branch from 70c1fc4 to c74d924 Compare June 1, 2018 20:11
@szanni
Copy link
Author

szanni commented Jun 1, 2018

Updated the branches. Should compile on VS2013 now too.

@szanni
Copy link
Author

szanni commented Sep 26, 2018

Any chance of getting this merged? Anything blocking this pr?

@mischnic
Copy link
Contributor

👍 from a bindings perspective. Currently in proton-native, updating a slider's range means removing all children (because there is no uiBoxInsertAt or uiBoxRemoveAt, but that's another issue), replacing the slider element and readding all children.

@andlabs andlabs added this to the Unassigned Backlog milestone Dec 31, 2018
szanni added 2 commits June 30, 2019 16:31
…tion.

Some applications need to modify the slider range at runtime.
Instead of re-creating a slider with a new range, provide a function
to modify the range of an existing uiSlider.
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.

4 participants