This is a follow up from https://github.com/Immediate-Mode-UI/Nuklear/pull/883
Change the field type in nk_style_slider and nk_style_scrollbar to nk_bool.
I would rather not do this. Not only there are way more int fields with bool semantics like these two, there is simply no other use of nk_bool type anywhere in the style structs (EDIT: I think I found one somewhere). If you want these two changed, you should probably also change other similar fields elsewhere [...]
I may missing something, but I’m not entirely sure why this issue is being generalized to such a broad scope in the discussion. [...] broadening the problem to all similar cases, the conclusion naturally becomes that fixing it would be complex and risky. To me, this feels like a conceptual trap.
Those were the only ones that we had the code coverage to catch. [...] If you want the struct members changed, go open a new PR/Issue for it. I'm not going to do this myself (you already know my opinion on this).
https://github.com/Immediate-Mode-UI/Nuklear/blob/25f33c8780a8b41668447f83e7d154e735cf424f/src/nuklear.h#L4642 https://github.com/Immediate-Mode-UI/Nuklear/blob/25f33c8780a8b41668447f83e7d154e735cf424f/src/nuklear.h#L5038 https://github.com/Immediate-Mode-UI/Nuklear/blob/25f33c8780a8b41668447f83e7d154e735cf424f/src/nuklear.h#L5134 https://github.com/Immediate-Mode-UI/Nuklear/blob/25f33c8780a8b41668447f83e7d154e735cf424f/src/nuklear.h#L5359 https://github.com/Immediate-Mode-UI/Nuklear/blob/25f33c8780a8b41668447f83e7d154e735cf424f/src/nuklear.h#L5741
^ Those are the ones I was able to quickly find. There are probably more.
I personally do NOT want to change any of this. It can cause more issues like https://github.com/Immediate-Mode-UI/Nuklear/issues/812 and this isn't affecting any new code anyway. I gave many reasons under https://github.com/Immediate-Mode-UI/Nuklear/pull/883 why changing any of those struct fields is too risky.
I'm only reporting this for the sake of documenting. I will not be doing anything about it myself, and would suggest everyone else to ignore it, at least for now. Seriously, fixing this isn't worth it and may break someone's code.
Activity
19
sleeptightAnsiC8mo agoAnd also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example: unsigned int instead of nk_uint). So this isn't really only about nk_bool at this point (but the bool ones are the most obvious)
sleeptightAnsiC8mo agoAnd also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example: unsigned int instead of nk_uint). So this isn't really only about nk_bool at this point (but the bool ones are the most obvious)
sleeptightAnsiC8mo agoAnd also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example: unsigned int instead of nk_uint). So this isn't really only about nk_bool at this point (but the bool ones are the most obvious)
sleeptightAnsiC8mo agoAnd also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example: unsigned int instead of nk_uint). So this isn't really only about nk_bool at this point (but the bool ones are the most obvious)
sleeptightAnsiC8mo agoAnd also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example: unsigned int instead of nk_uint). So this isn't really only about nk_bool at this point (but the bool ones are the most obvious)
sleeptightAnsiC8mo agoAnd also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example: unsigned int instead of nk_uint/nk_uint32). So this isn't really only about nk_bool at this point (but the bool ones are the most obvious)
sleeptightAnsiC8mo agoAnd also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example: unsigned int instead of nk_uint/nk_uint32). So this isn't really only about nk_bool at this point (but the bool ones are the most obvious)
RobLoach8mo agoI think it does warrant cleaning up the data types. Understand that it could break compatibility, but honestly, a nk_bool should be used for boolean-cases 🙃
RobLoach8mo agoI think it does warrant cleaning up the data types. Understand that it could break compatibility, but honestly, a nk_bool should be used for boolean-cases 🙃
RobLoach8mo agoI think it does warrant cleaning up the data types. Understand that it could break compatibility, but honestly, a nk_bool should be used for boolean-cases 🙃
RobLoach8mo agoI think it does warrant cleaning up the data types. Understand that it could break compatibility, but honestly, a nk_bool should be used for boolean-cases 🙃
RobLoach8mo agoI think it does warrant cleaning up the data types. Understand that it could break compatibility, but honestly, a nk_bool should be used for boolean-cases 🙃
RobLoach8mo agoI think it does warrant cleaning up the data types. Understand that it could break compatibility, but honestly, a nk_bool should be used for boolean-cases 🙃
RobLoach8mo agoI think it does warrant cleaning up the data types. Understand that it could break compatibility, but honestly, a nk_bool should be used for boolean-cases 🙃
sleeptightAnsiC5mo agoNot exactly the same issue but very similar one: there are lots of enum types with only two members. Many of those are used in situations where simple bool would be fine. Most are nothing else than literal nk_bool alias: https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L285 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L287-L288 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L1000 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L4665-L4668 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L4791-L4794 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear_internal.h#L127 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear_internal.h#L198-L201
sleeptightAnsiC5mo agoNot exactly the same issue but very similar one: there are lots of enum types with only two members. Many of those are used in situations where simple bool would be fine. Most are nothing else than literal nk_bool alias: https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L285 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L287-L288 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L1000 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L4665-L4668 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L4791-L4794 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear_internal.h#L127 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear_internal.h#L198-L201
sleeptightAnsiC5mo agoNot exactly the same issue but very similar one: there are lots of enum types with only two members. Many of those are used in situations where simple bool would be fine. Most are nothing else than literal nk_bool alias: https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L285 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L287-L288 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L1000 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L4665-L4668 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear.h#L4791-L4794 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear_internal.h#L127 https://github.com/Immediate-Mode-UI/Nuklear/blob/859d17b71a6c44836d0dc1ced897ec41707ac670/src/nuklear_internal.h#L198-L201
sleeptightAnsiC4mo agoThere are few that use unsigned char but behave like boolean. In some cases char was used for getting specific padding and struct size, and I guess it may not be so trivial to swap it. I'm also pretty sure at least one of these came from STB, and there might be more in internal code.
https://github.com/Immediate-Mode-UI/Nuklear/blob/e634a0dfa1b235a45a09a6d09d78b59af663f2e3/src/nuklear.h#L4080-L4082 https://github.com/Immediate-Mode-UI/Nuklear/blob/e634a0dfa1b235a45a09a6d09d78b59af663f2e3/src/nuklear.h#L4398-L4402 https://github.com/Immediate-Mode-UI/Nuklear/blob/e634a0dfa1b235a45a09a6d09d78b59af663f2e3/src/nuklear.h#L4727-L4729 https://github.com/Immediate-Mode-UI/Nuklear/blob/e634a0dfa1b235a45a09a6d09d78b59af663f2e3/src/nuklear.h#L5574
And also, for anyone who ever comes here: if you take a look at the structs in Nuklear, you will notice that most of the fields use wrong types in one way or another (for example:
unsigned intinstead ofnk_uint). So this isn't really only aboutnk_boolat this point (but the bool ones are the most obvious)