Skip to content

Conversation

@james7132
Copy link
Member

@james7132 james7132 commented Mar 15, 2022

Objective

First/naive attempt to generalize transform_propagate_system for any inheritable component.

Solution

  • Add a Heritable trait that defines the inheritance behavior across a hierarchy. GlobalTransform implements this trait.
  • Generalize transform_propagate_system using Heritable as generic systems.
  • Add extension trait for App for inserting these systems into the schedule.
  • Add dedicated system labels for scheduling.

This implementation includes the optimizations found in #4180.


There are a few drawbacks to this naive approach (originally detailed here: #4213 (reply in thread))

There are a few things to note with this initial set of assumptions if we are to generalize the existing transform_propagate_system.

  • Root queries are no longer sufficient unless we allow skipping levels. transform_propagate_system relies on every Entity from the root to the leaf to have the inheritable components.
  • Allowing skipping levels requires full hierarchy traversals for all inheritance systems, not just the hierarchies with the components present, which would negatively impact performance.
  • Top down inheritance is the only mode supported. Bottoms-up composition (i.e. AABBs as a BVH) isn't well supported. This could be supported via a different trait if need be.

Followup Work

Implement this for Visibility and ComputedVisibility. Will likely need to work with existing systems for computed visibility in some way.

@james7132 james7132 added the A-Transform Translations, rotations and scales label Mar 15, 2022
@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 15, 2022
@james7132 james7132 added C-Feature A new feature, making something new possible and removed S-Needs-Triage This issue needs to be labelled labels Mar 15, 2022
@james7132 james7132 marked this pull request as ready for review March 16, 2022 19:05
@superdump
Copy link
Contributor

‘Heritable’ is an odd word to me. I would have expected ‘Inheritable’. However, I feel like ‘Hierarchical’ or ‘Hierarchic’ might be a better fit semantically. Hmm.

@james7132
Copy link
Member Author

‘Heritable’ is an odd word to me. I would have expected ‘Inheritable’. However, I feel like ‘Hierarchical’ or ‘Hierarchic’ might be a better fit semantically. Hmm.

Heritable

  • (of a characteristic) transmissible from parent to offspring.

Seems pretty apt to me.

@ickk
Copy link
Contributor

ickk commented Mar 16, 2022

I'll throw in 'Hereditary' for good measure 😂

Heritable and inheritable are synonyms, a bit like flammable and inflammable. Personally I would err on the side of 'inheritable' because it's far more common.

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Apr 25, 2022
@Weibye Weibye added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Aug 10, 2022
@DJMcNab DJMcNab mentioned this pull request Aug 13, 2022
2 tasks
@nicopap nicopap mentioned this pull request Apr 21, 2023
1 task
@bas-ie
Copy link
Contributor

bas-ie commented Oct 2, 2024

Closing as part of backlog cleanup: inactivity, no clear consensus.

@bas-ie bas-ie closed this Oct 2, 2024
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed A-Hierarchy labels Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events A-Transform Translations, rotations and scales C-Feature A new feature, making something new possible S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! X-Controversial There is active debate or serious implications around merging this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants