Add warning when a hierarchy component is missing

[nicopap]

trafficstars

Objective

A common pitfall since 0.8 is the requirement on ComputedVisibility being present on all ancestors of an entity that itself has ComputedVisibility, without which, the entity becomes invisible.

I myself hit the issue and got very confused, and saw a few people hit it as well, so it makes sense to provide a hint of what to do when such a situation is encountered.

• Fixes #5849
• Closes #5616
• Closes #2277
• Closes #5081

Solution

We now check that all entities with both a Parent and a ComputedVisibility component have parents that themselves have a ComputedVisibility component.

Note that the warning is only printed once.

We also add a similar warning to GlobalTransform.

This only emits a warning. Because sometimes it could be an intended behavior.

Alternatives:

• Do nothing and keep repeating to newcomers how to avoid recurring pitfalls
• Make the transform and visibility propagation tolerant to missing components (#5616)
• Probably archetype invariants, though the current draft would not allow detecting that kind of errors

---

Changelog

• Add a warning when encountering dubious component hierarchy structure

[ramirezmike]

I like this change but did have one comment about the term "HealthCheck" although please take it with a grain of salt; I'm pretty new at looking at Bevy code!

To me it's not quite clear from "hierarchy_healthcheck_system" or the VisibilitySystems enum variant "HealthCheck" that it's related to verifying that the entities within a hierarchy are consistent with regards to these specific (hierarchy-affecting?) components. My initial reaction is "what is a healthy hierarchy vs unhealthy?"

hierarchy_consistency_check_system and VisibilitySystems::CheckHierarchyConsistency or something like that? Again, it's totally possible HealthCheck is an established pattern that I'm oblivious to though just thought I'd comment and learn

[nicopap]

@ramirezmike I actually took the "health check" term from both neovim and my previous job (where we created a tool to check if internal services were online).

I do agree that what the system does is not well reflected in the helthcheck name, but I don't think that hierarchy_consistency_check is necessarily clearer than hierarchy_healthcheck. Neither of those hint at what it actually being checked, which is that no parents are missing the required component.

Maybe there is a better name yeT?

[ramirezmike]

I do agree that what the system does is not well reflected in the helthcheck name, but I don't think that hierarchy_consistency_check is necessarily clearer than hierarchy_healthcheck. Neither of those hint at what it actually being checked, which is that no parents are missing the required component.

Maybe there is a better name yeT?

Perhaps we will discover it!

hierarchy_component_mismatch_check?

[mockersf]

This feels like the same kind of diagnostic level as system ordering ambiguities, and with the same kind of consequences in case of an issue (it kinda doesn't work but not in an explicit way).

What do you think of having a resource ReportHierarchyIssue similar to ReportExecutionOrderAmbiguities, and add those systems if the resource is present?

[nicopap]

What do you think of having a resource ReportHierarchyIssue similar to ReportExecutionOrderAmbiguities, and add those systems if the resource is present?

I like the idea, but I think it should be on by default. Supposedly, the user only benefits from the warning message if they don't know why their code is not working. But adding the resource requires knowing why their code is not working, making the message moot. The resource could be added on #[cfg(debug_assertions)] though, which I think is still better than adding the plugin on cfg flag.

[nicopap]

In the last two weeks, I've encountered the ComputedVisibility issue one more time and answered five different #help threads that would have been answered by this diagnostic.

[nicopap]

Btw I counted three more questions that would have been answered by this diagnostic.

[bors[bot]]

Build failed (retrying...):

• build-and-install-on-iOS

[bors[bot]]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12