Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
412d1f9
Baseline framework for GDScript Structs
decryptedchaos Jan 9, 2026
907dedb
Memory cleanup
decryptedchaos Jan 9, 2026
5aa9cc4
Fix race condition and issues found by Coderabbit
decryptedchaos Jan 9, 2026
2cd8d99
More fixes
decryptedchaos Jan 9, 2026
f027c2a
(De)serialization implementation
decryptedchaos Jan 9, 2026
7f43c0f
Ref counting & more Coderabbit suggestions
decryptedchaos Jan 9, 2026
126783a
Still chasing memeory holes
decryptedchaos Jan 9, 2026
6ea6d00
Fix infinite loop
decryptedchaos Jan 10, 2026
01a4c05
Coderabbit: Potential memory leak: unreference() return value ignored.
decryptedchaos Jan 10, 2026
ef89c7f
The constructor is functional
decryptedchaos Jan 10, 2026
ed04cb2
I feel like i'm going in circles here
decryptedchaos Jan 11, 2026
4c99518
Fix bugs
decryptedchaos Jan 11, 2026
d540e6d
Structs are functional but have architectural flaw
decryptedchaos Jan 11, 2026
259393e
Refactor: COW Wrapper
decryptedchaos Jan 13, 2026
072ced0
Debugging & Syntax Highlighting
decryptedchaos Jan 13, 2026
8c4de69
Attempt fixing CI build
decryptedchaos Jan 13, 2026
433113f
Add missing STRUCT cases to test_class_db.h
decryptedchaos Jan 13, 2026
1f1bb67
Fix C-linkage incompatibility for struct JSON serialization
decryptedchaos Jan 13, 2026
58e756b
Remove redundant STRUCT condition in method call handling
decryptedchaos Jan 13, 2026
6b2239e
Fix double-free struct method.
mcdubhghlas Jul 17, 2026
ead222f
Fix crash on struct instance call.
mcdubhghlas Jul 17, 2026
97bdc7b
Fix struct marshalling corrupting nested containers.
mcdubhghlas Jul 17, 2026
f816969
reject self inside struct nested in a class.
mcdubhghlas Jul 17, 2026
88a80e9
Resolve struct body before field-type lookup
mcdubhghlas Jul 17, 2026
fd1ad97
Register struct wrapper global on hot-reload.
mcdubhghlas Jul 17, 2026
ae725fb
Ensure unique copy in struct deserialize
mcdubhghlas Jul 17, 2026
b89e06b
Do not call ScriptLanguage struct virtuals under languages_mutex
mcdubhghlas Jul 17, 2026
65568c3
Fix struct keyed ptr_get/ptr_has base-pointer convention
mcdubhghlas Jul 17, 2026
bfb7dc8
Clean up struct sig lookup.
mcdubhghlas Jul 17, 2026
0fe2915
Initialize r_error
mcdubhghlas Jul 17, 2026
61d1aa6
Preserve struct type through text serialization + keeps it strict. No…
mcdubhghlas Jul 17, 2026
9f3a822
struct types should be gdscript-only, not C# bindings.
mcdubhghlas Jul 17, 2026
9cf04bf
Move struct to the end to preserve type IDs.
mcdubhghlas Jul 17, 2026
991fdad
removed unused code to stop the complaining.
mcdubhghlas Jul 17, 2026
22bc867
skip struct in operator NOT all-types loop
mcdubhghlas Jul 17, 2026
8b9aa20
Merge branch 'master' of https://github.com/Redot-Engine/redot-engine…
mcdubhghlas Jul 17, 2026
3dfe741
Merge branch 'master' of https://github.com/Redot-Engine/redot-engine…
mcdubhghlas Jul 17, 2026
af0b101
CI complaints quelled.
mcdubhghlas Jul 17, 2026
358ef84
cleanup
mcdubhghlas Jul 17, 2026
3c0446f
struct warnings and what not.
mcdubhghlas Jul 17, 2026
9a2e1bf
An evil requirement thwarts our hero one last time.
mcdubhghlas Jul 17, 2026
2190c83
documentation is important.
mcdubhghlas Jul 17, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core/extension/extension_api_dump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
#include "core/templates/pair.h"
#include "core/version.h"

// Forward declaration for struct type
class GDScriptStructInstance;
Comment thread
mcdubhghlas marked this conversation as resolved.
Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this methodology

#ifdef TOOLS_ENABLED
#include "editor/doc/editor_help.h"

Expand Down Expand Up @@ -210,6 +213,7 @@ Dictionary GDExtensionAPIDump::generate_extension_api(bool p_include_docs) {
{ Variant::NODE_PATH, ptrsize_32, ptrsize_64, ptrsize_32, ptrsize_64 },
{ Variant::RID, sizeof(uint64_t), sizeof(uint64_t), sizeof(uint64_t), sizeof(uint64_t) },
{ Variant::OBJECT, ptrsize_32, ptrsize_64, ptrsize_32, ptrsize_64 },
{ Variant::STRUCT, ptrsize_32, ptrsize_64, ptrsize_32, ptrsize_64 },
{ Variant::CALLABLE, sizeof(Callable), sizeof(Callable), sizeof(Callable), sizeof(Callable) }, // Hardcoded align.
{ Variant::SIGNAL, sizeof(Signal), sizeof(Signal), sizeof(Signal), sizeof(Signal) }, // Hardcoded align.
{ Variant::DICTIONARY, ptrsize_32, ptrsize_64, ptrsize_32, ptrsize_64 },
Expand Down Expand Up @@ -252,6 +256,7 @@ Dictionary GDExtensionAPIDump::generate_extension_api(bool p_include_docs) {
static_assert(type_size_array[Variant::NODE_PATH][sizeof(void *)] == sizeof(NodePath), "Size of NodePath mismatch");
static_assert(type_size_array[Variant::RID][sizeof(void *)] == sizeof(RID), "Size of RID mismatch");
static_assert(type_size_array[Variant::OBJECT][sizeof(void *)] == sizeof(Object *), "Size of Object mismatch");
static_assert(type_size_array[Variant::STRUCT][sizeof(void *)] == sizeof(GDScriptStructInstance *), "Size of Struct mismatch");
static_assert(type_size_array[Variant::CALLABLE][sizeof(void *)] == sizeof(Callable), "Size of Callable mismatch");
static_assert(type_size_array[Variant::SIGNAL][sizeof(void *)] == sizeof(Signal), "Size of Signal mismatch");
static_assert(type_size_array[Variant::DICTIONARY][sizeof(void *)] == sizeof(Dictionary), "Size of Dictionary mismatch");
Expand Down
10 changes: 10 additions & 0 deletions core/extension/gdextension_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,11 @@ static GDExtensionPtrOperatorEvaluator gdextension_variant_get_ptr_operator_eval
return (GDExtensionPtrOperatorEvaluator)Variant::get_ptr_operator_evaluator(Variant::Operator(p_operator), Variant::Type(p_type_a), Variant::Type(p_type_b));
}
static GDExtensionPtrBuiltInMethod gdextension_variant_get_ptr_builtin_method(GDExtensionVariantType p_type, GDExtensionConstStringNamePtr p_method, GDExtensionInt p_hash) {
// STRUCT type doesn't support built-in methods
if (p_type == (int)Variant::STRUCT) {
return nullptr;
}

const StringName method = *reinterpret_cast<const StringName *>(p_method);
uint32_t hash = Variant::get_builtin_method_hash(Variant::Type(p_type), method);
if (hash != p_hash) {
Expand All @@ -827,6 +832,11 @@ static GDExtensionPtrBuiltInMethod gdextension_variant_get_ptr_builtin_method(GD
return (GDExtensionPtrBuiltInMethod)Variant::get_ptr_builtin_method(Variant::Type(p_type), method);
}
static GDExtensionPtrConstructor gdextension_variant_get_ptr_constructor(GDExtensionVariantType p_type, int32_t p_constructor) {
// STRUCT type doesn't support constructors (yet)
if (p_type == (int)Variant::STRUCT) {
return nullptr;
}

return (GDExtensionPtrConstructor)Variant::get_ptr_constructor(Variant::Type(p_type), p_constructor);
}
static GDExtensionPtrDestructor gdextension_variant_get_ptr_destructor(GDExtensionVariantType p_type) {
Expand Down
93 changes: 93 additions & 0 deletions core/io/json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,29 @@
#include "core/config/engine.h"
#include "core/object/script_language.h"
#include "core/variant/container_type_validate.h"
#include "core/variant/variant_internal.h"

// Forward declaration for GDScript struct serialization
class GDScriptStructInstance;
class GDScriptStruct;

// We need to declare these methods without including the GDScript headers
// This is a bit of a hack, but necessary due to layering (core/io can't include modules/gdscript)
// In the future, this should be replaced with a proper Variant-level API
Comment thread
mcdubhghlas marked this conversation as resolved.
Outdated

extern "C" {
// These will be defined in gdscript.cpp with C linkage to avoid name mangling issues
Dictionary gdscript_struct_instance_serialize(const GDScriptStructInstance *p_instance);
bool gdscript_struct_instance_get_type_name(const GDScriptStructInstance *p_instance, String &r_name);
}

// Helper function to serialize a struct
static Dictionary _serialize_struct(const GDScriptStructInstance *p_instance) {
ERR_FAIL_NULL_V(p_instance, Dictionary());

// Call the GDScript helper function
return gdscript_struct_instance_serialize(p_instance);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const char *JSON::tk_name[TK_MAX] = {
"'{'",
Expand Down Expand Up @@ -824,6 +847,44 @@ Variant JSON::_from_native(const Variant &p_variant, bool p_full_objects, int p_
return ret;
} break;

case Variant::STRUCT: {
// Serialize struct as tagged dictionary with __type__ metadata
// This allows round-trip deserialization
const GDScriptStructInstance *struct_instance = reinterpret_cast<const GDScriptStructInstance *>(VariantInternal::get_struct(&p_variant));
ERR_FAIL_NULL_V(struct_instance, Variant());

Dictionary ret;
ret[TYPE] = Variant::get_type_name(p_variant.get_type());

// Get the serialized data from the struct instance using helper
Dictionary struct_data = _serialize_struct(struct_instance);

// Ensure __type__ field exists (it should from serialize())
if (!struct_data.has("__type__")) {
ERR_FAIL_V_MSG(Variant(), "Struct serialization failed: missing __type__ field.");
}

// Encode field values using _from_native to ensure proper JSON encoding
// This handles non-JSON-native types (nested structs, objects, typed arrays, etc.)
Dictionary encoded_struct_data;
encoded_struct_data["__type__"] = struct_data["__type__"]; // Copy type identifier as-is

for (const KeyValue<Variant, Variant> &kv : struct_data) {
if (kv.key == "__type__") {
continue; // Already copied above
}
// Encode the value using _from_native, keys are strings (field names) and don't need encoding
encoded_struct_data[kv.key] = _from_native(kv.value, p_full_objects, p_depth + 1);
}

// Wrap the encoded serialized data in the expected format
Array args;
args.push_back(encoded_struct_data);
ret[ARGS] = args;

return ret;
} break;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

case Variant::DICTIONARY: {
const Dictionary dict = p_variant;

Expand Down Expand Up @@ -1297,6 +1358,38 @@ Variant JSON::_to_native(const Variant &p_json, bool p_allow_objects, int p_dept
// Nothing to do at this stage. `Object` should be treated as a class, not as a built-in type.
} break;

case Variant::STRUCT: {
// Deserialize struct from tagged dictionary
LOAD_ARGS();

ERR_FAIL_COND_V_MSG(args.size() != 1, Variant(), "Invalid struct data: expected single dictionary argument.");

Dictionary encoded_data = args[0];
ERR_FAIL_COND_V_MSG(!encoded_data.has("__type__"), Variant(), "Invalid struct data: missing __type__ field.");

String type_name = encoded_data["__type__"];
ERR_FAIL_COND_V_MSG(type_name.is_empty(), Variant(), "Invalid struct data: empty __type__ field.");

// Decode field values using _to_native to properly reconstruct non-JSON-native types
// This handles nested structs, objects, typed arrays, etc.
Dictionary decoded_struct_data;
decoded_struct_data["__type__"] = encoded_data["__type__"]; // Copy type identifier as-is

for (const KeyValue<Variant, Variant> &kv : encoded_data) {
if (kv.key == "__type__") {
continue; // Already copied above
}
// Decode the value using _to_native, keys are strings (field names) and don't need decoding
decoded_struct_data[kv.key] = _to_native(kv.value, p_allow_objects, p_depth + 1);
}

// Use ScriptServer to create the struct instance with decoded data
Variant struct_instance = ScriptServer::create_struct_instance(type_name, decoded_struct_data);
ERR_FAIL_COND_V_MSG(struct_instance.get_type() != Variant::STRUCT, Variant(), vformat("Failed to create struct instance for type '%s'.", type_name));

return struct_instance;
} break;

Comment on lines +1380 to +1416

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Harden decoding: validate args[0] is a Dictionary before using it.

Right now Dictionary encoded_data = args[0]; will rely on Variant conversion rules and can quietly yield an empty dictionary for malformed inputs. This makes error reporting less precise.

Targeted hardening
 					LOAD_ARGS();

 					ERR_FAIL_COND_V_MSG(args.size() != 1, Variant(), "Invalid struct data: expected single dictionary argument.");

-					Dictionary encoded_data = args[0];
+					ERR_FAIL_COND_V_MSG(((Variant)args[0]).get_type() != Variant::DICTIONARY, Variant(), "Invalid struct data: expected Dictionary argument.");
+					Dictionary encoded_data = args[0];
 					ERR_FAIL_COND_V_MSG(!encoded_data.has("__type__"), Variant(), "Invalid struct data: missing __type__ field.");
🤖 Prompt for AI Agents
In @core/io/json.cpp around lines 1361 - 1392, The code assumes args[0] is a
Dictionary when handling Variant::STRUCT; add an explicit type check before
assigning args[0] to Dictionary encoded_data (e.g.
ERR_FAIL_COND_V_MSG(args[0].get_type() != Variant::DICTIONARY, Variant(),
"Invalid struct data: expected dictionary argument.");) then only create
Dictionary encoded_data = args[0] and continue with the existing checks/decoding
that use _to_native and ScriptServer::create_struct_instance; reference symbols:
Variant::STRUCT, LOAD_ARGS(), args, encoded_data, _to_native,
ScriptServer::create_struct_instance.

case Variant::DICTIONARY: {
LOAD_ARGS_CHECK_FACTOR(2);

Expand Down
42 changes: 42 additions & 0 deletions core/object/script_language.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,48 @@ void ScriptServer::save_global_classes() {
ProjectSettings::get_singleton()->store_global_class_list(gcarr);
}

/***************** STRUCT SERIALIZATION *****************/

Variant ScriptServer::create_struct_instance(const String &p_fully_qualified_name, const Dictionary &p_data) {
MutexLock lock(languages_mutex);
if (!languages_ready) {
ERR_FAIL_V_MSG(Variant(), "Cannot create struct instance: languages not initialized.");
}

// Try each language to see if it can create the struct
for (int i = 0; i < _language_count; i++) {
ScriptLanguage *lang = _languages[i];
if (lang && lang->can_create_struct_by_name()) {
Variant result = lang->create_struct_by_name(p_fully_qualified_name, p_data);
if (result.get_type() != Variant::NIL) {
return result;
}
}
}

ERR_FAIL_V_MSG(Variant(), vformat("Cannot create struct instance: no language supports struct '%s'.", p_fully_qualified_name));
}

bool ScriptServer::global_struct_exists(const String &p_fully_qualified_name) {
MutexLock lock(languages_mutex);
if (!languages_ready) {
return false;
}

// Check if any language can create this struct
for (int i = 0; i < _language_count; i++) {
ScriptLanguage *lang = _languages[i];
if (lang && lang->can_create_struct_by_name()) {
// Try to create with empty data to check if it exists
Variant result = lang->create_struct_by_name(p_fully_qualified_name, Dictionary());
if (result.get_type() != Variant::NIL) {
return true;
}
}
}
return false;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Vector<Ref<ScriptBacktrace>> ScriptServer::capture_script_backtraces(bool p_include_variables) {
if (is_program_exiting) {
return Vector<Ref<ScriptBacktrace>>();
Expand Down
9 changes: 9 additions & 0 deletions core/object/script_language.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ class ScriptServer {
static void get_inheriters_list(const StringName &p_base_type, List<StringName> *r_classes);
static void save_global_classes();

// Struct serialization utilities (for GDScript)
static Variant create_struct_instance(const String &p_fully_qualified_name, const Dictionary &p_data);
static bool global_struct_exists(const String &p_fully_qualified_name);

static Vector<Ref<ScriptBacktrace>> capture_script_backtraces(bool p_include_variables = false);

static void init_languages();
Expand Down Expand Up @@ -446,6 +450,11 @@ class ScriptLanguage : public Object {
virtual bool handles_global_class_type(const String &p_type) const { return false; }
virtual String get_global_class_name(const String &p_path, String *r_base_type = nullptr, String *r_icon_path = nullptr, bool *r_is_abstract = nullptr, bool *r_is_tool = nullptr) const { return String(); }

/* STRUCT SERIALIZATION */
// These methods allow serialization systems to create struct instances without knowing about GDScript
virtual bool can_create_struct_by_name() const { return false; }
virtual Variant create_struct_by_name(const String &p_fully_qualified_name, const Dictionary &p_data) { return Variant(); }

virtual ~ScriptLanguage() {}
};

Expand Down
27 changes: 26 additions & 1 deletion core/variant/variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "core/io/resource.h"
#include "core/math/math_funcs.h"
#include "core/variant/variant_parser.h"
#include "modules/gdscript/gdscript_struct.h"

Comment thread
coderabbitai[bot] marked this conversation as resolved.
PagedAllocator<Variant::Pools::BucketSmall, true> Variant::Pools::_bucket_small;
PagedAllocator<Variant::Pools::BucketMedium, true> Variant::Pools::_bucket_medium;
Expand Down Expand Up @@ -119,6 +120,9 @@ String Variant::get_type_name(Variant::Type p_type) {
case OBJECT: {
return "Object";
}
case STRUCT: {
return "Struct";
}
case CALLABLE: {
return "Callable";
}
Expand Down Expand Up @@ -1311,6 +1315,15 @@ void Variant::reference(const Variant &p_variant) {
_data.packed_array = PackedArrayRef<Vector4>::create();
}
} break;
case STRUCT: {
// Reference the struct instance
GDScriptStructInstance *struct_instance = const_cast<GDScriptStructInstance *>(reinterpret_cast<const GDScriptStructInstance *>(p_variant._data._mem));
if (struct_instance) {
struct_instance->reference();
}
// Copy the pointer - only copy pointer size, not entire _mem
memcpy(_data._mem, p_variant._data._mem, sizeof(void *));
} break;
default: {
}
}
Expand Down Expand Up @@ -1480,6 +1493,18 @@ void Variant::_clear_internal() {
case PACKED_VECTOR4_ARRAY: {
PackedArrayRefBase::destroy(_data.packed_array);
} break;
case STRUCT: {
// Unreference the struct instance
GDScriptStructInstance *struct_instance = reinterpret_cast<GDScriptStructInstance *>(_data._mem);
if (struct_instance) {
struct_instance->unreference();
if (struct_instance->get_reference_count() == 0) {
memdelete(struct_instance);
}
}
// Clear the pointer
memset(_data._mem, 0, sizeof(_data._mem));
} break;
default: {
// Not needed, there is no point. The following do not allocate memory:
// VECTOR2, VECTOR3, VECTOR4, RECT2, PLANE, QUATERNION, COLOR.
Expand Down Expand Up @@ -3490,7 +3515,7 @@ void Variant::construct_from_string(const String &p_string, Variant &r_value, Ob

String Variant::get_construct_string() const {
String vars;
VariantWriter::write_to_string(*this, vars, nullptr, nullptr, true, true);
VariantWriter::write_to_string(*this, vars);

return vars;
}
Expand Down
4 changes: 4 additions & 0 deletions core/variant/variant.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@

class Object;
class RefCounted;
class GDScriptStructInstance;

template <typename T>
class Ref;
Expand Down Expand Up @@ -123,6 +124,7 @@
NODE_PATH,
RID,
OBJECT,
STRUCT,
CALLABLE,
SIGNAL,
DICTIONARY,
Expand Down Expand Up @@ -175,6 +177,7 @@

friend struct _VariantCall;
friend class VariantInternal;
friend class GDScriptStructClass; // Needed for proper struct construction
// Variant takes 24 bytes when real_t is float, and 40 bytes if double.
// It only allocates extra memory for AABB/Transform2D (24, 48 if double),
// Basis/Transform3D (48, 96 if double), Projection (64, 128 if double),
Expand Down Expand Up @@ -310,6 +313,7 @@
true, //NODE_PATH,
false, //RID,
true, //OBJECT,
true, //STRUCT,
true, //CALLABLE,
true, //SIGNAL,
true, //DICTIONARY,
Expand Down Expand Up @@ -929,7 +933,7 @@

template <typename... VarArgs>
String vformat(const String &p_text, const VarArgs... p_args) {
Variant args[sizeof...(p_args) + 1] = { p_args..., Variant() }; // +1 makes sure zero sized arrays are also supported.

Check failure on line 936 in core/variant/variant.h

View workflow job for this annotation

GitHub Actions / 🍏 iOS / Template (target=template_release)

conversion from 'const unsigned long' to 'Variant' is ambiguous

Check failure on line 936 in core/variant/variant.h

View workflow job for this annotation

GitHub Actions / 🍏 iOS / Template (target=template_release)

conversion from 'const unsigned long' to 'Variant' is ambiguous

Check failure on line 936 in core/variant/variant.h

View workflow job for this annotation

GitHub Actions / 🌐 Web / Template w/ threads (target=template_release, threads=yes)

conversion from 'const unsigned long' to 'Variant' is ambiguous

Check failure on line 936 in core/variant/variant.h

View workflow job for this annotation

GitHub Actions / 🌐 Web / Template w/ threads (target=template_release, threads=yes)

conversion from 'const unsigned long' to 'Variant' is ambiguous

Check failure on line 936 in core/variant/variant.h

View workflow job for this annotation

GitHub Actions / 🌐 Web / Template w/o threads (target=template_release, threads=no)

conversion from 'const unsigned long' to 'Variant' is ambiguous

Check failure on line 936 in core/variant/variant.h

View workflow job for this annotation

GitHub Actions / 🌐 Web / Template w/o threads (target=template_release, threads=no)

conversion from 'const unsigned long' to 'Variant' is ambiguous
Array args_array;
args_array.resize(sizeof...(p_args));
for (uint32_t i = 0; i < sizeof...(p_args); i++) {
Expand Down
Loading
Loading