Skip to content

refactor: remove switch case from Parser::buildComponent()#14

Open
MichalSzandar wants to merge 5 commits into
feat/parser-implementationfrom
refactor/parser-refactor
Open

refactor: remove switch case from Parser::buildComponent()#14
MichalSzandar wants to merge 5 commits into
feat/parser-implementationfrom
refactor/parser-refactor

Conversation

@MichalSzandar
Copy link
Copy Markdown
Collaborator

@MichalSzandar MichalSzandar commented May 17, 2026

  • add ComponentRegistry class responsible for mapping protobuf component types with functions that create our Component objects
  • replace includes with forward declarations where it is possible
  • verify if Scene Objects contain only one component of given type
  • SceneObject::setTransform now takes Transform as an argument

…tRegistry

* replace some of includes with forward declarations
* verify if Scene Objects contain only one component of given type
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors component construction during protobuf scene parsing by introducing a ComponentRegistry to replace the switch in Parser::buildComponent(), while also reducing header dependencies via forward declarations and adding validation to prevent duplicate component types on a single SceneObject.

Changes:

  • Added ComponentRegistry singleton for mapping protobuf oneof cases to Component factory functions (with static registration in BlinkComponent).
  • Updated Parser to build components through the registry and to reject duplicate component types per scene object.
  • Refactored scene headers/usage (forward declarations, removed SceneAll.hpp) and changed SceneObject::setTransform to take a Transform value.

Reviewed changes

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

Show a summary per file
File Description
src/scene/components/ComponentRegistry.cpp Implements registry-based component construction.
src/scene/components/BlinkComponent.cpp Registers BlinkComponent factory into the registry at startup.
src/parser/Parser.cpp Switch-to-registry refactor; adds duplicate component-type validation; adapts to new setTransform signature.
src/CMakeLists.txt Adds new component sources; changes runtime_core to an OBJECT library.
protoFiles/tests/test_scene.pbtxt Updates test data to remove duplicate components per object.
include/scene/SceneObject.hpp Replaces include with forward declaration; changes setTransform signature.
include/scene/SceneAll.hpp Removes the umbrella header.
include/scene/Scene.hpp Replaces include with forward declaration.
include/scene/components/ComponentRegistry.hpp Declares the registry API and storage.
include/scene/components/BlinkComponent.hpp Declares factory function and adjusts includes.
include/parser/Parser.hpp Replaces includes with forward declarations.

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

Comment thread include/scene/SceneObject.hpp
Comment thread include/scene/SceneObject.hpp
Comment thread include/scene/components/ComponentRegistry.hpp
Copy link
Copy Markdown

@WojtussToKox WojtussToKox left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants