Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

8777 Cleanup hookstate usage around components #8863

Merged
merged 9 commits into from
Sep 29, 2023

Conversation

DanielBelmes
Copy link
Contributor

@DanielBelmes DanielBelmes commented Sep 20, 2023

Summary

Changed removeComponent set to none as per hookstates api.
Removed EntityRemovedComponent system
Removed subscribable from existanceMapState

References

#8777

Explanation

🤖 Generated by Copilot at 347d53f

  • Simplify entity removal logic by using bitECS.removeEntity and getAllEntities instead of a custom EntityRemovedComponent and query (link, link, link, link, link, link, link, link, link, link, link, link)
  • Remove Subscribable generic parameter and subscribable() argument from existenceMapState property in Component interface and defineComponent function, as they are not needed (link, link, link)
  • Use none instead of undefined to set the component state when removing a component, and add some commented out code for future implementation of destroying and deleting the state map entry (link)
  • Add a comment explaining why a hookstate is needed for the stateMap entry even if the component is undefined in useOptionalComponent function (link)

Checklist

  • If this PR is still a WIP, convert to a draft
  • When this PR is ready, mark it as "Ready for review"
  • ensure all checks pass
  • Changes have been manually QA'd
  • Changes reviewed by at least 2 approved reviewers

@DanielBelmes DanielBelmes linked an issue Sep 20, 2023 that may be closed by this pull request
@DanielBelmes DanielBelmes changed the title 8777 bug not using hookstate destroy on statemap clear 8777 Cleanup hookstate usage around components Sep 20, 2023
Copy link
Member

@speigg speigg left a comment

Choose a reason for hiding this comment

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

Build is failing, please run npm run check-errors

Copy link
Member

@HexaField HexaField left a comment

Choose a reason for hiding this comment

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

we spoke, and the test can be improved

packages/engine/tests/ecs/ecs.test.ts Outdated Show resolved Hide resolved
@HexaField HexaField merged commit 26c5328 into dev Sep 29, 2023
13 checks passed
@HexaField HexaField deleted the 8777-bug-not-using-hookstate-destroy-on-statemap-clear branch September 29, 2023 22:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Cleanup hookstate usages in ComponentFunction
3 participants