Skip to content

fixes crash when adding/removing components from a 'other' context #31

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 8, 2023
Merged

fixes crash when adding/removing components from a 'other' context #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f58912
fix crashes caused by interacting with associated components
zaucy Jan 7, 2023
a0d69e1
moved dynamic test down
zaucy Jan 7, 2023
4b892e4
fixes, yo
zaucy Jan 8, 2023
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
39 changes: 37 additions & 2 deletions ecsact/entt/runtime.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,14 +486,17 @@ private:
using boost::mp11::mp_first;
using boost::mp11::mp_flatten;
using boost::mp11::mp_for_each;
using boost::mp11::mp_iota_c;
using boost::mp11::mp_map_find;
using boost::mp11::mp_push_back;
using boost::mp11::mp_size;
using boost::mp11::mp_transform;
using boost::mp11::mp_unique;
using ecsact::entt::detail::pending_add;
using ecsact::entt_mp11_util::mp_map_find_value_or;

using caps_info = ecsact::system_capabilities_info<SystemT>;
using associations = typename caps_info::associations;

using system_generates = mp_transform<
mp_first,
Expand All @@ -507,7 +510,7 @@ private:
::ecsact::mp_list<>>,
::ecsact::mp_list<>>>;

mp_for_each<addables>([&]<typename C>(C) {
auto for_each_addable = [&]<typename C>(C) {
using boost::mp11::mp_apply;
using boost::mp11::mp_bind_front;
using boost::mp11::mp_transform_q;
Expand All @@ -532,25 +535,52 @@ private:
}

info.registry.template clear<pending_add<C>>();
};

mp_for_each<addables>(for_each_addable);

mp_for_each<mp_iota_c<mp_size<associations>::value>>([&](auto I) {
using boost::mp11::mp_at;
using boost::mp11::mp_size_t;

using Assoc = mp_at<associations, mp_size_t<I>>;
using addables = typename Assoc::adds_components;

mp_for_each<addables>(for_each_addable);
});
}

template<typename SystemT>
void _apply_pending_removes(registry_info& info) {
using boost::mp11::mp_for_each;
using boost::mp11::mp_iota_c;
using boost::mp11::mp_size;
using ecsact::entt::detail::pending_remove;
using ecsact::entt_mp11_util::mp_map_find_value_or;

using caps_info = ecsact::system_capabilities_info<SystemT>;
using associations = typename caps_info::associations;

using removes_components = typename caps_info::removes_components;

mp_for_each<removes_components>([&]<typename C>(C) {
auto for_each_removable = [&]<typename C>(C) {
auto view = info.registry.template view<pending_remove<C>>();
view.each([&](auto entity) { info.template remove_component<C>(entity); }
);

info.registry.template clear<pending_remove<C>>();
};

mp_for_each<removes_components>(for_each_removable);

mp_for_each<mp_iota_c<mp_size<associations>::value>>([&](auto I) {
using boost::mp11::mp_at;
using boost::mp11::mp_size_t;

using Assoc = mp_at<associations, mp_size_t<I>>;
using removes_components = typename Assoc::removes_components;

mp_for_each<removes_components>(for_each_removable);
});
}

Expand Down Expand Up @@ -677,6 +707,11 @@ private:

auto& assoc_view = std::get<I>(assoc_views);
auto& assoc_view_itr = std::get<I>(assoc_views_itrs);
if(assoc_view.begin() == assoc_view.end()) {
missing_assoc_entities = true;
return;
}

assert(view.contains(entity));
auto& comp = view.template get<ComponentT>(entity);

Expand Down
148 changes: 103 additions & 45 deletions runtime/test/runtime_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ void runtime_test::TrivialRemove::impl(context& ctx) {
std::abort();
}

static std::atomic_bool AssocTestAction_ran = false;

void runtime_test::AssocTestAction::impl(context& ctx) {
AssocTestAction_ran = true;
ctx.add(OtherEntityComponent{
.num = 42,
.target = ctx.action().assoc_entity,
Expand All @@ -73,6 +76,17 @@ void runtime_test::AttackDamageWeakened::impl(context& ctx) {
// target_ctx.update(target_health);
}

static std::atomic_bool AddAssocTest_ran = false;

void runtime_test::AddAssocTest::impl(context& ctx) {
AddAssocTest_ran = true;
auto other_entity = ctx.get<OtherEntityComponent>();

// Get Target other context from OtherEntityComponent
auto target_ctx = ctx._ctx.other(other_entity.target);
target_ctx.add(AddAssocTestComponent{.num = 10});
}

TEST(Core, CreateRegistry) {
auto reg_id = ecsact_create_registry("CreateRegistry");
EXPECT_NE(reg_id, ecsact_invalid_registry_id);
Expand Down Expand Up @@ -393,51 +407,6 @@ TEST(Core, EventCollector) {
}
}

TEST(Core, DynamicSystemImpl) {
auto reg = ecsact::core::registry("DynamicSystemImpl");
auto entity = reg.create_entity();
auto other_entity = reg.create_entity();

ComponentA comp{.a = 42};
reg.add_component(entity, comp);
reg.add_component(other_entity, comp);

OtherEntityComponent other_comp{.num = 3, .target = other_entity};
ASSERT_EQ(reg.add_component(entity, other_comp), ECSACT_ADD_OK);

// Sanity check
ASSERT_TRUE(reg.has_component<ComponentA>(entity));
ASSERT_EQ(reg.get_component<ComponentA>(entity), comp);
ASSERT_TRUE(reg.has_component<ComponentA>(other_entity));
ASSERT_EQ(reg.get_component<ComponentA>(other_entity), comp);
ASSERT_TRUE(reg.has_component<OtherEntityComponent>(entity));
ASSERT_EQ(reg.get_component<OtherEntityComponent>(entity), other_comp);

ecsact_set_system_execution_impl(
ecsact_id_cast<ecsact_system_like_id>(runtime_test::SimpleSystem::id),
&runtime_test__SimpleSystem
);

ecsact_set_system_execution_impl(
ecsact_id_cast<ecsact_system_like_id>(runtime_test::OtherEntitySystem::id),
&runtime_test__OtherEntitySystem
);

auto exec_err = ecsact_execute_systems(reg.id(), 1, nullptr, nullptr);
ASSERT_EQ(exec_err, ECSACT_EXEC_SYS_OK);

// Sanity check
ASSERT_TRUE(reg.has_component<ComponentA>(entity));

auto comp_get = reg.get_component<ComponentA>(entity);

EXPECT_NE(comp_get.a, comp.a);

// Simulate what the system should be doing.
comp.a += 2;
EXPECT_EQ(comp_get.a, comp.a);
}

TEST(Core, ExecuteSystemsErrors) {
auto reg = ecsact::core::registry("ExecuteSystemsErrors");
auto options = ecsact_execution_options{};
Expand Down Expand Up @@ -476,6 +445,50 @@ TEST(Core, ExecuteSystemsAssocActionOk) {
EXPECT_EQ(exec_err, ECSACT_EXEC_SYS_OK);
}

TEST(Core, AddAssocOk) {
ecsact_set_system_execution_impl(
ecsact_id_cast<ecsact_system_like_id>(runtime_test::AssocTestAction::id),
&runtime_test__AssocTestAction
);
ecsact_set_system_execution_impl(
ecsact_id_cast<ecsact_system_like_id>(runtime_test::AddAssocTest::id),
&runtime_test__AddAssocTest
);

auto reg = ecsact::core::registry("AddAssocOk");
auto test_entity1 = reg.create_entity();
reg.add_component(
test_entity1,
runtime_test::ComponentA{
.a = 42,
}
);

auto test_entity2 = reg.create_entity();
reg.add_component<runtime_test::AddAssocTestTag>(test_entity2);

auto options = ecsact_execution_options{};
auto test_action = runtime_test::AssocTestAction{
.assoc_entity = test_entity2,
};
auto test_action_c = ecsact_action{
.action_id = runtime_test::AssocTestAction::id,
.action_data = &test_action,
};

options.actions_length = 1;
options.actions = &test_action_c;
AddAssocTest_ran = false;
AssocTestAction_ran = false;
auto exec_err = ecsact_execute_systems(reg.id(), 1, &options, nullptr);
EXPECT_TRUE(AddAssocTest_ran) << "AddAssocTest Impl Didn't Executed";
EXPECT_TRUE(AssocTestAction_ran) << "AssocTestAction Impl Didn't Executed";
EXPECT_EQ(exec_err, ECSACT_EXEC_SYS_OK);

exec_err = ecsact_execute_systems(reg.id(), 1, nullptr, nullptr);
EXPECT_EQ(exec_err, ECSACT_EXEC_SYS_OK);
}

TEST(Core, AssociationEntityCorrectness) {
using runtime_test::AttackDamage;
using runtime_test::AttackDamageWeakened;
Expand Down Expand Up @@ -585,6 +598,51 @@ TEST(Core, AssociationEntityCorrectness) {
}
}

TEST(Core, DynamicSystemImpl) {
auto reg = ecsact::core::registry("DynamicSystemImpl");
auto entity = reg.create_entity();
auto other_entity = reg.create_entity();

ComponentA comp{.a = 42};
reg.add_component(entity, comp);
reg.add_component(other_entity, comp);

OtherEntityComponent other_comp{.num = 3, .target = other_entity};
ASSERT_EQ(reg.add_component(entity, other_comp), ECSACT_ADD_OK);

// Sanity check
ASSERT_TRUE(reg.has_component<ComponentA>(entity));
ASSERT_EQ(reg.get_component<ComponentA>(entity), comp);
ASSERT_TRUE(reg.has_component<ComponentA>(other_entity));
ASSERT_EQ(reg.get_component<ComponentA>(other_entity), comp);
ASSERT_TRUE(reg.has_component<OtherEntityComponent>(entity));
ASSERT_EQ(reg.get_component<OtherEntityComponent>(entity), other_comp);

ecsact_set_system_execution_impl(
ecsact_id_cast<ecsact_system_like_id>(runtime_test::SimpleSystem::id),
&runtime_test__SimpleSystem
);

ecsact_set_system_execution_impl(
ecsact_id_cast<ecsact_system_like_id>(runtime_test::OtherEntitySystem::id),
&runtime_test__OtherEntitySystem
);

auto exec_err = ecsact_execute_systems(reg.id(), 1, nullptr, nullptr);
ASSERT_EQ(exec_err, ECSACT_EXEC_SYS_OK);

// Sanity check
ASSERT_TRUE(reg.has_component<ComponentA>(entity));

auto comp_get = reg.get_component<ComponentA>(entity);

EXPECT_NE(comp_get.a, comp.a);

// Simulate what the system should be doing.
comp.a += 2;
EXPECT_EQ(comp_get.a, comp.a);
}

#ifdef ECSACT_ENTT_TEST_STATIC_SYSTEM_IMPL
TEST(Core, StaticSystemImpl) {
auto reg_id = ecsact_create_registry("StaticSystemImpl");
Expand Down
14 changes: 14 additions & 0 deletions runtime/test/runtime_test.ecsact
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,20 @@ action AssocTestAction {
adds OtherEntityComponent;
}

component AddAssocTestComponent {
i32 num;
}

component AddAssocTestTag;

system AddAssocTest {
readwrite ComponentA;
readwrite OtherEntityComponent with target {
include AddAssocTestTag;
adds AddAssocTestComponent;
}
}

system AttackDamage {
readwrite Attacking with target {
readwrite Health;
Expand Down