Skip to content

bsn: improve docs and add tests#24464

Open
laundmo wants to merge 32 commits into
bevyengine:mainfrom
laundmo:bsn-better-docs
Open

bsn: improve docs and add tests#24464
laundmo wants to merge 32 commits into
bevyengine:mainfrom
laundmo:bsn-better-docs

Conversation

@laundmo
Copy link
Copy Markdown
Member

@laundmo laundmo commented May 27, 2026

Objective

Improve documentation of bevy_scene/bsn.

Rendered: https://share.yadamiel.com/docs/doc/bevy/scene/index.html

Closes: #24299
Closes: #24540
Closes: #24541

Solution

This PR depends on/includes changes from

Docs changes (approx)

Testing

  • cargo doc -Zrustdoc-mergeable-info -p bevy_scene --lib locally works fine
  • the other cargo doc command also works fine (see cargo.toml diff for full command, its ugh)

Todo

@laundmo laundmo changed the title progress on docs bsn: improve docs and add tests May 27, 2026
@alice-i-cecile alice-i-cecile added this to the 0.19 milestone May 27, 2026
Comment thread crates/bevy_scene/src/lib.rs
@laundmo laundmo force-pushed the bsn-better-docs branch from 6c84495 to afba143 Compare May 28, 2026 12:54
@laundmo
Copy link
Copy Markdown
Member Author

laundmo commented May 29, 2026

Note to self: Document #24469 done

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation C-Code-Quality A section of code that is hard to understand or change A-Scenes Composing and serializing ECS objects labels May 31, 2026
@alice-i-cecile alice-i-cecile self-requested a review May 31, 2026 16:37
@alice-i-cecile alice-i-cecile added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label May 31, 2026
@alice-i-cecile alice-i-cecile requested a review from cart May 31, 2026 16:37
@laundmo laundmo marked this pull request as ready for review June 1, 2026 17:59
@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 1, 2026
Comment thread crates/bevy_scene/macros/src/lib.rs Outdated
Comment thread crates/bevy_scene/macros/src/lib.rs Outdated
Comment thread crates/bevy_scene/macros/src/lib.rs Outdated
Comment thread crates/bevy_scene/macros/src/lib.rs Outdated
Comment thread crates/bevy_scene/macros/src/lib.rs Outdated
Comment thread Cargo.toml Outdated
Comment thread crates/bevy_scene/src/lib.rs
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
cart and others added 3 commits June 1, 2026 17:13
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs
Comment thread crates/bevy_scene/src/lib.rs Outdated
//! both would supply a [`FromTemplate`] impl and conflict.
//! You still have access to a default constructor of sorts though: the derive generates a companion
//! `YourTypeTemplate` struct that implements `Default`, so `YourTypeTemplate::default()` serves the same purpose.
//! For this reason, there is a custom "derive" which isn't actually a Trait, called [`VariantDefaults`](bevy_ecs::VariantDefaults)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is very confusing to read; I think we're leaking implementation details here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Perhaps, tho it feels like it needs to, otherwise people will go looking for the VariantDefaults trait which is just not a thing. I've tried re-wording it a bit.

Comment thread crates/bevy_scene/src/lib.rs Outdated
//!
//! For a quick rundown on how to read and write BSN syntax,
//! see the docs for [`bsn!`].
//! We plan on making "conditional scenes" easier to define in future releases.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Issue link would be nice, but I don't think one currently exists?

Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

A fair bit of clean up to do, but a marked improvement.

Moving from "inheritance" to "caching" has clarified a tremendous number of things for me.

Comment thread docs-rs/trait-tags.html
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs Outdated
Comment thread crates/bevy_scene/src/lib.rs
alice-i-cecile and others added 3 commits June 3, 2026 13:48
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Comment thread crates/bevy_scene/src/lib.rs Outdated
@laundmo laundmo requested a review from alice-i-cecile June 6, 2026 16:26
@laundmo
Copy link
Copy Markdown
Member Author

laundmo commented Jun 6, 2026

Note: This PR does not include docs for any of the new features and syntax @cart made PRs for after I un-Draft-ed this.

Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Fixed a failing test in the last commit: I stared at it for quite a bit and came to the conclusion that it was simply asserting the wrong thing. That should be passing CI soon. IMO we should merge this in ASAP, and refine further in follow-up (such as for those new features).

Copy link
Copy Markdown
Contributor

@Trashtalk217 Trashtalk217 left a comment

Choose a reason for hiding this comment

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

Some notes, take what you need.

//! Enums are special-cased to allow for better implicit defaults: [`bsn!`] requires that enums have defaults for all variant arms, not just the type as a whole.
//!
//! When [`bsn!`] encounters a Enum, it will try to get the default value for the variant using static methods like `default_{variant_lower}`.
//! To help with setting up these methods, theres a pseudod-`derive` called [`VariantDefaults`](bevy_ecs::VariantDefaults).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
//! To help with setting up these methods, theres a pseudod-`derive` called [`VariantDefaults`](bevy_ecs::VariantDefaults).
//! To help with setting up these methods, theres a pseudo-`derive` called [`VariantDefaults`](bevy_ecs::VariantDefaults).

//! [`Scene`], so patches from multiple sources merge into a single [`ResolvedScene`].
//!
//! For programmatic patching outside of [`bsn!`], see the [`PatchFromTemplate`] and
//! For programmatic patching outside of [`bsn!`], see the [`PatchFromTemplate`] and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
//! For programmatic patching outside of [`bsn!`], see the [`PatchFromTemplate`] and
//! For programmatic patching outside of [`bsn!`], see the [`PatchFromTemplate`] and

/// | `@MySceneComp { @prop: val }` | Include a [`SceneComponent`] with a `prop` field, passed to this components scene function |
/// | `@MySceneComp { name: val }` | Include a [`SceneComponent`] with a normal field, works the same as it does for normal components |
/// | `@MySceneComp { @prop: val1, name: val2 }` | Include a [`SceneComponent`] with both a `prop` and a field |
/// | `:"scene.bsn"` | <div class="warning">Asset format no yet implemented!</div> Include a cached scene asset file |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// | `:"scene.bsn"` | <div class="warning">Asset format no yet implemented!</div> Include a cached scene asset file |
/// | `:"scene.bsn"` | <div class="warning">Asset format not yet implemented!</div> Include a cached scene asset file |

/// | `on(my_observer)` | Attaches an entity [`observer`] for the [`EntityEvent`] `Ev` to this entity. In this example, using a function |
/// | **Relationships** | |
/// | `Children []` | Spawns each entry as a child of this entity, see **Scene Lists** below for details |
/// | `ChildOf(entity)` | Makes **this** entity a child of `entity`, accepts a [`Entity`] or a `#Name` reference ([`EntityTemplate`]) |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// | `ChildOf(entity)` | Makes **this** entity a child of `entity`, accepts a [`Entity`] or a `#Name` reference ([`EntityTemplate`]) |
/// | `ChildOf(entity)` | Makes **this** entity a child of `entity`, accepts an [`Entity`] or a `#Name` reference ([`EntityTemplate`]) |

/// | `1.1` or `-0.1` or `1.` or `-2.` | Float | Floating point number, common types: `f32`, `f64` |
/// | `true` or `false` | Bool | Boolean, type: [`bool`] |
/// | `"somename"` | String | Text, types: `String` or `&'static str` |
/// | `"mypicture.png"` | Asset path | Asset, when used in a field which expects a [`Handle`] to the matching `Asset` type |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I saw another import that just used "goblin" instead of "goblin.png" which is correct?

/// | `true` or `false` | Bool | Boolean, type: [`bool`] |
/// | `"somename"` | String | Text, types: `String` or `&'static str` |
/// | `"mypicture.png"` | Asset path | Asset, when used in a field which expects a [`Handle`] to the matching `Asset` type |
/// | `some(1)` | Function call | Calls a function with the provided arguments |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// | `some(1)` | Function call | Calls a function with the provided arguments |
/// | `some_function(1)` | Function call | Calls a function with the provided arguments |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because some(1) might be confused with Some.

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

Labels

A-Scenes Composing and serializing ECS objects C-Code-Quality A section of code that is hard to understand or change C-Docs An addition or correction to our documentation S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs needs to be updated after : -> @ migration move bsn!/bsn_list! macro docs to bevy_scene crate re-export location to enable doc-links

4 participants