Store GLTF model animation names

[alfredbaudisch]

• Added ModelAnimation.name in order to keep GLTF animation names.
• Updated models_loading_gltf, showing the name of the animation currently playing at the bottom

Why?

• In the creation of tools, it allows picking animations by name
• In the creation of a game, it allows choosing an animation to run by name instead of only by array position (currently by being based only by index position it's either error prone or confusing/hard).

[hanaxar]

A small note about strncpy
No null-character is implicitly appended at the end of destination if source is longer than num. Thus, in this case, destination shall not be considered a null terminated C string (reading it as such would overflow).
So I suggest manually adding a null terminator after copying string, something like this:
animations[i].name[sizeof(animations[i].name) - 1] = '\0';

[alfredbaudisch]

Thanks for the tip.

What I did was copy how it is already done for the bone names https://github.com/raysan5/raylib/blob/master/src/rmodels.c#L4735 which also does not close the bone name as a null terminated string:
strncpy(bones[i].name, node.name, sizeof(bones[i].name));

So shouldn't then the null be added to both places?

[hanaxar]

I think it should be added because node is a cgltf_node and in that struct name field is defined as char*
https://github.com/raysan5/raylib/blob/master/src/external/cgltf.h#L651
I'm not sure about length of node names in gltf models, but in rmodels bone names are harcoded to be 32 chars long.
Imho null terminating is necessary for safety when using strncpy. That function supposed to be a safer version of strcpy but it can still overflow if source is longer than n chars, and that's kind of annoying.

[orcmid]

I am fond of strncat_s() in these situations, along with ensuring there is always an extra \0 beyond where I can copy.

    char *pVCrayVer = getenv("VCRAYVER");
    if (pVCrayVer == NULL)
         pVCrayVer = "unidentified version";

    char verstring[LINE_MAX+1] = { '\0' };

    strncat_s( verstring, LINE_MAX,
               pVCrayVer, _TRUNCATE);
        // capturing VCRAYVER

LINE_MAX is about 80 in this case, and I was being wasteful. I don't know if _TRUNCATE is defined in all modern string.h implementations. The value is -1.

Efficiency isn't important in this particular code. and the technique is handy for building lines of text for display.

char line2[LINE_MAX+1] = { '\0' };

    strncat_s( line2, LINE_MAX,
               "Using raylib ", _TRUNCATE);
    strncat_s( line2, LINE_MAX,
               verstring, _TRUNCATE);

More than needed here, but it helps avoid length errors and over-writing a terminating \0 in storage of a char*.

[alfredbaudisch]

@hanaxar @orcmid so I'm a bit confused due to code conventions/style, as I haven't contributed to raylib before. What approach should I go then?

For now, I force-pushed with @hanaxar tip https://github.com/raysan5/raylib/pull/3044/files#diff-6cc7e70947ba59c1411280dbc0316d4025327b9050db405c56ffb3ddc9555f75R5383

[raysan5]

@alfredbaudisch I'm merging current improvement. I will review strncpy() usage, not only there but all around raylib, maybe replacing it by memcpy()...
My only concern with this PR was the addition of 32 extra bytes to ModelAnimation struct but still it's not that big so I think it's acceptable... I also updated that info on raylib Wiki.

[Peter0x44]

strncpy can be replaced by memcpy here, with change in behavior.
I think it is a function with close to zero valid uses.

[hanaxar]

Maybe a new function in rtext can be a good solution for replacing for all strncpy() uses in raylib.
Something like void TextNCopy(char *dst, const char *src, int length) which uses memcpy() and deals with all that unsafe cases.

EDIT: I was about to open a new PR about it, but then I noticed everything will be dependent on rtext and also on SUPPORT_TEXT_MANIPULATION definition, and probably that's not a good idea.