Add component checking.

[sgodwincs]

Here's my attempt at #24

I wasn't sure where the tests should be added, maybe in the defer_fns.cpp test?

[vittorioromeo]

Thanks! LGTM - waiting for a test before merging. I think it would be best to have a separate has_component.cpp test file which would be very similar to defer_fns.cpp. There should be a few systems that defer the addition/removal of components and that verify that happens correctly.

Maybe something like:

• System 0 (matches entities with ct::test0):

  TEST_ASSERT(data.has_component(ct::test0, eid));
  TEST_ASSERT(!data.has_component(ct::test1, eid));
  data.defer([&]
  { 
      data.remove_component(ct::test0, eid); 
      data.add_component(ct::test1, eid);
  });

• System 1 (matches entities with ct::test1):

  TEST_ASSERT(!data.has_component(ct::test0, eid));
  TEST_ASSERT(data.has_component(ct::test1, eid));
  data.defer([&]
  { 
      data.add_component(ct::test0, eid); 
      data.remove_component(ct::test1, eid);
  });

...and have some entities ping-pong between system 0 and system 1 a few times.

[sgodwincs]

I'm working on the test at the moment, but I seem to have come across a problem. When attempting to use remove_component on the defer proxy, I am getting that the metadata class does not have a remove_component member as it is used by main_storage (see L81). Is this actually meant to be del_component in main_storage, because I don't see any tests that actually take into consideration remove_component on the defer/step proxies, so this may have gone by unnoticed.

[vittorioromeo]

@sgodwincs: it is, good catch! As you may have noticed my tests weren't very thorough.
I think it would be best to rename del_component with remove_component in order to make naming more consistent - as far as I can see there are no usages of del_component apart from its declaration and definition.

[sgodwincs]

Well, I've written a test, but I seem to be getting some weird behavior. I'll have to look more into tomorrow, but I'll paste it here in case you notice any small mistakes. The implementation of has_component is really simple, so I doubt the problem is with that, seems more to do with how the entities are being matched to s0 and s1 and remove_component. None of the assertions even fail, the test just never completes. I'm assuming it has something to do with removing a component that isn't added, but I'm not sure at the moment.

#include <ecst.hpp>
#include "./settings_generator.hpp"

namespace example
{
    using vrm::core::uint;
    using vrm::core::sz_t;

    namespace c
    {
        struct c0 { };
        struct c1 { };
    }

    namespace ct
    {
        constexpr auto c0 = ecst::tag::component::v<c::c0>;
        constexpr auto c1 = ecst::tag::component::v<c::c1>;
    }

    namespace s
    {
        struct s0
        {
            template <typename TData>
            void process(TData& data)
            {
                data.for_entities([&](auto eid)
                    {
                        TEST_ASSERT(data.has(ct::c0, eid));
                        TEST_ASSERT(!data.has(ct::c1, eid));

                        data.defer([eid](auto& proxy)
                            {
                                proxy.remove_component(ct::c0, eid);
                                proxy.add_component(ct::c1, eid);
                            });
                    });
            }
        };

        struct s1
        {
            template <typename TData>
            void process(TData& data)
            {
                data.for_entities([&](auto eid)
                    {
                        TEST_ASSERT(!data.has(ct::c0, eid));
                        TEST_ASSERT(data.has(ct::c1, eid));

                        data.defer([eid](auto& proxy)
                            {
                                proxy.remove_component(ct::c1, eid);
                                proxy.add_component(ct::c0, eid);
                            });
                    });
            }
        };
    }

#define SYS_TAG(x)                                     \
    namespace s                                        \
    {                                                  \
        struct x;                                      \
    }                                                  \
    namespace st                                       \
    {                                                  \
        constexpr auto x = ecst::tag::system::v<s::x>; \
    }

    SYS_TAG(s0)
    SYS_TAG(s1)

    namespace ecst_setup
    {
        constexpr auto make_csl()
        {
            namespace c = example::c;
            namespace sc = ecst::signature::component;
            namespace slc = ecst::signature_list::component;

            return slc::make(
                sc::make(ct::c0).contiguous_buffer(),
                sc::make(ct::c1).contiguous_buffer());
        }

        constexpr auto entity_count = ecst::sz_v<100>;

        constexpr auto make_ssl()
        {
            using ecst::sz_v;

            namespace c = example::c;
            namespace s = example::s;
            namespace ss = ecst::signature::system;
            namespace sls = ecst::signature_list::system;
            namespace ips = ecst::inner_parallelism::strategy;

            constexpr auto test_p = // .
                ips::split_every_n::v(sz_v<entity_count / 8>);

            constexpr auto ssig_s0 =     // .
                ss::make(st::s0)         // .
                    .parallelism(test_p) // .
                    .write(ct::c0);      // .

            constexpr auto ssig_s1 =
                ss::make(st::s1)
                    .parallelism(test_p)
                    .write(ct::c1);

            return sls::make(ssig_s0, ssig_s1);
        }
    }

    namespace sea = ::ecst::system_execution_adapter;

    auto execute_systems = [&](auto& proxy)
    {
        proxy.execute_systems_from(st::s0, st::s1)( // .
            sea::all().for_subtasks([](auto& s, auto& data)
                {
                    s.process(data);
                }));
    };

    auto test_impl_f = [](auto& ctx)
    {
        ctx.step([&](auto& proxy)
            {
                for(sz_t ie = 0; ie < ecst_setup::entity_count; ++ie)
                {
                    auto e = proxy.create_entity();
                    TEST_ASSERT(!proxy.has_component(ct::c0, ecst::entity_id(ie)));
                    TEST_ASSERT(!proxy.has_component(ct::c1, ecst::entity_id(ie)));
                    proxy.add_component(ct::c0, e);
                }
            });

        ctx.step(execute_systems);
        ctx.step(execute_systems);

        ctx.step([&](auto& proxy)
            {
                for(sz_t ie = 0; ie < ecst_setup::entity_count; ++ie)
                {
                    TEST_ASSERT(proxy.has_component(ct::c0, ecst::entity_id(ie)));
                    TEST_ASSERT(!proxy.has_component(ct::c1, ecst::entity_id(ie)));
                }
            });
    };
}

int main()
{
    using namespace example;
    using namespace example::ecst_setup;

    test::run_tests(test_impl_f, entity_count, make_csl(), make_ssl());
    return 0;
}

[vittorioromeo]

I do get an assertion when I try to run your test:

./test/test.ecst.has_component

entity storage: dynamic (initial: 500)
multithreading: disallows inner parallelism
time: 8.55855ms

entity storage: fixed (capacity: 100)
multithreading: disallows inner parallelism
time: 5.44788ms

**entity storage: dynamic (initial: 500)
multithreading: allows inner parallelism
ASSERTION FAILED**

[file: "../include/./ecst/./inner_parallelism/./utils/execute_split.hpp"]
[line: 54]

split_count > 0

**split_count = 0**
0 = 0

(0 > 0) == false

0) Skip this assertion.
1) Skip all assertions.
_) Terminate the program.

This means that the chosen inner parallelism results in 0 splits. Why?

This is the chosen strategy:

constexpr auto test_p = ips::split_every_n::v(sz_v<entity_count / 8>);

It basically means: "split a system into an additional subtask every 100 / 8 entities". It assumes that the system has at least one entity. At the beginning of your test, s0 has some entities, but s1 hasn't, so the assertion is triggered.

This can be solved by composing inner parallelism strategies:

constexpr auto test_p =
    ipc::none_below_threshold::v(ecst::sz_v<10>,
        ips::split_evenly_fn::v_cores());

The above strategy means: "do not use inner parallelism if there are less than 10 entities in the system, otherwise split evenly between the number of hardware cores".

The test now compiles and passes! I am merging and manually adding the test. Thanks a lot!

[sgodwincs]

Good to know it was something simple! Though strange how I never actually got an assertion when I ran the test. Also, don't forget to fix the del_component when you make the changes since that wasn't in the PR.

[vittorioromeo]

bf751c9 should take care of it. I've also fixed another broken test