raysan5/raygui

Possible functions redesign to use pass-by-reference

raysan5 opened this issue · 5 comments

Some raygui controls reque one value to be passed and returned after being updated, those function could be redesigned to just pass the variable by reference for internal modification. This will simplify the function and also adds room for a possible return state value, like result of operation or similar (to be analyzed).

Affected functions:

Rectangle GuiScrollPanel(Rectangle bounds, const char *text, Rectangle content, Vector2 *scroll)

int GuiToggleGroup(Rectangle bounds, const char *text, int active);
int GuiComboBox(Rectangle bounds, const char *text, int active);
bool GuiCheckBox(Rectangle bounds, const char *text, bool checked);

float GuiSlider(Rectangle bounds, const char *textLeft, const char *textRight, float value, float minValue, float maxValue);
float GuiSliderBar(Rectangle bounds, const char *textLeft, const char *textRight, float value, float minValue, float maxValue);
float GuiProgressBar(Rectangle bounds, const char *textLeft, const char *textRight, float value, float minValue, float maxValue);

int GuiListView(Rectangle bounds, const char *text, int *scrollIndex, int active);
Color GuiColorPicker(Rectangle bounds, const char *text, Color color);

Vector2 GuiGrid(Rectangle bounds, const char *text, float spacing, int subdivs);

Possible review:

int GuiScrollPanel(Rectangle bounds, const char *text, Rectangle content, Vector2 *scroll, Rectangle *view);

int GuiToggleGroup(Rectangle bounds, const char *text, int *active);    // Return result=1 if toggle happened
int GuiComboBox(Rectangle bounds, const char *text, int *active);   // Return result=1 if toggle happened
int GuiCheckBox(Rectangle bounds, const char *text, bool *checked);  // Return result=1 if change happened

int GuiSlider(Rectangle bounds, const char *textLeft, const char *textRight, float *value, float minValue, float maxValue);
int GuiSliderBar(Rectangle bounds, const char *textLeft, const char *textRight, float *value, float minValue, float maxValue);
int GuiProgressBar(Rectangle bounds, const char *textLeft, const char *textRight, float *value, float minValue, float maxValue); // Return result=1 if 0% reached or result=2 if 100% reached

int GuiListView(Rectangle bounds, const char *text, int *scrollIndex, int *active);  // Return result=1 if change happened
int GuiColorPicker(Rectangle bounds, const char *text, Color *color);   // Return result=1 if mouse pressed

int GuiGrid(Rectangle bounds, const char *text, float spacing, int subdivs, Vector2 *mouseCell);

Return values are optional. Consistency with other functions should also be considered.

In any case, it is a breaking change, probably to be considered for a raygui 4.0 release.

Complete redesign implemented in 4.0-dev branch: https://github.com/raysan5/raygui/tree/4.0-dev

As mentioned, this is a very breaking change, still considering if this is the best approach... Some of my thoughts about it:

PROS

  • All controls use a unified design, all follow same function signature:
    [result] Gui[ControlName](Rectangle bounds, const char *text, [Additional control parameters]);
  • All controls now allow a result return value, useful to report back control states or errors or extra control-internal info
  • User need to write less code to do the same and most controls could be visually aligned, improving code readability

CONS

  • Most controls now require pointers to deal with input/output data
  • Dealing with pointers is more prone to crash the app if missused
    • There is no way to validate if a pointer passed to a function is valid, it's up to the user to use it responsible
  • No backward compatibility, all programs using raygui need to be reviewed/updated

More feedback is welcome!

I think it is not a bad idea!
You can optionally make a macro for mentioning that it's a variable to output by making a macro as such:

#define RAYGUI_OUTPUT_VARIABLE

And using it as

int GuiScrollPanel(Rectangle bounds, const char *text, Rectangle content, Vector2 RAYGUI_OUTPUT_VARIABLE *scroll, Rectangle RAYGUI_OUTPUT_VARIABLE *view);

You can use something small like ROUT instead of long as RAYGUI_OUTPUT_VARIABLE
But hey it's just a suggestion! I think this change is fine to me! Might even make things easier!

@anstropleuton The pointer should be enough to note it's an output variable.

Okay sure!
I am up for this redesign!

I think for this kind of GUI, you are going to have to deal with pointers. Pass by ref is a concept programmers are going to have to deal with in C at some point.
This is use of pointers should not be as 'scarry' as memory allocaion.

I can't see any way around it, other than returning some large structure with new values and that just makes it a pain to use.

This method makes the API more consistent, and is inline with other APIs that do the same thing (Dear ImGui).

While I understand the desire to minimize pointers, I feel this is a well warranted case for use, and the resulting API will be much easier to use.