-
Notifications
You must be signed in to change notification settings - Fork 294
Implement Structs in GDScript #1152
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
base: dev
Are you sure you want to change the base?
Changes from all commits
412d1f9
907dedb
5aa9cc4
2cd8d99
f027c2a
7f43c0f
126783a
6ea6d00
01a4c05
ef89c7f
ed04cb2
4c99518
d540e6d
259393e
072ced0
8c4de69
433113f
1f1bb67
58e756b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -390,3 +390,5 @@ $RECYCLE.BIN/ | |
| *.msp | ||
| *.lnk | ||
| *.generated.props | ||
| .gdbinit | ||
| CLAUDE.md | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,40 @@ | |
| #include "core/config/engine.h" | ||
| #include "core/object/script_language.h" | ||
| #include "core/variant/container_type_validate.h" | ||
| #include "core/variant/variant_internal.h" | ||
|
|
||
| // GDScript struct serialization support | ||
| // Guarded with MODULE_GDSCRIPT_ENABLED since core/io can't depend on modules/gdscript | ||
| #ifdef MODULE_GDSCRIPT_ENABLED | ||
| // 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 | ||
|
|
||
| extern "C" { | ||
| // These will be defined in gdscript.cpp with C linkage to avoid name mangling issues | ||
| void gdscript_struct_instance_serialize(const GDScriptStructInstance *p_instance, Dictionary &r_dict); | ||
| 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 | ||
| Dictionary dict; | ||
| gdscript_struct_instance_serialize(p_instance, dict); | ||
| return dict; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| #else | ||
| // Stub implementation when GDScript module is disabled | ||
| static Dictionary _serialize_struct(const void *p_instance) { | ||
| ERR_FAIL_V_MSG(Dictionary(), "STRUCT serialization requires MODULE_GDSCRIPT_ENABLED."); | ||
| } | ||
| #endif // MODULE_GDSCRIPT_ENABLED | ||
|
|
||
| const char *JSON::tk_name[TK_MAX] = { | ||
| "'{'", | ||
|
|
@@ -824,6 +858,50 @@ Variant JSON::_from_native(const Variant &p_variant, bool p_full_objects, int p_ | |
| return ret; | ||
| } break; | ||
|
|
||
| #ifdef MODULE_GDSCRIPT_ENABLED | ||
| 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; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| #else | ||
| case Variant::STRUCT: { | ||
| ERR_FAIL_V_MSG(Variant(), "STRUCT serialization requires MODULE_GDSCRIPT_ENABLED."); | ||
| } break; | ||
| #endif // MODULE_GDSCRIPT_ENABLED | ||
|
|
||
| case Variant::DICTIONARY: { | ||
| const Dictionary dict = p_variant; | ||
|
|
||
|
|
@@ -1297,6 +1375,44 @@ 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; | ||
|
|
||
| #ifdef MODULE_GDSCRIPT_ENABLED | ||
| 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; | ||
| #else | ||
| case Variant::STRUCT: { | ||
| ERR_FAIL_V_MSG(Variant(), "STRUCT deserialization requires MODULE_GDSCRIPT_ENABLED."); | ||
| } break; | ||
| #endif // MODULE_GDSCRIPT_ENABLED | ||
|
|
||
|
Comment on lines
+1379
to
+1415
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Harden decoding: validate Right now 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 |
||
| case Variant::DICTIONARY: { | ||
| LOAD_ARGS_CHECK_FACTOR(2); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,18 @@ | |
| #include <climits> | ||
| #include <cstdio> | ||
|
|
||
| // GDScript struct serialization support | ||
| // Guarded with MODULE_GDSCRIPT_ENABLED since core/io can't depend on modules/gdscript | ||
| #ifdef MODULE_GDSCRIPT_ENABLED | ||
|
|
||
| extern "C" { | ||
| // These functions are defined in modules/gdscript/gdscript.cpp with C linkage | ||
| Error gdscript_variant_encode_struct(const Variant &p_variant, uint8_t *r_buffer, int &r_len); | ||
| Error gdscript_variant_decode_struct(const uint8_t *p_buffer, int p_len, int *r_len, Variant &r_variant); | ||
| } | ||
|
|
||
| #endif // MODULE_GDSCRIPT_ENABLED | ||
|
|
||
| void EncodedObjectAsID::_bind_methods() { | ||
| ClassDB::bind_method(D_METHOD("set_object_id", "id"), &EncodedObjectAsID::set_object_id); | ||
| ClassDB::bind_method(D_METHOD("get_object_id"), &EncodedObjectAsID::get_object_id); | ||
|
|
@@ -1296,6 +1308,17 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int | |
| r_variant = varray; | ||
|
|
||
| } break; | ||
| #ifdef MODULE_GDSCRIPT_ENABLED | ||
| case Variant::STRUCT: { | ||
| // Decode struct via GDScript helper function | ||
| Error err = gdscript_variant_decode_struct(buf, len, r_len, r_variant); | ||
| ERR_FAIL_COND_V(err, err); | ||
| } break; | ||
| #else | ||
| case Variant::STRUCT: { | ||
| ERR_FAIL_V(ERR_BUG); | ||
| } break; | ||
| #endif | ||
|
Comment on lines
+1311
to
+1321
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: STRUCT encode/decode will return wrong Proposed fix (use a temporary payload size and add it) #ifdef MODULE_GDSCRIPT_ENABLED
case Variant::STRUCT: {
- // Decode struct via GDScript helper function
- Error err = gdscript_variant_decode_struct(buf, len, r_len, r_variant);
+ int payload_used = 0;
+ Error err = gdscript_variant_decode_struct(buf, len, &payload_used, r_variant);
ERR_FAIL_COND_V(err, err);
+ if (r_len) {
+ (*r_len) += payload_used;
+ }
} break;
#else
case Variant::STRUCT: {
ERR_FAIL_V(ERR_BUG);
} break;
#endif #ifdef MODULE_GDSCRIPT_ENABLED
case Variant::STRUCT: {
- // Encode struct via GDScript helper function
- Error err = gdscript_variant_encode_struct(p_variant, buf, r_len);
+ int payload_len = 0;
+ Error err = gdscript_variant_encode_struct(p_variant, buf, payload_len);
ERR_FAIL_COND_V(err, err);
+ r_len += payload_len;
} break;
#else
case Variant::STRUCT: {
ERR_FAIL_V(ERR_BUG);
} break;
#endifAlso applies to: 2150-2160 🤖 Prompt for AI Agents |
||
| default: { | ||
| ERR_FAIL_V(ERR_BUG); | ||
| } | ||
|
|
@@ -2124,6 +2147,17 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo | |
| r_len += sizeof(real_t) * 4 * len; | ||
|
|
||
| } break; | ||
| #ifdef MODULE_GDSCRIPT_ENABLED | ||
| case Variant::STRUCT: { | ||
| // Encode struct via GDScript helper function | ||
| Error err = gdscript_variant_encode_struct(p_variant, buf, r_len); | ||
| ERR_FAIL_COND_V(err, err); | ||
| } break; | ||
| #else | ||
| case Variant::STRUCT: { | ||
| ERR_FAIL_V(ERR_BUG); | ||
| } break; | ||
| #endif | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| default: { | ||
| ERR_FAIL_V(ERR_BUG); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 on lines
+553
to
+593
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid calling Proposed fix (snapshot languages under lock, then call outside) 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;
- }
- }
- }
+ Vector<ScriptLanguage *> langs;
+ {
+ MutexLock lock(languages_mutex);
+ if (!languages_ready) {
+ ERR_FAIL_V_MSG(Variant(), "Cannot create struct instance: languages not initialized.");
+ }
+ langs.resize(_language_count);
+ for (int i = 0; i < _language_count; i++) {
+ langs.write[i] = _languages[i];
+ }
+ }
+
+ for (int i = 0; i < langs.size(); i++) {
+ ScriptLanguage *lang = langs[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;
- }
- }
- }
+ Vector<ScriptLanguage *> langs;
+ {
+ MutexLock lock(languages_mutex);
+ if (!languages_ready) {
+ return false;
+ }
+ langs.resize(_language_count);
+ for (int i = 0; i < _language_count; i++) {
+ langs.write[i] = _languages[i];
+ }
+ }
+
+ for (int i = 0; i < langs.size(); i++) {
+ ScriptLanguage *lang = langs[i];
+ if (lang && lang->can_create_struct_by_name()) {
+ Variant result = lang->create_struct_by_name(p_fully_qualified_name, Dictionary());
+ if (result.get_type() != Variant::NIL) {
+ return true;
+ }
+ }
+ }
return false;
}🤖 Prompt for AI Agents |
||
|
|
||
| Vector<Ref<ScriptBacktrace>> ScriptServer::capture_script_backtraces(bool p_include_variables) { | ||
| if (is_program_exiting) { | ||
| return Vector<Ref<ScriptBacktrace>>(); | ||
|
|
||
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.
Not sure about this methodology