fix: generates missing internal sorting (#115)

Author: zaucy

ecsact/entt/wrapper/dynamic.hh

@@ -188,6 +188,7 @@ auto context_generate_add(
 
 	const auto& component = *static_cast<const C*>(component_data);
 	registry.template emplace<pending_add<C>>(entity, component);
+	detail::add_system_markers_if_needed<C>(registry, entity);
 }
 
 } // namespace ecsact::entt::wrapper::dynamic

rt_entt_codegen/core/print_sys_exec.cc

@@ -392,6 +392,8 @@ static auto print_sys_exec_ctx_generate(
 	using ecsact::cc_lang_support::cpp_identifier;
 	using ecsact::cpp_codegen_plugin_util::block;
 	using ecsact::meta::decl_full_name;
+	using ecsact::rt_entt_codegen::ecsact_entt_system_details;
+	using ecsact::rt_entt_codegen::get_all_sorted_systems;
 	using ecsact::rt_entt_codegen::util::method_printer;
 
 	auto printer = //
@@ -543,6 +545,9 @@ static auto print_apply_pendings(
 }
 
 template<typename SystemLikeID>
+	requires(!std::is_same_v<
+					 ecsact_system_like_id,
+					 std::remove_cvref_t<SystemLikeID>>)
 struct print_ecsact_entt_system_details_options {
 	SystemLikeID sys_like_id;
 	std::string  system_name;
@@ -609,8 +614,14 @@ static auto print_ecsact_entt_system_details(
 		additional_view_components.push_back(pending_lazy_exec_struct);
 	}
 
-	if(system_needs_sorted_entities(options.sys_like_id, details)) {
-		additional_view_components.push_back(system_sorting_struct_name);
+	constexpr auto is_system = std::is_same_v<
+		std::remove_cvref_t<decltype(options.sys_like_id)>,
+		ecsact_system_id>;
+
+	if constexpr(is_system) {
+		if(system_needs_sorted_entities(options.sys_like_id)) {
+			additional_view_components.push_back(system_sorting_struct_name);
+		}
 	}
 
 	if(is_notify_system(options.sys_like_id)) {
@@ -633,8 +644,10 @@ static auto print_ecsact_entt_system_details(
 		additional_view_components
 	);
 
-	if(system_needs_sorted_entities(options.sys_like_id, details)) {
-		ctx.write("view.use<", system_sorting_struct_name, ">();\n");
+	if constexpr(is_system) {
+		if(system_needs_sorted_entities(options.sys_like_id)) {
+			ctx.write("view.use<", system_sorting_struct_name, ">();\n");
+		}
 	}
 
 	block(ctx, "struct : ecsact_system_execution_context ", [&] {
@@ -825,6 +838,18 @@ static auto print_ecsact_entt_system_details(
 				ctx,
 				"for(ecsact::entt::entity_id entity : view_no_pending_lazy_)",
 				[&] {
+					ctx.write(
+						"// If this assertion triggers this is an indicator of a codegen "
+						"failure.\n"
+						"// Please report to https://github.com/ecsact-dev/ecsact_rt_entt\n"
+					);
+					ctx.write(
+						"assert(",
+						options.registry_var_name,
+						".all_of<",
+						system_sorting_struct_name,
+						">(entity));\n"
+					);
 					ctx.write("view_no_pending_lazy_count_ += 1;\n");
 					ctx.write(
 						options.registry_var_name,

rt_entt_codegen/core/sorting_components.cc

@@ -118,7 +118,7 @@ auto ecsact::rt_entt_codegen::core::print_entity_sorting_components(
 		auto sys_details =
 			ecsact_entt_system_details::from_system_like(sys_like_id);
 
-		if(system_needs_sorted_entities(sys_id, sys_details)) {
+		if(system_needs_sorted_entities(sys_id)) {
 			auto sys_like_id = ecsact_id_cast<ecsact_system_like_id>(sys_id);
 			auto sys_details =
 				ecsact_entt_system_details::from_system_like(sys_like_id);

rt_entt_codegen/core/system_markers.cc

@@ -45,25 +45,18 @@ auto ecsact::rt_entt_codegen::core::print_system_marker_add_fn(
 				continue;
 			}
 
-			if(system_needs_sorted_entities(system_id, sys_details)) {
+			if(system_needs_sorted_entities(system_id)) {
 				auto system_name = ecsact::meta::decl_full_name(system_id);
 				auto system_cpp_ident =
 					"::" + cc_lang_support::cpp_identifier(system_name);
 
 				auto system_sorting_struct_name =
 					std::format("system_sorted<{}>", system_cpp_ident);
 
-				block(
-					ctx,
-					"if(::ecsact::entt::entity_matches_system<" + system_cpp_ident +
-						">(reg, entity))",
-					[&] {
-						ctx.write(
-							"reg.emplace_or_replace<",
-							system_sorting_struct_name,
-							">(entity);"
-						);
-					}
+				ctx.write(
+					"reg.emplace_or_replace<",
+					system_sorting_struct_name,
+					">(entity);"
 				);
 			}
 		}

rt_entt_codegen/shared/BUILD.bazel

@@ -20,6 +20,7 @@ cc_library(
 cc_library(
     name = "sorting",
     hdrs = ["sorting.hh"],
+    srcs = ["sorting.cc"],
     copts = copts,
     deps = [
         ":ecsact_entt_details",

rt_entt_codegen/shared/sorting.cc

@@ -0,0 +1,32 @@
+#include "rt_entt_codegen/shared/sorting.hh"
+
+#include "ecsact/runtime/meta.hh"
+
+auto ecsact::rt_entt_codegen::system_needs_sorted_entities( //
+	ecsact_system_id id
+) -> bool {
+	auto needs_sorted_entities = false;
+
+	auto lazy_rate = ecsact_meta_get_lazy_iteration_rate(id);
+	if(lazy_rate > 0) {
+		needs_sorted_entities = true;
+	}
+
+	return needs_sorted_entities;
+}
+
+auto ecsact::rt_entt_codegen::get_all_sorted_systems()
+	-> std::vector<ecsact_system_like_id> {
+	auto sys_like_ids = std::vector<ecsact_system_like_id>{};
+	for(auto pkg_id : ecsact::meta::get_package_ids()) {
+		auto pkg_sys_ids = ecsact::meta::get_system_ids(pkg_id);
+
+		for(auto sys_id : pkg_sys_ids) {
+			if(system_needs_sorted_entities(sys_id)) {
+				sys_like_ids.push_back(ecsact_id_cast<ecsact_system_like_id>(sys_id));
+			}
+		}
+	}
+
+	return sys_like_ids;
+}

rt_entt_codegen/shared/sorting.hh

@@ -1,34 +1,10 @@
 #pragma once
 
-#include <type_traits>
+#include <vector>
+
 #include "ecsact/runtime/common.h"
-#include "ecsact/runtime/meta.h"
-#include "rt_entt_codegen/shared/ecsact_entt_details.hh"
 
 namespace ecsact::rt_entt_codegen {
-
-template<typename SystemLikeID>
-	requires(std::is_same_v<std::remove_cvref_t<SystemLikeID>, ecsact_system_id> ||
-					 std::is_same_v<std::remove_cvref_t<SystemLikeID>, ecsact_action_id>)
-ECSACT_ALWAYS_INLINE auto system_needs_sorted_entities(
-	SystemLikeID                      id,
-	const ecsact_entt_system_details& details
-) -> bool {
-	auto sys_like_id = ecsact_id_cast<ecsact_system_like_id>(id);
-	auto needs_sorted_entities = false;
-
-	if constexpr(std::is_same_v<
-								 std::remove_cvref_t<SystemLikeID>,
-								 ecsact_system_id>) {
-		auto lazy_rate = ecsact_meta_get_lazy_iteration_rate(
-			static_cast<ecsact_system_id>(sys_like_id)
-		);
-
-		if(lazy_rate > 0) {
-			needs_sorted_entities = true;
-		}
-	}
-
-	return needs_sorted_entities;
-}
+auto system_needs_sorted_entities(ecsact_system_id id) -> bool;
+auto get_all_sorted_systems() -> std::vector<ecsact_system_like_id>;
 } // namespace ecsact::rt_entt_codegen

test/runtime_test.cc

@@ -144,6 +144,12 @@ void runtime_test::LazyParentSystem::LazyParentNestedSystem::impl( //
 	ctx.update(comp);
 }
 
+auto runtime_test::LazyUpdateGeneratedEntity::impl(context& ctx) -> void {
+	auto comp = ctx.get<ComponentA>();
+	comp.a += 1;
+	ctx.update(comp);
+}
+
 static std::atomic_bool AddAssocTest_ran = false;
 
 void runtime_test::AddAssocTest::impl(context& ctx) {
@@ -312,8 +318,8 @@ TEST(Core, AddComponentError) {
 	// invalid entity error.
 	EXPECT_EQ(add_err, ECSACT_ADD_ERR_ENTITY_INVALID);
 
-	// When we receive an error when adding a component our component will not be
-	// added to the registry.
+	// When we receive an error when adding a component our component will not
+	// be added to the registry.
 	EXPECT_FALSE(reg.has_component<OtherEntityComponent>(entity));
 }
 
@@ -761,10 +767,7 @@ TEST(Core, DynamicSystemImpl) {
 }
 
 TEST(Core, GeneratesCreateEvent) {
-	ecsact_set_system_execution_impl(
-		ecsact_id_cast<ecsact_system_like_id>(runtime_test::MakeAnother::id),
-		&runtime_test__MakeAnother
-	);
+	SET_SYSTEM_IMPL(MakeAnother);
 
 	auto reg = ecsact::core::registry("GeneratesCreateEvent");
 
@@ -1184,6 +1187,67 @@ TEST(Core, LazyParentSystem) {
 #undef CALC_CHANGED_INDEX
 }
 
+TEST(Core, LazyUpdateGeneratedEntity) {
+	using runtime_test::ComponentA;
+	using runtime_test::ComponentB;
+	using runtime_test::MakeAnother;
+
+	// these systems mess with ComponentA
+	CLEAR_SYSTEM_IMPL(AddsAutoRemovedTag);
+	CLEAR_SYSTEM_IMPL(SimpleSystem);
+	CLEAR_SYSTEM_IMPL(OtherEntitySystem);
+	CLEAR_SYSTEM_IMPL(AssocTestAction);
+	CLEAR_SYSTEM_IMPL(TestAction);
+
+	auto reg = ecsact::core::registry{"Core_LazyUpdateGeneratedEntity"};
+
+	auto test_entity = reg.create_entity();
+	reg.add_component(test_entity, ComponentA{});
+	reg.add_component(test_entity, ComponentB{});
+
+	auto exec_opts = ecsact::core::execution_options{};
+	auto make_another_action = MakeAnother{};
+	exec_opts.push_action(&make_another_action);
+
+	// generate 7 more entities from MakeAnother system impl
+	SET_SYSTEM_IMPL(MakeAnother);
+	auto err = reg.execute_systems(std::array{exec_opts, exec_opts, exec_opts});
+	ASSERT_EQ(err, ECSACT_EXEC_SYS_OK);
+	ASSERT_EQ(reg.count_entities(), 8);
+	CLEAR_SYSTEM_IMPL(MakeAnother);
+
+	auto count_entities_with_value = [&](int num) -> int {
+		auto count = 0;
+		for(auto entity : reg.get_entities()) {
+			auto comp = reg.get_component<ComponentA>(entity);
+
+			if(comp.a == num) {
+				count += 1;
+			}
+		}
+
+		return count;
+	};
+
+	// this system should increment ComponentA::a one by one
+	SET_SYSTEM_IMPL(LazyUpdateGeneratedEntity);
+
+	reg.execute_systems();
+	EXPECT_EQ(count_entities_with_value(1), 1);
+
+	reg.execute_systems();
+	EXPECT_EQ(count_entities_with_value(1), 2);
+
+	reg.execute_systems();
+	EXPECT_EQ(count_entities_with_value(1), 3);
+
+	reg.execute_systems();
+	EXPECT_EQ(count_entities_with_value(1), 4);
+
+	reg.execute_systems();
+	EXPECT_EQ(count_entities_with_value(1), 5);
+}
+
 #ifdef ECSACT_ENTT_TEST_STATIC_SYSTEM_IMPL
 TEST(Core, StaticSystemImpl) {
 	auto reg_id = ecsact_create_registry("StaticSystemImpl");
@@ -1200,7 +1264,8 @@ TEST(Core, StaticSystemImpl) {
 	// Sanity check
 	ASSERT_EQ(comp_get->a, comp.a);
 
-	// Clear any system impls that may already be set so we can use the static one
+	// Clear any system impls that may already be set so we can use the static
+	// one
 	ecsact_set_system_execution_impl(
 		ecsact_id_cast<ecsact_system_like_id>(runtime_test::SimpleSystem::id),
 		nullptr

test/runtime_test.ecsact

@@ -198,6 +198,10 @@ system LazyParentSystem(lazy) {
     }
 }
 
+system LazyUpdateGeneratedEntity(lazy) {
+  readwrite ComponentA;
+}
+
 // Notify systems
 
 component NotifyComponentA {