RandyGaul / cute_framework

The *cutest* framework out there for creating 2D games in C++!
https://randygaul.github.io/cute_framework/#/
Other
543 stars 32 forks source link

C interface with optional C++ API #45

Closed jonstvns closed 2 years ago

jonstvns commented 2 years ago

What are we doing?

Converting cute_framework headers to be C compatible, while leaving an optional namespaced C++ API in place.

What work is remaining?

Per-file progress

Notes from discord chat

[10:20 PM] Randy: The main problem would be the usage of templated data structures in the implementation of some of the features, like the serialization and the ECS
[10:21 PM] Randy: However, it's possible to compile a C interface and leave the implementation using some C++
[10:21 PM] Randy: Then the main pain point is just some of the templated stuff in the API, but it's not too much
[10:21 PM] Limyc: I'm mostly interested in a c interface, and if it's just some grunt work, I might just do it
[10:22 PM] Randy: It's not a big deal to write C versions of function overloads and whatnot
[10:22 PM] Randy: I would absolutely support you here. I would do it myself, but priorities dictate I work on the other things first, like docs and bugs
[10:22 PM] Randy: So if you do it, that would super awesome
[10:25 PM] Limyc: is there a particular way you would want the interface work done?
[10:26 PM] Randy: https://github.com/RandyGaul/cute_headers/blob/master/cute_sound.h
GitHub
[cute_headers/cute_sound.h at master · RandyGaul/cute_headers](https://github.com/RandyGaul/cute_headers/blob/master/cute_sound.h)
Collection of cross-platform one-file C/C++ libraries with no dependencies, primarily used for games - cute_headers/cute_sound.h at master · RandyGaul/cute_headers
cute_headers/cute_sound.h at master · RandyGaul/cute_headers
[10:27 PM] Randy: This is a pretty good way to create C versions without default arguments
[10:27 PM] Randy:
cs_audio_source_t* cs_load_wav(const char* path, cs_error_t* err /* = NULL */);
cs_audio_source_t* cs_read_mem_wav(const void* memory, size_t size, cs_error_t* err /* = NULL */);
[10:27 PM] Randy: In each header you can #ifdef __cplusplus the C++ header, and in the #else have the C versions
[10:28 PM] Randy: For overloaded functions you can append 2 to the name
[10:28 PM] Randy: Should probably also have a macro to force C-only as an optional macro
[10:29 PM] Limyc: Would you want to keep the namespaced C++ functions wrapped with #ifdef _cplusplus, and those just call into the cf_* functions?
[10:29 PM] Randy: Yeah I think that's a good way to do it
[10:29 PM] Randy: In the places where an array<T> is returned there's a few options on what to do
[10:30 PM] Randy: We can probably discuss those case-by-case
[10:33 PM] Randy: cute_string.h should probably not have a C version at all
[10:33 PM] Randy: I see array<T> in cute_ecs.h and cute_https.h
[10:33 PM] Randy: but those should be easy to change to a C-style implementation
[10:35 PM] Randy: Overall it's a straightforward port to C
[10:35 PM] Limyc: okay, so globals, functions, types all get cf_
overloads get a numerical postfix.
keep the namespaced functions wrapped by an ifdef
deal with templates on a case-by-case basis
[10:35 PM] Randy: sounds pretty good
[10:35 PM] Randy: And ask questions if you need help with anything, we can always discuss things if you're unsure which option to take
[10:37 PM] Limyc: do you want to alias types inside the cpp namespace for convenience and not breaking anyone who updates?
namespace cute {
using ecs_entity_t = cf_ecs_entity_t;
}

[10:37 PM] Randy: yeah that's a good idea
[10:38 PM] Randy: that works well
[10:38 PM] Randy: Probably the big one here would be the sprite
[10:39 PM] Randy: https://github.com/RandyGaul/cute_framework/blob/master/include/cute_handle_table.h
[10:39 PM] Randy: Here's a similar example to how the sprite can be implemented. The C version is the "main" version and the C++ one just calls into the C functions and holds the struct instance
[10:40 PM] Randy: But the sprite would still just be POD style as opposed to the handle_table is heap allocated
jonstvns commented 2 years ago

Should I duplicate comments for the cf_ and namespaced functions?

jonstvns commented 2 years ago

When an exported struct or function uses an array<T>, should that be replaced with a typeless_array or a raw pointer? typeless_array uses constructors, we'd have to make sure we add a CUTE_FREE anywhere a destructor would normally be called

This same issue will come up with priority_queue<T>. lru_cache won't be useable from a C api without writing a typeless version.

Ideally, the C++ API can keep the templated types, and then pass in wrapped or raw data into the C api. Idk how well that will work in practice.

RandyGaul commented 2 years ago

Should I duplicate comments for the cf_ and namespaced functions?

We can just document the C functions and leave the C++ ones with no comments.

When an exported struct or function uses an array, should that be replaced with a typeless_array or a raw pointer?

It should probably just be a const pointer and integer length, which would require a free function to also be called. So no constructors or anything fancy.

This same issue will come up with priority_queue. lru_cache won't be useable from a C api without writing a typeless version.

We don't need to create C style data structures for the templated C++ ones. The only reason I have typeless array is because I actually needed it to implement the ECS, not because I wanted to provide C versions of C++ templated data structures. So the C API wouldn't have an array, prioritity_queue, or dictionary.

jonstvns commented 2 years ago

@RandyGaul let me know what you think of the changes in the above commit in cute_a_star.h/cpp, cute_gfx.h/cpp, and cute_defines.h. I added some comments for context.

jonstvns commented 2 years ago

cute_a_star.h is considered done

jonstvns commented 2 years ago

Finished renaming all the types, functions, and globals with a cf_ prefix

jonstvns commented 2 years ago

cute_gfx.h and cute_handle_table.h are considered done

RandyGaul commented 2 years ago

Looks good!

jonstvns commented 2 years ago

@RandyGaul lmk your thoughts on cute_ecs.h. I left a few comments

https://github.com/SomeSoftwareDev/cute_framework/commit/a95972fefb7738e321086b074827333a31df48ab

jonstvns commented 2 years ago

Also, do we want extern "c" to be defined as a block in the header or per function?

jonstvns commented 2 years ago

@RandyGaul I could use some feedback on the changes to cute_https.h and .cpp. The signature of function cf_https_response was changed to accommodate previous usage of array<t> inside cf_https_response_t.

There's now a cf_internal_https_response_t inside cute_https.cpp, and the values in there are copied out to a cf_https_response_t instead of directly returning a pointer to the internal response member like the C++ api does.

https://github.com/SomeSoftwareDev/cute_framework/commit/82cbfb4ba3a43a71b4b5fbf8b087f465f3ac573a

RandyGaul commented 2 years ago

cf_https_response can return a cf_https_response_t rather than a bool. Otherwise looks good!

Also, do we want extern "c" to be defined as a block in the header or per function?

Blocks is good, not per function.

RandyGaul commented 2 years ago

For ecs header, lets have all those get functions return **, instead of taking an out param. Triple pointer will confuse everyone. Let's also add a NULL entry as the last pointer in each array, and also allow the the out count param to be NULL.

Example:

CUTE_API const char** CUTE_CALL cf_ecs_get_entity_list(int* entities_count_out);

Then people can write a loop like this:

const char** list = cf_ecs_get_entity_list(NULL);
while (*list) {
    do_stuff(*list);
    ++list;
}
cf_ecs_free_list(list);

Your implementation looks good otherwise!

RandyGaul commented 2 years ago

Found small typo

image

jonstvns commented 2 years ago

Yeah, I realized the triple pointer might be confusing, hence the feedback request. Thanks!

RandyGaul commented 2 years ago

Really good work! Ping me for more feedback as needed

jonstvns commented 2 years ago

@RandyGaul Are these function names good in cute_kv.h?

  1. https://github.com/SomeSoftwareDev/cute_framework/commit/924b8662412059a74c48947d1a1b867074e8f7a8#r77327654
  2. https://github.com/SomeSoftwareDev/cute_framework/commit/924b8662412059a74c48947d1a1b867074e8f7a8#r77327676
RandyGaul commented 2 years ago

Looks pretty good to me

jonstvns commented 2 years ago

@RandyGaul The changes to cute_math.h need a thorough review.

A lot of overloaded functions were renamed, and while I tried to be consistent, sometimes it made more sense to me to postfix with full types, sometimes abbreviated types, and sometimes with numbers. It'd be great if you could help adjust any naming changes.

All the math operations inside headers were converted to use functions instead of operator overloads. I didn't see any translation errors, but it'd be good to check.

https://github.com/SomeSoftwareDev/cute_framework/commit/e331c8028ab1e69a657345355faa083770c5a2b0#diff-2727d9eef5cd85a7c616c9b8a917469c62a074ba84918767322914a871564e56

jonstvns commented 2 years ago

Hid cf_string_t when compiling as C. It's only accessible in C++ mode, even when using the C api

jonstvns commented 2 years ago

still need to replace cf_dictionary<T> with cf_hash_table_t in cf_sprite_t

RandyGaul commented 2 years ago

You did a really good job with the math naming! I added my comments in the commit you linked above. Good choice on the C++ string -- it's going to likely be deleted in the future anyways, or heavily refactored, and is a C++ only sort of thing anyways. In C we can use the strpool API which is way simpler and a better API imo.

jonstvns commented 2 years ago

For sprite.h, I made a small breaking change to the C++ api for cf_animation_t and cf_animation_table_t. I did not try to keep animation_table_t a dictionary and I did not try to keep animation_t-frames a cf_array.

Supporting that would be annoying and not worth it in this case, imo. I just added the equivalent functions for mutation, so all the C++ folks have to do it adjust a few call sites.

jonstvns commented 2 years ago

All that's left is code review and verifying that everything still compiles for emscripten

jonstvns commented 2 years ago

Oh, and we need to merge the changes that were pushed since I began this task

jonstvns commented 2 years ago

Merging complete. Hope you don't have any local changes