From c2b408842ea036395cec5bd32bd435c6202e743c Mon Sep 17 00:00:00 2001 From: GsLogimaker Date: Wed, 27 Nov 2024 21:47:53 -0600 Subject: [PATCH 1/7] Rework adding and setting components Setting components will now add them. Setting a component member with a wrong value will raise an error instead of crashing now. Removed `add_entity`, it was redundent and maybe confusing. Updated tests. --- .vscode/launch.json | 6 +- cpp/src/component.cpp | 33 +++++ cpp/src/component.h | 1 + cpp/src/entity.cpp | 182 +++++++++++++++++--------- cpp/src/entity.h | 12 +- cpp/src/utils.cpp | 72 ++++++++++ cpp/src/utils.h | 1 + cpp/src/world.cpp | 6 + examples/asteroids/asteroids.gd | 16 +-- gd/modules.gd | 5 +- unittests/test_components.gd | 4 +- unittests/test_events.gd | 12 +- unittests/test_module_rendering_2d.gd | 2 +- unittests/test_prefab.gd | 8 +- unittests/test_simple_systems.gd | 2 +- 15 files changed, 267 insertions(+), 95 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 3746b7f..dd969b9 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -9,10 +9,10 @@ "program": "${config:godotTools.editorPath.godot4}", "args": [ "--path", - "../../${workspaceFolder}", - "--headless" + "./", + "--unittest=all" ], - "cwd": "../../${workspaceFolder}" + "cwd": "${workspaceFolder}/../../" } ] } diff --git a/cpp/src/component.cpp b/cpp/src/component.cpp index 12baf44..14d8d86 100644 --- a/cpp/src/component.cpp +++ b/cpp/src/component.cpp @@ -306,6 +306,39 @@ void GFComponent::build_data_from_variant( } } +void GFComponent::build_data_from_members( + Array members, + void* output, + ecs_entity_t component_id, + GFWorld* world +) { + ecs_world_t* w_raw = world->raw(); + + const EcsStruct* struct_data = ecs_get(w_raw, component_id, EcsStruct); + if (struct_data == nullptr) { + ERR(/**/, + "Could not build data from Variant\n", + "Component is not a struct." + ); + } + + for (int i=0; i != members.size() && i != ecs_vec_size(&struct_data->members); i++) { + // Iterate the combined sizes of the passed array and the members vector + Variant value = members[i]; + + ecs_member_t* member_data = /* Get member metadata */ ecs_vec_get_t( + &struct_data->members, + ecs_member_t, + i + ); + void* member_ptr = /* Get member pointer */ static_cast( + static_cast(output) + member_data->offset + ); + // Set member value + Utils::set_type_from_variant(value, member_data->type, w_raw, member_ptr); + } +} + void GFComponent::set_source_id(ecs_entity_t id) { source_entity_id = id; } diff --git a/cpp/src/component.h b/cpp/src/component.h index 893f60a..909d2c4 100644 --- a/cpp/src/component.h +++ b/cpp/src/component.h @@ -50,6 +50,7 @@ namespace godot { void _register_internal(); void build_data_from_variant(Variant, void* output); + static void build_data_from_members(Array, void*, ecs_entity_t, GFWorld*); void set_source_id(ecs_entity_t id); static Ref new_internal(); diff --git a/cpp/src/entity.cpp b/cpp/src/entity.cpp index e38007e..7f1f936 100644 --- a/cpp/src/entity.cpp +++ b/cpp/src/entity.cpp @@ -1,6 +1,7 @@ #include "entity.h" #include "godot_cpp/classes/script.hpp" +#include "godot_cpp/variant/array.hpp" #include "utils.h" // needed here because entity.h does not include // component.h, but uses forward declaration instead @@ -31,54 +32,63 @@ Ref GFEntity::from_id(ecs_entity_t id, GFWorld* world) { return from_id_template(id, world); } -Ref GFEntity::add_component(Variant component, Variant data) { - GFWorld* w = get_world(); - - ecs_entity_t c_id = w->coerce_id(component); - - if (ECS_IS_PAIR(c_id)) { - add_pair( - ECS_PAIR_FIRST(c_id), - ECS_PAIR_SECOND(c_id), - data - ); - return Ref(this); +Ref GFEntity::add_component( + const Variant** args, GDExtensionInt arg_count, GDExtensionCallError &error +) { + if (arg_count < 1) { + // Too few arguments, return with error. + error.error = GDExtensionCallErrorType::GDEXTENSION_CALL_ERROR_TOO_FEW_ARGUMENTS; + error.argument = arg_count; + error.expected = 1; + return this; } - if (!ecs_has_id(w->raw(), c_id, ecs_id(EcsComponent))) { - ERR(Ref(this), - "Failed to add component to entity\n", - "ID coerced from ", component, " is not a component" - ); + // Parse arguments + Array members = Array(); + members.resize(arg_count); + for (int i=0; i != arg_count; i++) { + members[i] = *args[i]; } + Variant comopnent = members.pop_front(); - ecs_add_id(w->raw(), get_id(), c_id); - - if (data != Variant()) { - set_component(c_id, data); - } + _add_component(comopnent, members); - return Ref(this); + return this; } -Ref GFEntity::add_entity(Variant entity, Variant data) { +Ref GFEntity::_add_component(Variant component, Array members) { GFWorld* w = get_world(); - ecs_add_id(w->raw(), get_id(), w->coerce_id(entity)); - if (data != Variant()) { - set_component(entity, data); + ecs_entity_t c_id = w->coerce_id(component); + + if (ecs_has_id(w->raw(), get_id(), c_id)) { + ERR(this, + "Can't add component to entity\n", + "ID coerced from ", component, " is already added to ", get_id() + ) } - return Ref(this); + _set_component(c_id, members); + + return this; } -Ref GFEntity::add_pair(Variant first, Variant second, Variant data) { +Ref GFEntity::add_pair(Variant first, Variant second, Array members) { GFWorld* w = get_world(); ecs_entity_t first_id = w->coerce_id(first); ecs_entity_t second_id = w->coerce_id(second); ecs_entity_t pair_id = w->pair_ids(first_id, second_id); - add_entity(pair_id, data); + if ( + members.size() != 0 + || ecs_has_id(w->raw(), pair_id, FLECS_IDEcsComponentID_) + ) { + // Add pair as a component + _add_component(pair_id, members); + } else { + // Add pair as a dataless tag + add_tag(pair_id); + } return Ref(this); } @@ -87,11 +97,12 @@ Ref GFEntity::add_tag(Variant tag) { GFWorld* w = get_world(); ecs_entity_t tag_id = w->coerce_id(tag); + if (ecs_has(w->raw(), tag_id, EcsComponent)) { ERR(Ref(this), "Failed to add tag to entity\n", - "ID, ", tag_id, "is a component, not a tag\n" - "(Tags are any non-component entity)" + " ID, ", tag_id, " is a component, not a tag\n" + " (Tags are any entity with no data)" ); } @@ -121,41 +132,74 @@ Ref GFEntity::get_component(Variant component) { return c; } -Ref GFEntity::set_component(Variant component, Variant data) { - GFWorld* world = get_world(); +Ref GFEntity::set_component( + const Variant** args, GDExtensionInt arg_count, GDExtensionCallError &error +) { + if (arg_count < 1) { + // Too few arguments, return with error. + error.error = GDExtensionCallErrorType::GDEXTENSION_CALL_ERROR_TOO_FEW_ARGUMENTS; + error.argument = arg_count; + error.expected = 1; + return this; + } + + // Parse arguments + Array members = Array(); + members.resize(arg_count); + for (int i=0; i != arg_count; i++) { + members[i] = *args[i]; + } + Variant comopnent = members.pop_front(); + + return _set_component(comopnent, members); +} + +Ref GFEntity::_set_component( + Variant component, + Array members +) { + GFWorld* w = get_world(); - ecs_entity_t id = world->coerce_id(component); + ecs_entity_t c_id = w->coerce_id(component); - if (!ECS_IS_PAIR(id)) { - if (!ecs_has(world->raw(), id, EcsComponent)) { + if (!ecs_has_id(w->raw(), get_id(), c_id)) { + // Component is not present in entity, add it + + if (ECS_IS_PAIR(c_id)) { + // ID is a pair, perform pair specific checks + ecs_entity_t first_id = ECS_PAIR_FIRST(c_id); + ecs_entity_t second_id = ECS_PAIR_SECOND(c_id); + if ( + !ecs_has_id(w->raw(), first_id, ecs_id(EcsComponent)) + && !ecs_has_id(w->raw(), second_id, ecs_id(EcsComponent)) + ) { + // ID is not a component, error + ERR(Ref(this), + "Failed to set data in pair\n", + "Neither ID ", first_id, + " nor ", second_id, "are components" + ); + } + + } else if (!ecs_has_id(w->raw(), c_id, ecs_id(EcsComponent))) { + // Error, passed variant is not a real component ERR(Ref(this), - "Failed to set data in component\n", - "ID, ", id, "is not a component" - ); - } - } else { - ecs_entity_t first_id = ECS_PAIR_FIRST(id); - ecs_entity_t second_id = ECS_PAIR_SECOND(id); - if ( - !ecs_has_id(world->raw(), first_id, ecs_id(EcsComponent)) - && !ecs_has_id(world->raw(), second_id, ecs_id(EcsComponent)) - ) { - // ID is not a component, err - ERR(Ref(this), - "Failed to set data in pair\n", - "Neither ID ", first_id, - " nor ", second_id, "are components" + "Failed to add component to entity\n", + "ID coerced from ", component, " is not a component" ); + } + + ecs_add_id(w->raw(), get_id(), c_id); } - // TODO: change build_data_from_variant to a static method - Ref(memnew(GFComponent(get_id(), id, world))) - ->build_data_from_variant( - data, - ecs_get_mut_id(world->raw(), get_id(), id) - ); - ecs_modified_id(world->raw(), get_id(), id); + GFComponent::build_data_from_members( + members, + ecs_get_mut_id(w->raw(), get_id(), c_id), + c_id, + get_world() + ); + ecs_modified_id(w->raw(), get_id(), c_id); return Ref(this); } @@ -270,12 +314,22 @@ void GFEntity::_bind_methods() { godot::ClassDB::bind_static_method(GFEntity::get_class_static(), D_METHOD("from", "entity", "world"), &GFEntity::from, nullptr); godot::ClassDB::bind_static_method(GFEntity::get_class_static(), D_METHOD("from_id", "id", "world"), &GFEntity::from_id, nullptr); - godot::ClassDB::bind_method(D_METHOD("add_component", "component", "data"), &GFEntity::add_component, nullptr); - godot::ClassDB::bind_method(D_METHOD("add_entity", "entity", "data"), &GFEntity::add_entity, nullptr); - godot::ClassDB::bind_method(D_METHOD("add_pair", "first", "second", "data"), &GFEntity::add_pair, nullptr); + { + MethodInfo mi; + mi.arguments.push_back(PropertyInfo(Variant::NIL, "component")); + mi.name = "add_component"; + godot::ClassDB::bind_vararg_method(METHOD_FLAGS_DEFAULT, StringName("add_component"), &GFEntity::add_component, mi); + } + { + MethodInfo mi; + mi.arguments.push_back(PropertyInfo(Variant::NIL, "component")); + mi.name = "set_component"; + godot::ClassDB::bind_vararg_method(METHOD_FLAGS_DEFAULT, StringName("set_component"), &GFEntity::set_component, mi); + } + + godot::ClassDB::bind_method(D_METHOD("add_pair", "first", "second", "data"), &GFEntity::add_pair, Array()); godot::ClassDB::bind_method(D_METHOD("add_tag", "tag"), &GFEntity::add_tag); godot::ClassDB::bind_method(D_METHOD("get_component", "component"), &GFEntity::get_component); - godot::ClassDB::bind_method(D_METHOD("set_component", "component", "value"), &GFEntity::set_component); godot::ClassDB::bind_method(D_METHOD("delete"), &GFEntity::delete_); diff --git a/cpp/src/entity.h b/cpp/src/entity.h index f80b35b..2cc02ae 100644 --- a/cpp/src/entity.h +++ b/cpp/src/entity.h @@ -35,13 +35,17 @@ namespace godot { static Ref from(Variant, GFWorld*); static Ref from_id(ecs_entity_t, GFWorld*); - Ref add_component(Variant, Variant data); - Ref add_entity(Variant, Variant data); - Ref add_pair(Variant, Variant, Variant data); + Ref add_component(const Variant**, GDExtensionInt, GDExtensionCallError&); + Ref _add_component(Variant, Array); + + Ref add_pair(Variant, Variant, Array data); Ref add_tag(Variant); Ref get_component(Variant); - Ref set_component(Variant, Variant data); + + Ref set_component(const Variant**, GDExtensionInt, GDExtensionCallError&); + Ref _set_component(Variant, Array); + void delete_(); diff --git a/cpp/src/utils.cpp b/cpp/src/utils.cpp index 183b97c..2c6dcfe 100644 --- a/cpp/src/utils.cpp +++ b/cpp/src/utils.cpp @@ -1,6 +1,7 @@ #include "utils.h" #include "godot_cpp/core/defs.hpp" +#include "godot_cpp/variant/utility_functions.hpp" #include "godot_cpp/variant/variant.hpp" #include "world.h" @@ -159,6 +160,70 @@ void Utils::set_gd_struct_from_variant( } } +bool Utils::can_convert_type_to_primitive(Variant::Type type, ecs_primitive_kind_t primi) { + switch (type) { + case Variant::NIL: + return false; + case Variant::BOOL: + return primi == ecs_primitive_kind_t::EcsBool; + case Variant::INT: { + switch (primi) { + case EcsU8: case EcsU16: case EcsU32: case EcsU64: + case EcsI8: case EcsI16: case EcsI32: case EcsI64: + return true; + default: + return false; + } + } + case Variant::FLOAT: { + switch (primi) { + case EcsF32: case EcsF64: + return true; + default: + return false; + } + } + case Variant::STRING: + case Variant::VECTOR2: + case Variant::VECTOR2I: + case Variant::RECT2: + case Variant::RECT2I: + case Variant::VECTOR3: + case Variant::VECTOR3I: + case Variant::TRANSFORM2D: + case Variant::VECTOR4: + case Variant::VECTOR4I: + case Variant::PLANE: + case Variant::QUATERNION: + case Variant::AABB: + case Variant::BASIS: + case Variant::TRANSFORM3D: + case Variant::PROJECTION: + case Variant::COLOR: + case Variant::STRING_NAME: + case Variant::NODE_PATH: + case Variant::RID: + case Variant::OBJECT: + case Variant::CALLABLE: + case Variant::SIGNAL: + case Variant::DICTIONARY: + case Variant::ARRAY: + case Variant::PACKED_BYTE_ARRAY: + case Variant::PACKED_INT32_ARRAY: + case Variant::PACKED_INT64_ARRAY: + case Variant::PACKED_FLOAT32_ARRAY: + case Variant::PACKED_FLOAT64_ARRAY: + case Variant::PACKED_STRING_ARRAY: + case Variant::PACKED_VECTOR2_ARRAY: + case Variant::PACKED_VECTOR3_ARRAY: + case Variant::PACKED_COLOR_ARRAY: + case Variant::PACKED_VECTOR4_ARRAY: + case Variant::VARIANT_MAX: + return false; + } + return false; +} + void Utils::set_primitive_from_variant( const Variant value, const ecs_primitive_kind_t primi_kind, @@ -194,6 +259,13 @@ void Utils::set_type_from_variant( ) { const EcsPrimitive* primi = ecs_get(world, type, EcsPrimitive); if (primi != nullptr) { + if (!Utils::can_convert_type_to_primitive(value.get_type(), primi->kind)) { + // TODO: Display names of primitive and variant type, instead of numbers + ERR(/**/, "Set value failed.\n", + "Setting Flecs primitive type, ", primi->kind, ", to value of Godot type, ", + value.get_type(), ", is invalid." + ); + } set_primitive_from_variant(value, primi->kind, out); return; } diff --git a/cpp/src/utils.h b/cpp/src/utils.h index fa060a1..73623d9 100644 --- a/cpp/src/utils.h +++ b/cpp/src/utils.h @@ -94,6 +94,7 @@ namespace godot { static Variant primitive_value_to_variant(const void*, ecs_primitive_kind_t); static Variant::Type primitive_type_to_variant(ecs_primitive_kind_t); static EntityResult variant_type_to_id(Variant::Type type); + static bool can_convert_type_to_primitive(Variant::Type type, ecs_primitive_kind_t primi); static String into_pascal_case(String str); diff --git a/cpp/src/world.cpp b/cpp/src/world.cpp index daa1ce9..7a9c948 100644 --- a/cpp/src/world.cpp +++ b/cpp/src/world.cpp @@ -690,6 +690,12 @@ ecs_entity_t GFWorld::register_script_id(Ref