Skip to content

Component lifecycle reorganization and documentation #19543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Jun 8, 2025

Objective

I set out with one simple goal: clearly document the differences between each of the component lifecycle events via module docs.

Unfortunately, no such module existed: the various lifecycle code was scattered to the wind.
Without a unified module, it's very hard to discover the related types, and there's nowhere good to put my shiny new documentation.

Solution

  1. Unify the assorted types into a single bevy_ecs::component_lifecycle module.
  2. Write docs.
  3. Write a migration guide.

Testing

Thanks CI!

Follow-up

  1. The lifecycle event names are pretty confusing, especially OnReplace. We should consider renaming those. No bikeshedding in my PR though!
  2. Observers need real module docs too :(
  3. Any additional functional changes should be done elsewhere; this is a simple docs and re-org PR.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Uncontroversial This work is generally agreed upon D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 8, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jun 8, 2025
@alice-i-cecile alice-i-cecile added D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jun 8, 2025
@james-j-obrien
Copy link
Contributor

This is a nit: while not strictly contradictory nor inaccurate, these three passages all seem to have been written at different points in time and so come across as inconsistent:
On RemovedComponents:

/// Note that this does not allow you to see which data existed before removal.
/// If you need this, you will need to track the component data value on your own,
/// using a regularly scheduled system that requests `Query<(Entity, &T), Changed<T>>`
/// and stores the data somewhere safe to later cross-reference.

On on_insert and on_replace:

/// # Warning
///
/// The hook won't run if the component is already present and is only mutated, such as in a system via a query.
/// As a result, this is *not* an appropriate mechanism for reliably updating indexes and other caches.

Compared to the crate root:

//! To reliably synchronize data structures using with component lifecycle events,
//! you can combine [`OnInsert`] and [`OnReplace`] to fully capture any changes to the data.
//! This is particularly useful in combination with immutable components,
//! to avoid any lifecycle-bypassing mutations.

The crate root references immutability to make the hook consistently update data structures while the corresponding method explicitly warns against using them to keep indexes up to date without mentioning immutable components. Similarly the comment on RemovedComponents makes no mention of either hooks or immutability as possible solutions and instead refers to change detection which is not mentioned in the crate root at all (Added is but Changed is not). These could just link to the module docs, my concern is that someone looks at the docs on a singular method and gets the wrong idea about what solutions are available.

Copy link
Contributor

@james-j-obrien james-j-obrien left a comment

Choose a reason for hiding this comment

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

Definitely an overall improvement and all seems accurate.


The same move has been done for the more internal (but public) `ComponentId` constants: `ON_ADD`, `ON_INSERT`, `ON_REMOVE`, `ON_REPLACE`, `ON_DESPAWN`.

The code for hooks (`HookContext`, `ComponentHook`, `ComponentHooks`) has been extracted from the very long `bevy_ecs::components` module, and now lives in the `bevy_ecs::components` module.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The code for hooks (`HookContext`, `ComponentHook`, `ComponentHooks`) has been extracted from the very long `bevy_ecs::components` module, and now lives in the `bevy_ecs::components` module.
The code for hooks (`HookContext`, `ComponentHook`, `ComponentHooks`) has been extracted from the very long `bevy_ecs::components` module, and now lives in the `bevy_ecs::component_lifecycle` module.

///
/// This information is stored in the [`ComponentInfo`] of the associated component.
///
/// There is two ways of configuring hooks for a component:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// There is two ways of configuring hooks for a component:
/// There are two ways of configuring hooks for a component:

/// 1. Defining the relevant hooks on the [`Component`] implementation
/// 2. Using the [`World::register_component_hooks`] method
///
/// # Example 2
Copy link
Contributor

Choose a reason for hiding this comment

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

"Example 2" made me wonder what happened to example 1

@theotherphil
Copy link
Contributor

Having recently experienced the process of grep-ing around trying to find the relevant pieces: this is much better!

@Bleachfuel
Copy link
Contributor

Why not component::lifecycle, and have the lifecycle module be a submodule of the component module?

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 C-Code-Quality A section of code that is hard to understand or change C-Docs An addition or correction to our documentation D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon
Projects
Status: Observer overhaul
Development

Successfully merging this pull request may close these issues.

4 participants