Dev

[ThorstenSuckow]

No description provided.

[Copilot]

Pull request overview

This PR updates Helios ECS iteration and lifecycle handling by introducing “optional components” support in ecs::View and simplifying entity activation to rely solely on the Active tag.

Changes:

• Refactors ecs::View into a PartialView model, adds withOptional<...>(), and extends iteration to yield nullptr for missing optional components.
• Removes Inactive tag usage from Entity::setActive() and adjusts activation logic accordingly.
• Updates documentation/comments around view iteration return values and activation behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
include/helios/ecs/View.ixx	Introduces PartialView + View alias and adds optional-component iteration via withOptional<...>().
include/helios/ecs/Entity.ixx	Simplifies activation toggling to only add/remove Active and updates related logic/docs.

Comments suppressed due to low confidence (1)

include/helios/ecs/Entity.ixx:301

• The setActive doc comment still states that activating removes an Inactive tag component, but this function no longer adds/removes Inactive at all. Update the comment to match the new behavior, or reintroduce the Inactive tag handling if other parts of the ECS still expect it.

         * @details When deactivated:
         * - The `Active` tag component is removed
         * - `onDeactivate()` is called on components that support it
         * - If a `HierarchyComponent` is present, it is marked dirty for propagation
         *
         * When activated:
         * - The `Inactive` tag component is removed
         * - An `Active` tag component is added
         * - `onActivate()` is called on components that support it

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

withOptional() constructs a PartialView with a non-empty TOptional... pack, but this constructor unconditionally static_assert(sizeof...(TOptional) == 0). As written, any call to withOptional<...>() will fail to compile. Move the assertion/constraint onto withOptional() (e.g., require the current view to have no optional components) or remove it from this constructor and enforce the “single call” rule differently.

Suggested change

static_assert(sizeof...(TOptional) == 0, "withOptional() should provide all optional components types in a single call.");

[Copilot]

The optional-component IIFE lambda returns nullptr in some branches and a component pointer in others. With deduced auto return type, return nullptr; deduces std::nullptr_t, which won’t match TOptional* and typically fails return-type deduction. Explicitly specify the lambda return type (e.g., -> decltype(sets->get(entityId))) and cast nullptr to that pointer type in the early-return branches.

Suggested change

	([filterEnabledOnly, entityId, &sets]() {
	if (!sets || !sets->contains(entityId)) {
	return nullptr;
	}
	auto* component = sets->get(entityId);
	if constexpr (requires {component->isEnabled(); }) {
	if (filterEnabledOnly && !component->isEnabled()) {
	return nullptr;
	([filterEnabledOnly, entityId, &sets]() -> decltype(sets->get(entityId)) {
	if (!sets || !sets->contains(entityId)) {
	return static_cast<decltype(sets->get(entityId))>(nullptr);
	}
	auto* component = sets->get(entityId);
	if constexpr (requires {component->isEnabled(); }) {
	if (filterEnabledOnly && !component->isEnabled()) {
	return static_cast<decltype(sets->get(entityId))>(nullptr);

[Copilot]

isInActive is now just !isActive, which makes the subsequent condition if (!isInActive && !active) equivalent to if (isActive && !active) but harder to read (and the name suggests an Inactive tag that’s no longer used). Consider simplifying the boolean logic and renaming to isInactive/wasActive (or removing the extra variable) to avoid confusion.