refactor(surface): minor changes

modules/ecs/private/sparse_set.test.cpp

@@ -76,8 +76,33 @@ Suite element_raii = [] {
 			set.remove(idx);
 
 			expect_eq(set.get_size(), 10'000 - (idx + 1));
-			expect_throw([&] { std::ignore = set.at(idx); });
 			expect_false(set.contains(idx));
+			expect_throw([&] { std::ignore = set.at(idx); });
+		}
+	};
+
+	Case { "removed elements won't be iterated again" } = [] {
+		auto set = Set {};
+
+		for (auto idx : std::views::iota(0, 10'000))
+		{
+			set.insert(idx, idx);
+		}
+
+		set.remove(0);
+		set.remove(32);
+		set.remove(69);
+		set.remove(420);
+		set.remove(9'999);
+
+		for (auto &[identifier, value] : set)
+		{
+			expect_eq(identifier, value);
+			expect_ne(value, 0);
+			expect_ne(value, 32);
+			expect_ne(value, 69);
+			expect_ne(value, 420);
+			expect_ne(value, 9'999);
 		}
 	};
 };

modules/ecs/public/registry.hpp

@@ -8,9 +8,16 @@ using Entity = uint32_t;
 
 /** A registry of components, the heart of an ECS architecture.
  *
- * @todo(Light): optimize multi-component views
+ * @todo(Light): Implement grouping
- * @todo(Light): support more than 2-component views
+ * @todo(Light): Implement identifier recycling
- * @todo(Light): handle edge cases or specify the undefined behaviors
+ * @todo(Light): Optimize views/each
+ * @todo(Light): Support >2 component views
+ * @todo(Light): Handle more edge cases or specify the undefined behaviors
+ *
+ * @ref https://skypjack.github.io/
+ * @ref https://github.com/alecthomas/entityx
+ * @ref https://github.com/skypjack/entt
+ * @ref https://github.com/SanderMertens/flecs
  */
 class Registry
 {

modules/ecs/public/sparse_set.hpp

@@ -25,10 +25,6 @@ public:
 	virtual void remove(Identifier_T identifier) = 0;
 };
 
-/**
- *
- * @todo(Light): implement identifier recycling.
- */
 template<typename Value_T, typename Identifier_T = uint32_t>
 class SparseSet: public TypeErasedSparseSet<Identifier_T>
 {
@@ -72,12 +68,38 @@ public:
 		return m_dense.emplace_back(identifier, std::move(value));
 	}
 
+	/** @warn invalidates begin/end iterators
+	 *
+	 * @todo(Light): make it not invalidate the iterators >:c
+	 */
 	void remove(Identifier_T identifier) override
 	{
 		auto &idx = m_sparse[identifier];
 		auto &[entity, component] = m_dense[idx];
 
+		auto &[last_entity, last_component] = m_dense.back();
+		auto &last_idx = m_sparse[last_entity];
+
+		// removed entity is in dense's back, just pop and invalidate sparse[identifier]
+		if (entity == last_entity)
+		{
 			idx = null_identifier;
+			m_dense.pop_back();
+		}
+		else
+		{
+			// swap dense's 'back' to 'removed'
+			std::swap(component, last_component);
+			entity = last_entity;
+
+			// make sparse point to new idx
+			last_idx = idx;
+
+			// pop dense and invalidate sparse[identifier]
+			idx = null_identifier;
+			m_dense.pop_back();
+		}
+
 		++m_dead_count;
 		--m_alive_count;
 	}

modules/surface/private/linux/system.cpp

@@ -58,7 +58,14 @@ System::System(Ref<ecs::Registry> registry): m_registry(std::move(registry))
 
 System::~System()
 {
+	// TODO(Light): make registry.remove not validate iterators
+	auto entities_to_remove = std::vector<ecs::Entity> {};
 	for (auto &[entity, surface] : m_registry->view<SurfaceComponent>())
+	{
+		entities_to_remove.emplace_back(entity);
+	}
+
+	for (auto entity : entities_to_remove)
 	{
 		m_registry->remove<SurfaceComponent>(entity);
 	}
@@ -157,8 +164,11 @@ void System::on_surface_construct(ecs::Registry &registry, ecs::Entity entity)
 			XUnmapWindow(display, main_window);
 		}
 	}
-	catch (...)
+	catch (const std::exception &exp)
 	{
+		log_err("Exception thrown when on_constructing surface component");
+		log_err("\tentity: {}", entity);
+		log_err("\twhat: {}", exp.what());
 		registry.remove<SurfaceComponent>(entity);
 		throw;
 	}