Add nk_col_pick and nk_col_picker

[aganm]

All widget functions that have a color param take nk_color, except for nk_color_picker/pick that take nk_colorf instead.

This adds nk_col_pick and nk_col_picker wrappers which take a nk_color param and does the conversion to nk_colorf.

[dumblob]

Should we also mark the nk_color_picker/pick functions as deprecated and add to comment a very approximate estimation when they'll be removed (e.g. will be removed any time after 2023-12-14 i.e. in about 2 years from now)?

There is e.g. NK_UNUSED and alike so we could introduce NK_DEPRECATED which would make it clear it's not advised to be used for new code (it should ideally produce a warning during compilation also - at least in the biggest compilers like clang & gcc & msvc). Together with the estimated time of removal in the comment (and thus in the doc) this should prepare a good ground for future.

Thoughts @Hejsil and others?

[Hejsil]

Instead of a timeframe, maybe remove it at next major version bump? So the comment would be will be removed in v6.0.0 or something like that.

[RobLoach]

I think having both is fine, but would be nice to flip the names to match what argument they return. nk_colorf_picker picks a nk_colorf, while nk_color_picker picks a nk_color

NK_API struct nk_colorf nk_colorf_picker(struct nk_context*, struct nk_colorf, enum nk_color_format);
NK_API nk_bool nk_colorf_pick(struct nk_context*, struct nk_colorf*, enum nk_color_format);
NK_API struct nk_color nk_color_picker(struct nk_context*, struct nk_color, enum nk_color_format);
NK_API nk_bool nk_color_pick(struct nk_context*, struct nk_color*, enum nk_color_format);

To make the API switch, they would just add an f to nk_color_picker.

[dumblob]

Instead of a timeframe, maybe remove it at next major version bump? So the comment would be will be removed in v6.0.0 or something like that.

Yep, that's easier. Would you also like to see NK_DEPRECATED or just leave it up to the vigility of programmers to follow changes to the doc without affecting their compilation pipelines?

I think having both is fine,

Forever? I.e. even in new major versions you'd prefer to have both?

but would be nice to flip the names to match what argument they return.

Yep, in the next major version, this could be the default recommended way to name the API.

[Hejsil]

Instead of a timeframe, maybe remove it at next major version bump? So the comment would be will be removed in v6.0.0 or something like that.

Yep, that's easier. Would you also like to see NK_DEPRECATED or just leave it up to the vigility of programmers to follow changes to the doc without affecting their compilation pipelines?

Hmm good question. I'm not sure how -Werror effects deprecation warnings. We don't wonna break people who use -Werror but it would be nice if people who don't got a warning about the deprecation before the functions removal.

[dumblob]

Hmm good question. I'm not sure how -Werror effects deprecation warnings. We don't wonna break people who use -Werror but it would be nice if people who don't got a warning about the deprecation before the functions removal.

I wouldn't try to make it a compiler warning but rather an ordinary compiler message. Didn't look up any side effects but I'd first investigate #pragma message "my compile-time message\n".

[RobLoach]

I would recommend against even a #pragma message , as it could get quite annoying for builds that are not affected by the deprecated code.

How would it affect them? I mean, old code won't update nuklear.h to this newer version which would have the #pragma. Or what exactly did you have in mind?

I don't think #pragma message would affect any builds in any way. It'll merely add some more messages to the build output.

[RobLoach]

@aganm @dumblob @hejsil Is this good to merge? While there are some good suggestions for breaking changes, I think this is the best intermediate change that won't break anything. Consider the breaking/deprecating changes in a future issue.

[aganm]

@RobLoach It's good to merge.

[dumblob]

I would recommend against even a #pragma message , as it could get quite annoying for builds that are not affected by the deprecated code.

What do you mean? Builds not affected by the deprecated code wouldn't produce such message, would they?

How would it affect them? I mean, old code won't update nuklear.h to this newer version which would have the #pragma. Or what exactly did you have in mind?

I see 3 cases:

1. I'm already using Nuklear (i.e. its previous/older version). But I don't want to update Nuklear at all and thus I'm totally unaffected by anything.
2. I'm already using Nuklear (i.e. its previous/older version). And I do want to update Nuklear (because there are some bug fixes, new features, etc.). Then I certainly welcome that Nuklear tells me with 0 effort (i.e. during build) that I should also migrate my code to new API without breaking the build itself.
3. I'm starting a new app with Nuklear. Thus I won't use any deprecated API and use only the new one. Therefore I'll not see any messages about deprecated APIs during build.

Does this make sense?

I don't think #pragma message would affect any builds in any way. It'll merely add some more messages to the build output.

That's the point - it shouldn't affect the build itself, but the programmer (i.e. the output of the build which is the only communication channel - apart from exit code - which the compiler has and thus it's guaranteed the programmer will want to read the output).

[RobLoach]

Builds not affected by the deprecated code wouldn't produce such message, would they?

I thought #pragma message just displayed the message whether or not they are using the old methods. I may not be quite understanding how it would be used though. I could be wrong on that though. I've seen @deprecated be used in code comments before.

[dumblob]

@RobLoach perhaps the function would be a macro...

@deprecated is fine - but it's only for documentation (and not for build), so not enough on its own.

[RobLoach]

Having it as a macro is a great idea.