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

CLAY_ID uniqueness #276

Open
Roucou opened this issue Feb 19, 2025 · 13 comments
Open

CLAY_ID uniqueness #276

Roucou opened this issue Feb 19, 2025 · 13 comments

Comments

@Roucou
Copy link

Roucou commented Feb 19, 2025

I'm struggling with the CLAY_ID concept, and watching the great sets of videos by David Styrbjörn, I'm not the only one.

Most examples you find on defining your own "components" do something like this:

void button_element(const char*clay_id, Texture2D *texture) {
    CLAY({
            .id = CLAY_ID(clay_id),
            .image = {
                    .imageData = texture,
                    .sourceDimensions = { texture->width, texture->height }
            },
            .layout = { .sizing = {
                    .width = CLAY_SIZING_FIXED(texture->width),
                    .height = CLAY_SIZING_FIXED(texture->height),
            }}
    }) {}
}

But using clang, that doesn't even compile, it will complain about the CLAY_ID(clay_id) bit. A naive way around this would be:

void button_element(Clay_ElementId *clay_id, Texture2D *texture) {
    CLAY({
            .id = *clay_id,
            .image = {
                    .imageData = texture,
                    .sourceDimensions = { texture->width, texture->height }
            },
            .layout = { .sizing = {
                    .width = CLAY_SIZING_FIXED(texture->width),
                    .height = CLAY_SIZING_FIXED(texture->height),
            }}
    }) {}
}

And then call this function as such:

Clay_ElementId element_id = CLAY_ID("button1");
Texture_ID texture_id = TEX_BUTTON1;
Texture2D texture = textures[texture_id];
button_element(&element_id, &texture);
element_id = CLAY_ID("button2");
texture_id = TEX_BUTTON2;
texture = textures[texture_id];
button_element(&element_id, &texture);

But that somehow creates non-unique CLAY_ID's. A quick-fix would be to give each call its own local scope:

{
  Clay_ElementId element_id = CLAY_ID("button1");
  Texture_ID texture_id = TEX_BUTTON1;
  Texture2D texture = textures[texture_id];
  button_element(&element_id, &texture);
}
{
  Clay_ElementId element_id = CLAY_ID("button2");
  Texture_ID texture_id = TEX_BUTTON2;
  Texture2D texture = textures[texture_id];
  button_element(&element_id, &texture);
}

Or, as David is doing, create static const char* id's in the global scope.

I think you are supposed to generate the id's on the heap, using malloc, instead of on the stack, but that requires a lot of house-holding. And it all feels like super convoluted for something that should be super simple as creating a unique id. :-)

Any advice on this, i.e. is the CLAY_ID macro working correctly, and if so, how is a developer supposed to work with them?

@emoon
Copy link
Contributor

emoon commented Feb 19, 2025

Have a look at these https://github.com/nicbarker/clay/blob/main/clay.h#L70-L74

It allows you to do

CLAY_IDI("name", value) where value is an internal counter you you can set to a arbitrary value (such as a counter for example)

@Roucou
Copy link
Author

Roucou commented Feb 19, 2025

Thanks, Daniel. I think CLAY_IDI however has the same issues as CLAY_ID.

  1. The label argument needs to be a string literal. Using the clang compiler, this gives inconsistent behavior:
Clay_ElementId element_id = CLAY_ID("button1"); // works
// or
Clay_ElementId element_id = CLAY_IDI("button1", 0); // works

static const char *BUTTON1 = "button1";
Clay_ElementId element_id = CLAY_ID(BUTTON1); // doesn't work, shows "expected ')'" error
// or
Clay_ElementId element_id = CLAY_IDI(BUTTON1, 0); // doesn't work, shows "expected ')'" error

#define BUTTON1 "button1"
Clay_ElementId element_id = CLAY_ID(BUTTON1); // works
// or
Clay_ElementId element_id = CLAY_IDI(BUTTON1, 0); // works

// ---

static const char *BUTTON1 = "button1";
//or
#define BUTTON1 "button1"
button_element(BUTTON1, &texture);
// ...
void button_element(const char *clay_id, Texture2D *texture) {
  CLAY({
    .id = CLAY_ID(clay_id), // doesn't work, shows "expected ')'" error
    // ...
} 
  1. Using multiple CLAY_ID or CLAY_IDI calls in the same scope doesn't create unique IDs. See example in original post.

Again, pretty sure this is entirely related to my defunct C skills, but hoping to learn more. Also curious to learn about the design choice behind making the Clay_Element_Id more complex than something like a simple uint.

@emoon
Copy link
Contributor

emoon commented Feb 19, 2025

The idea is that you use it like this

Clay_ElementId element_id = CLAY_IDI("button", 0);
Clay_ElementId element_id = CLAY_IDI("button", 1);
etc
...

@Roucou
Copy link
Author

Roucou commented Feb 19, 2025

Thanks, Daniel, I get that, and I build my approach around that macro now:

typedef enum Clay_ID {
  CLAY_BUTTON1,
  CLAY_BUTTON2,
  //...
} Clay_ID;

void draw(...) {
  button_element(CLAY_BUTTON1, texture1);
  button_element(CLAY_BUTTON1, texture2);
}

void button_element(Clay_ID clay_id, Texture2D *texture) {
    CLAY({
            .id = CLAY_IDI("BTN", clay_id),
            .image = {
                    .imageData = texture,
                    .sourceDimensions = { texture->width, texture->height }
            },
            .layout = { .sizing = {
                    .width = CLAY_SIZING_FIXED(texture->width),
                    .height = CLAY_SIZING_FIXED(texture->height),
            }}
    }) {}
}

A bit of a hack, but it works. Would have loved to just pass something like "btn_new" as a parameter for the button_element call. Still super curious to understand why that doesn't work. It seems to work for David in his vids, and it's also used that way in the documentation and examples provided by Nic. It seems the CLAY_ID and CLAY_IDI marcros don't accept what I think should be a correct string literal (i.e. my static const char * or #define attempts). It hits the CLAY__ENSURE_STRING_LITERAL safeguard Nic build into the framework. Maybe it's a clang compiler thing? No clue.

And the other issue about CLAY_ID not generating a unique ID was entirely stupidity on my end:

// STUPID
Texture2D texture = textures[1];
button_element(CLAY_BUTTON1, &texture);
texture = textures[2];
button_element(CLAY_BUTTON2, &texture);

// CORRECT
Texture2D *texture = &textures[1];
button_element(CLAY_BUTTON1, texture);
texture = &textures[2];
button_element(CLAY_BUTTON2, texture);

Rookie mistake :-(

@emoon
Copy link
Contributor

emoon commented Feb 19, 2025

The reason that CLAY__ENSURE_STRING_LITERAL is there is because of lifetime issues with strings. Take this example

{
  char temp_data[10];
  sprintf(temp_data, "name %d", count);
  CLAY({ .id = CLAY_ID(temp_data) ...
}

If the above would be allowed the id string would now point to invalid memory after the code is leaving the scope. By using the CLAY__ENSURE_STRING_LITERAL the code will make sure that only static strings has been passed to the system. That being said if you know your own ownership of the strings you can pass the string in in manually. If you look at

Clay_ElementId Clay__HashString(Clay_String key, uint32_t offset, uint32_t seed);

You can create your own Clay_String being passed ino.

@nicbarker
Copy link
Owner

Btw @Roucou just making sure that you're aware of CLAY_ID_LOCAL? It allows you to still specify a string ID for the button, but won't conflict with other identical strings in the global namespace 🙂

@Roucou
Copy link
Author

Roucou commented Feb 19, 2025

Yes, thanks, Nic, I am aware of that macro, but all the macros have the same issue, they expect a string literal, and whatever silly things I try (static const char *id, or #define ...), I cannot get past the CLAY__ENSURE_STRING_LITERAL guardrail.

Look at the very first example in my original post, I cannot find a way to pass a string ID to the button_element function without hitting the CLAY__ENSURE_STRING_LITERAL guardrail. I would think if I declare a static const char *id in the global scope, this should work, but it doesn't. Meanwhile tried with both clang and gcc, both hit the CLAY__ENSURE_STRING_LITERAL guardrail.

Hence I settled on the CLAY_IDI macro for now, and only pass an int value. But that does mean the buttons have quite generic names ("button0", "button1", "button2", ...) instead of more sensible names ("btn_new", "btn_left", "btn_shoot", ...)

Weird thing is, the static const char *id thing seemed to work for David in his video (https://youtu.be/By4-VRL-5a0?t=2835), so no idea why I cannot get it to work. He does seem to be using a slightly older version of Clay, so maybe the guardrail was put in later.

And I understand why it is there, as per Daniel's explanation above. Maybe I was secretly hoping Clay was more forgiving for newbies. :-) It does a lot of cool stuff with memory allocation, so maybe in a future version Clay can copy the element ID to the arena (?) so it doesn't hurt if newbies allocate the string on the local stack where it gets lost a soon as the scope changes.

@nicbarker
Copy link
Owner

Ahhh yes I totally see the issue now, thanks for clarifying.
It's a tricky balance - we actually put that guardrail in there to make clay more forgiving to newer people (newer to C, that is) 😅
It would certainly make things easier to just copy the string into our internal arena, but because we're using fixed size memory it makes it difficult to estimate how much memory is required for ID strings, warn the user if it runs out, make that value configurable etc.

I think generally the best approach at the moment is for your function to take a Clay_ElementId directly, and generate it in the parent scope using CLAY_ID_LOCAL:

void CreateButton(Clay_ElementID id) {
    CLAY({ .id = id }) {}
}

// these will all be unique
CreateButton(CLAY_ID_LOCAL("btn_new"));
CreateButton(CLAY_ID_LOCAL("btn_left"));

@FintasticMan
Copy link
Contributor

We could make the CLAY_ID etc macros take a Clay_String? That way you could pass both a static string using CLAY_STRING as well as a differently-allocated string.

@nicbarker
Copy link
Owner

I have definitely considered that, the only issue is that it adds quite a significant amount of boilerplate to standard UI layouts, e.g.

CLAY({ .id = CLAY_ID(CLAY_STRING("OuterContainer")) })

@Roucou
Copy link
Author

Roucou commented Feb 21, 2025

@nicbarker the increased boilerplate code could maybe be resolved by adding a CLAY_SID macro converting the regular C string argument into a Clay_String. I.e. something like:

#define CLAY_SID(label) CLAY_ID(CLAY_STRING(label))

But not sure if it resolves anything, because the CLAY_STRING macro also calls the CLAY__ENSURE_STRING_LITERAL guardrail.

Anyway, for now I've settled on the CLAY_IDI macro, as suggested by @emoon. I really want to define my ID's only once, and then reused throughout my code. The CLAY_IDI macro allows me to simply define my ID's as enums, which brings the benefit of compile-time error-checking and IDE auto-completion.

typedef enum Clay_ID {
    CLAY_BTN_MAIN_MENU_NEW_ID,
    CLAY_BTN_MAIN_MENU_SETTINGS_ID,
    CLAY_BTN_MAIN_MENU_HELP_ID,
    CLAY_BTN_MAIN_MENU_EXIT_ID,
} Clay_ID;

void layout(...) {
   button_element(CLAY_BTN_MAIN_MENU_NEW_ID, &textures[TEX_BUTTON_NEW]);
   button_element(CLAY_BTN_MAIN_MENU_SETTINGS_ID, &textures[TEX_BUTTON_SETTINGS]);
   button_element(CLAY_BTN_MAIN_MENU_HELP_ID, &textures[TEX_BUTTON_HELP]);
   button_element(CLAY_BTN_MAIN_MENU_EXIT_ID, &textures[TEX_BUTTON_EXIT]);
   // ...
}

void button_element(Clay_ID clay_id, Texture2D *texture) {
    CLAY({
            .id = CLAY_IDI("BTN", clay_id),
            .image = {
                    .imageData = texture,
                    .sourceDimensions = { texture->width, texture->height }
            },
            .layout = { .sizing = {
                    .width = CLAY_SIZING_FIXED(texture->width),
                    .height = CLAY_SIZING_FIXED(texture->height),
            }}
    }) {}
}

The CLAY__ENSURE_STRING_LITERAL guardrail prevents my desired pattern, i.e. I cannot get it to accept #define or static const char*, so instead I'd be personally responsible to ensure I always type the string literal in exactly the same way. I'd rather have the IDE and compiler preventing me from being an idiot. ;-p

Thanks for all the help and feedback, very much appreciated! And now, back to actually writing my game. :-)

@FintasticMan
Copy link
Contributor

The CLAY__ENSURE_STRING_LITERAL guardrail prevents my desired pattern, i.e. I cannot get it to accept #define or static const char*, so instead I'd be personally responsible to ensure I always type the string literal in exactly the same way. I'd rather have the IDE and compiler preventing me from being an idiot. ;-p

static const char * wouldn't work even without the guardrail, because sizeof returns the size of the pointer rather than the size of the string. It should be possible however to make the CLAY__ENSURE_STRING_LITERAL accept #defined string literals, which should allow your use-case more easily. I'm going to have a play around with it today, and if I can get it to work, I'll submit a PR.

@FintasticMan
Copy link
Contributor

@Roucou the CLAY_ID functions should already accept #defined string literals:

#define TEST "test"

---

CLAY({.id = CLAY_ID(TEST)}) {}

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

No branches or pull requests

4 participants