Skip to content

adds missing exec error #26

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

Merged
merged 1 commit into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
155 changes: 99 additions & 56 deletions ecsact/entt/runtime.hh
Original file line number Diff line number Diff line change
Expand Up @@ -586,12 +586,13 @@ private:
const auto system_id = ecsact_id_cast<ecsact_system_like_id>(SystemT::id);

const void* action_data = nullptr;
auto each_cb = [&](auto& view, auto& assoc_views, auto entity) {
if constexpr(!mp_empty<ChildSystemsListT>::value) {
system_execution_context<SystemT>
ctx(info, system_id, view, assoc_views, entity, parent, action_data);
_execute_systems_list<ChildSystemsListT>(info, ctx.cptr(), actions);
}

auto each_cb = [&](auto& view, auto& assoc_views, auto entity) {
if constexpr(!mp_empty<ChildSystemsListT>::value) {
system_execution_context<SystemT>
ctx(info, system_id, view, assoc_views, entity, parent, action_data);
_execute_systems_list<ChildSystemsListT>(info, ctx.cptr(), actions);
}
};

if constexpr(is_action<SystemT>()) {
Expand Down Expand Up @@ -660,53 +661,55 @@ private:
auto assoc_views = system_association_views<SystemT>(info.registry);
auto assoc_views_itrs = system_association_views_iterators(assoc_views);
const void* action_data = nullptr;
auto itr_view = [&] {
for(auto entity : view) {
bool missing_assoc_entities = false;
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 ComponentT = typename Assoc::component_type;

auto& assoc_view = std::get<I>(assoc_views);
auto& assoc_view_itr = std::get<I>(assoc_views_itrs);
constexpr std::size_t offset = Assoc::field_offset;
assert(view.contains(entity));
auto& comp = view.template get<ComponentT>(entity);
auto field_entity_value = *reinterpret_cast<const ecsact_entity_id*>(
reinterpret_cast<const char*>(&comp) + offset
);
auto entt_field_entity_value =
info.get_entt_entity_id(field_entity_value);

bool found_associated_entity = false;
for(; assoc_view_itr != assoc_view.end(); ++assoc_view_itr) {
found_associated_entity = *assoc_view_itr ==
entt_field_entity_value;
if(found_associated_entity) {
break;
}
}

if(!found_associated_entity) {
missing_assoc_entities = true;
}
});

if(!missing_assoc_entities) {
_execute_system_user_itr<SystemT, ChildSystemsListT>(
info,
view,
assoc_views,
entity,
parent,
action_data,
actions
);
}
}

auto itr_view = [&] {
for(auto entity : view) {
bool missing_assoc_entities = false;
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 ComponentT = typename Assoc::component_type;

auto& assoc_view = std::get<I>(assoc_views);
auto& assoc_view_itr = std::get<I>(assoc_views_itrs);
constexpr auto offset = Assoc::field_offset;
assert(view.contains(entity));
auto& comp = view.template get<ComponentT>(entity);

auto field_entity_value = *reinterpret_cast<const ecsact_entity_id*>(
reinterpret_cast<const char*>(&comp) + offset
);
auto entt_field_entity_value =
info.get_entt_entity_id(field_entity_value);

bool found_associated_entity = false;
for(; assoc_view_itr != assoc_view.end(); ++assoc_view_itr) {
found_associated_entity = *assoc_view_itr ==
entt_field_entity_value;
if(found_associated_entity) {
break;
}
}

if(!found_associated_entity) {
missing_assoc_entities = true;
}
});

if(!missing_assoc_entities) {
_execute_system_user_itr<SystemT, ChildSystemsListT>(
info,
view,
assoc_views,
entity,
parent,
action_data,
actions
);
}
}
};

if constexpr(is_action<SystemT>()) {
Expand Down Expand Up @@ -1094,6 +1097,32 @@ private:
});
}

auto _validate_action(ecsact_registry_id registry_id, ecsact_action& action)
-> ecsact_execute_systems_error {
using boost::mp11::mp_for_each;

auto& info = _registries.at(registry_id);
auto result = ECSACT_EXEC_SYS_OK;

mp_for_each<typename package::actions>([&]<typename A>(A) {
if(A::id != action.action_id) {
return;
}
constexpr auto fields_info = ecsact::fields_info<A>();
for(auto& field : fields_info) {
if(field.storage_type == ECSACT_ENTITY_TYPE) {
auto entity_field =
field.template get<ecsact_entity_id>(action.action_data);
if(!info.entities_map.contains(entity_field)) {
result = ECSACT_EXEC_SYS_ERR_ACTION_ENTITY_INVALID;
}
}
}
});

return result;
}
Comment on lines +1100 to +1124
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the relevant part


public:
#ifdef ECSACT_ENTT_RUNTIME_DYNAMIC_SYSTEM_IMPLS
bool set_system_execution_impl(
Expand All @@ -1115,10 +1144,24 @@ public:
const ecsact_execution_options* execution_options_list,
std::optional<execution_events_collector> events_collector
) {
std::mutex mutex;
auto& info = _registries.at(reg_id);
auto mutex = std::mutex{};
auto& info = _registries.at(reg_id);
auto exec_err = ECSACT_EXEC_SYS_OK;
info.mutex = std::ref(mutex);

if(execution_options_list != nullptr) {
for(int n = 0; execution_count > n; ++n) {
auto opts = execution_options_list[n];
for(auto act_idx = 0; opts.actions_length > act_idx; ++act_idx) {
auto& act = opts.actions[act_idx];
exec_err = _validate_action(reg_id, act);
if(exec_err != ECSACT_EXEC_SYS_OK) {
return exec_err;
}
}
}
}

for(int n = 0; execution_count > n; ++n) {
actions_span_t actions;
if(execution_options_list != nullptr) {
Expand All @@ -1142,7 +1185,7 @@ public:
_clear_event_markers(info);

info.mutex = std::nullopt;
return ECSACT_EXEC_SYS_OK;
return exec_err;
}
};

Expand Down
3 changes: 1 addition & 2 deletions runtime/core.template.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
using namespace ecsact_entt_rt;

ecsact_registry_id ecsact_create_registry(const char* registry_name) {
return static_cast<ecsact_registry_id>(runtime.create_registry(registry_name)
);
return runtime.create_registry(registry_name);
}

void ecsact_destroy_registry(ecsact_registry_id reg_id) {
Expand Down
41 changes: 34 additions & 7 deletions runtime/test/runtime_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,29 @@ void runtime_test::OtherEntitySystem::impl(context& ctx) {
void runtime_test::MakeAnother::impl(context& ctx) {
}

void runtime_test::TrivialRemove::impl(context& ctx) {
void runtime_test::AlwaysRemove::impl(context& ctx) {
// This trivial remove should not even be required:
// SEE: https://github.com/ecsact-dev/ecsact_lang_cpp/issues/80
std::cerr << "TriviaLRemove impl called (SHOULD NOT HAPPEN)\n";
std::cerr << "AlwaysRemove impl called (SHOULD NOT HAPPEN)\n";
std::cerr.flush();
std::abort();
}

void runtime_test::AlwaysRemove::impl(context& ctx) {
void runtime_test::TrivialRemove::impl(context& ctx) {
// This trivial remove should not even be required:
// SEE: https://github.com/ecsact-dev/ecsact_lang_cpp/issues/80
std::cerr << "AlwaysRemove impl called (SHOULD NOT HAPPEN)\n";
std::cerr << "TriviaLRemove impl called (SHOULD NOT HAPPEN)\n";
std::cerr.flush();
std::abort();
}

void runtime_test::AssocTestAction::impl(context& ctx) {
ctx.add(OtherEntityComponent{
.num = 42,
.target = ctx.action().assoc_entity,
});
}

TEST(Core, CreateRegistry) {
auto reg_id = ecsact_create_registry("CreateRegistry");
EXPECT_NE(reg_id, ecsact_invalid_registry_id);
Expand Down Expand Up @@ -275,9 +282,9 @@ TEST(Core, TrivialRemoveEvent) {
}

TEST(Core, DynamicSystemImpl) {
ecsact::core::registry reg("DynamicSystemImpl");
auto entity = reg.create_entity();
auto other_entity = reg.create_entity();
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);
Expand Down Expand Up @@ -318,6 +325,26 @@ TEST(Core, DynamicSystemImpl) {
EXPECT_EQ(comp_get.a, comp.a);
}

TEST(Core, ExecuteSystemsErrors) {
auto reg = ecsact::core::registry("ExecuteSystemsErrors");
auto comp = OtherEntityComponent{
.num = 42,
.target = static_cast<ecsact_entity_id>(4000),
};
auto options = ecsact_execution_options{};
auto test_action = runtime_test::AssocTestAction{};
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;
auto exec_err = ecsact_execute_systems(reg.id(), 1, &options, nullptr);

EXPECT_EQ(exec_err, ECSACT_EXEC_SYS_ERR_ACTION_ENTITY_INVALID);
}

#ifdef ECSACT_ENTT_TEST_STATIC_SYSTEM_IMPL
TEST(Core, StaticSystemImpl) {
auto reg_id = ecsact_create_registry("StaticSystemImpl");
Expand Down
7 changes: 7 additions & 0 deletions runtime/test/runtime_test.ecsact
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,10 @@ action MakeAnother {
required OtherEntityComponent;
}
}

action AssocTestAction {
entity assoc_entity;
readwrite ComponentA;
adds OtherEntityComponent;
}