Immediate-Mode-UI / Nuklear

A single-header ANSI C immediate mode cross-platform GUI library
https://immediate-mode-ui.github.io/Nuklear/doc/index.html
Other
9.25k stars 562 forks source link

nuklear_group #202

Open exchg opened 4 years ago

exchg commented 4 years ago

https://github.com/Immediate-Mode-UI/Nuklear/blob/d2550448f3f457194728f1f5bf5a31cb1e0c486b/src/nuklear_group.c#L45-L56

Because this function now return boolean this code is senseless or not ?

If not, maybe this is better variant?

 ctx->current = win; 
 if (panel.layout->flags & (NK_WINDOW_CLOSED | NK_WINDOW_MINIMIZED)) 
     nk_group_scrolled_end(ctx); 
 return true; 
dumblob commented 4 years ago

@aganm what was your vision in this case?

I think we needed to distinguish it, so we baked it in like that. But it seems now is the time to reconsider whether distinguishing the state could be done any better.

aganm commented 4 years ago

Do you know if the return state is used by anybody? The documentation for the function says https://github.com/Immediate-Mode-UI/Nuklear/blob/d2550448f3f457194728f1f5bf5a31cb1e0c486b/src/nuklear.h#L2503

If we can make sure the return state isn't used, I would remove those 4 lines https://github.com/Immediate-Mode-UI/Nuklear/blob/d2550448f3f457194728f1f5bf5a31cb1e0c486b/src/nuklear_group.c#L51-L54

exchg commented 4 years ago

If we can make sure the return state isn't used, I would remove those 4 lines

If not, we should rollback the function return type to 'int'.

https://github.com/Immediate-Mode-UI/Nuklear/blob/d2550448f3f457194728f1f5bf5a31cb1e0c486b/src/nuklear_group.c#L9-L12

dumblob commented 4 years ago

Do you know if the return state is used by anybody?

I think I remember adding this as someone requested that and we forgot to update the doc. The more I think about this the more I'm inclining to returning it to int.

aganm commented 4 years ago

The bigger issue is the function has a chain effect. It's being called in some functions which are called in other functions. All of these functions are documented as returning a boolean value.

I propose to add a function such as nk_panel_get_flags() instead. The return flags can be removed in the next major update (being replaced by nk_panel_get_flags()).