-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Added initial C support to configfile.h, extension.h and resource.h #7889
Conversation
# Conflicts: # engine/extension/src/dmsdk/extension/extension.h
# Conflicts: # engine/liveupdate/src/liveupdate.cpp # share/extender/build_input.yml
# Conflicts: # engine/dlib/src/dmsdk/dlib/configfile.h # engine/extension/src/dmsdk/extension/extension.h # engine/extension/src/extension.cpp
@@ -0,0 +1,166 @@ | |||
;; Copyright 2020-2023 The Defold Foundation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an old branch, originally created for the Zig example/presentation
@@ -0,0 +1,30 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example of how we'll generate api files from our C api's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To regenerate the headers, we'll run:
./scripts/build.py gen_sdk_source
To keep things separated, we use "resource.h" for the C header, "resource.hpp" for the manually crafted C++ header, and "resource_gen.hpp" for the automatically generated header.
These are stored in the repository, as they don't need to be regenerated all the time (although it's currently quick). Another good reason is that it's easier to do hot fixes to the builds while this new workflow is developing.
@@ -30,65 +30,182 @@ | |||
#include "sys.h" | |||
#include "static_assert.h" | |||
|
|||
namespace dmConfigFile | |||
struct ConfigFileEntry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still use C++ as the implementation file (at least for now), as it 1) saves time, and 2) we can use containers more easily.
* @typedef | ||
* @name HConfigFile | ||
*/ | ||
typedef struct ConfigFile* HConfigFile; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How new C handles will look like (quite similar to what we do today)
* } | ||
* | ||
*/ | ||
const char* ConfigFileGetString(HConfigFile config, const char* key, const char* default_value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the ConfigFile
is the prefix/namespace.
@@ -0,0 +1,260 @@ | |||
/*# Resource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the resource.h more readable at-a-glance, I moved some of the documentation here.
@@ -0,0 +1,92 @@ | |||
// Copyright 2020-2024 The Defold Foundation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved some of the functionality to separate cpp files for easier reading
c_includes = ['extension.h', | ||
'configfile.h', | ||
'resource.h'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We test build the C api headers using C compiler, to make sure it's strict C code.
# ------------------------------------------------------------ | ||
# Gen source files -> | ||
|
||
def _gen_sdk_source_lib(self, libname, args, cwd, info): | ||
self._log('Generating source for %s' % libname) | ||
libargs = args + ['-i', info] | ||
run.env_command(self._form_env(), libargs, cwd = cwd) | ||
|
||
def gen_sdk_source(self): | ||
print("Generating source!") | ||
cmd = self.get_python() + [os.path.normpath(join(self.defold_root, './scripts/dmsdk/gen_sdk.py'))] | ||
for lib in ENGINE_LIBS: | ||
cwd = 'engine/%s' % lib | ||
info = join(self.defold_root, 'engine/%s/sdk_gen.json' % lib) | ||
if os.path.exists(info): | ||
self._gen_sdk_source_lib(lib, cmd, join(self.defold_root, cwd), info) | ||
|
||
# <- Gen source files | ||
# ------------------------------------------------------------ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code generation bit from the build.py
@@ -0,0 +1,224 @@ | |||
# Copyright 2020-2024 The Defold Foundation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these files are heavily modified from the Sokol bindgen tools.
However, at this point I feel that I've modified it so far, that it has little resemblance of the original scripts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phew, a lot to chew, but I think it looks good
//////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||
// C api | ||
|
||
const char* ConfigFileGetString(HConfigFile config, const char* key, const char* default_value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually like this separation of the public API and our internal files within the .cpp modules!
@@ -15,7 +15,7 @@ | |||
#ifndef DM_CONFIGFILE_H | |||
#define DM_CONFIGFILE_H | |||
|
|||
#include <dmsdk/dlib/configfile.h> | |||
#include <dmsdk/dlib/configfile_gen.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we should now include the _gen.hpp files if we include dmsdk headers internally?
// internal | ||
#define DM_REGISTER_CONFIGFILE_EXTENSION(symbol, desc, desc_size, name, create, destroy, get_string, get_int, get_float) extern "C" void symbol () { \ | ||
ConfigFileRegisterExtension((void*) &desc, desc_size, name, create, destroy, get_string, get_int, get_float); \ | ||
} | ||
|
||
// internal | ||
#define DM_DMCF_PASTE_SYMREG(x, y) x ## y | ||
// internal | ||
#define DM_DMCF_PASTE_SYMREG2(x, y) DM_DMCF_PASTE_SYMREG(x, y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if these are internal, should they be here? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're needed for the DM_DECLARE_CONFIGFILE_EXTENSION
below.
And even if I'd put them in another header, that header would still need to be public.
Also, our rule is "if it is documented, it's public", e.g. just like our scripting documentation.
@@ -76,6 +76,7 @@ platforms: | |||
|
|||
common: | |||
env: | |||
ZIG_PATH: '{{env.ZIG_PATH_0_11}}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that we now support zig files as well? We should make a note of that in the PR notes 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I think there was an issue with the extender server (for some reason), so I didn't pursue it more.
Yeah, this PR is really old (remember the Zig presentation?), and it was intended to land in a first iteration a long time ago.
# Conflicts: # engine/extension/src/dmsdk/extension/extension.h # engine/gameobject/src/gameobject/test/collection/test_gameobject_collection.cpp
# Conflicts: # scripts/build.py
We've added some first pure C api headers to our
dmSDK
:configfile.h
,extension.h
andresource.h
.This is a step in our current effort to add C# support to our engine.
The old C++ api's should be mostly intact, however we did change passing some structs as pointers (previously passed as references).
Also, due to limitations of typedef'ing enums, we generate some C++ enums from the C equivalents. Casting between them is safe.
While stricly a breaking change for extension developers, we don't anticipate many developers having to do any changes to their extensions.
New extension releases:
We have updated some extensions that use the new api's:
https://github.com/defold/extension-spine/releases/tag/3.3.0
https://github.com/britzl/defold-screenshot/releases/tag/1.11.0
https://github.com/britzl/extension-imgui/releases/tag/2.2.0
https://github.com/britzl/defold-sharing/releases/tag/4.5.0
https://github.com/defold/extension-rive/releases/tag/2.3.0
https://github.com/defold/extension-simpledata/releases/tag/v1.0.0
Migration:
For a full list of migration notes, see the PR
dmsdk/dlib/configfile.h
This file now contains both the the C and C++ api.
dmsdk/extension/extension.h
This file now contains both the the C and C++ api.
dmsdk/resource/resource.h
This file now contains both the the C and C++ api.
Fixes #8924
Technical details
Migration
The steps to migrate from our previous version of our resource C++ sdk.
This is only needed if you use any of these api's.
dmsdk/extension/extension.h
The C++ structs for the dmExtension, are now typedefs referring to the C struct. And in this struct, the event id enum is now of type
ExtensionEventID
. So you may need to cast the type like so.It is safe, as the C++ enum is generated exaclty from the
ExtensionEventID
enum:Extension callbacks
Add a cast to
FExtensionCallback
when registering the function.dmsdk/resource/resource.h
Some previous pointer types have also been changed into opaque handle types:
HResourceType
HResourceDescriptor
HResourcePreloadHintInfo
HResourceTypeContext
Registering a type:
The register/deregister functions passed to
DM_DECLARE_RESOURCE_TYPE
now take two arguments:RegisterType
butSetupType
Replace:
MyTypeRegister(dmResource::ResourceTypeRegisterContext& ctx)
toMyTypeRegister((HResourceTypeContext ctx, HResourceType type)
dmResource::RegisterType()
withResourceResult dmResource::SetupType(ctx, type, context, preload, create, post_create, destroy, recreate)
dmResource::ResourceCreateParams&
withdmResource::ResourceCreateParams*
.
to->
Example from extension-spine:
Resource loading
The preloader hint is also an opaque handle, with only one function. Replace:
params->m_HintInfo = ...
withdmResource::PreloadHint(params->m_HintInfo, sound_desc->m_Sound)
Resource reloaded callbacks
Use the pointer instead of the reference. Replace
dmResource::ResourceReloadedParams&
withdmResource::ResourceReloadedParams*
Manipulating a resource descriptor
As an example, the getting/setting a resource pointer to a resource descriptor, replace:
params.m_Resource->m_Resource
withdmResource::GetResource(params.m_Resource);
params.m_Resource->m_Resource = p
withdmResource::SetResource(params.m_Resource, p);
The full list of descriptor functions:
PR checklist