Next Generation Scenes

[split harness]

I see where your head is at, and I think that is a valid way to frame things. I do still disagree though 🙂
I think people should think about their "scenes" as analagous to "blueprints / schematics". A "scene" is not the final generated product, but the description of what that might be. We already have words for the final generated products (Entities, components, systems), and those are not meaningfully compatible with the "scene" functionality. Likewise ResolvedScene is enough of an implementation detail that I would not give it the Scene name. ScenePatch also isn't a canonical scene object and also isn't user-facing enough to merit the Scene name.

[rare whale]

I don't really care how the separator is spelled (you know my suggestion but I'm not married to it), but I think that using a separator of some kind is vastly better than defining a new kind of bracket.

[split harness]

If you ask somewhere "where is the player scene"? They would point to the player.bsn file, or the fn player() -> impl Scene function

The terminology lines up with how people will think about and use these things in practice

FWIW we have World::spawn_scene / Commands::spawn_scene now, which guarantees an immediate spawn, provided the scene has no dependencies (or the dependencies are already loaded)

[rare whale]

Not sure if I can guarantee that. Consider a switch statement, with different cases: we change the inputs and it will delete case 1 and replace it with case 2. But case 2 might have different dependencies than case 1.

It would be more robust to use the non-immediate version. I'm not worried about the latency, other than I don't want a gap

[split harness]

In my ideal world, that switch statement would be aware of the dependencies of each branch and could surface them to the root of the scene

Of course, that is not how Rusts match works

[rare whale]

I have no idea how you would implement that, though

[split harness]

But for some reactive switch, this is 100% possible

(maybe not currently, but theoretically doable in userspace)

[rare whale]

I mean, in practice the dependencies will almost always be loaded, but we can't guarantee that, and panicking 1% of the time is a deal breaker

[split harness]

For sure. It needs to be predictable / bulletproof

[rare whale]

So instead, my idea is that the list of children contains a poison pill that nukes the old children

(mixing deadly metaphors is deadly)

[split harness]

But I'll note that pausing for an indefinite period of time to wait for a scene to load and resolve at the result of a button click doesn't feel good. I think we should be aiming to build a reactive system that is aware of anything that couldn't be spawned immediately / gives the developer the tools to specify a given context as "wait at the beginning for this dependency before spawning the root" and "wait for this dependency lazily whenever it is first encountered"

And I think in general we should be pushing people toward the former, in the context of scene inheritance

Ideally in such a way that lets us "pre-resolve" the entire scene

[golden notch]

this is going to be the case for graphics eventually anyway as asset streaming implies a kind of eventually consistent model (texture pops on mip level cache misses, etc). it’s better to encourage devs to think of their stuff as being potentially asynchronous and non-consistent in this respect

[rare whale]

The easy thing to do, implementation-wise, is to allow the user to pre-declare the dependencies - basically a way to say "here are some handles, don't display them or do anything with them, but just hold on to them". This puts the extra burden on the developer to embed the handle pre-declarations, and would be onerous especially in the case of libraries that have dependencies (potentially breaking encapsulation). But I can't think of an alternative, because we don't know how far down the dependency tree we'd have to search otherwise.

[split harness]

For sure. However for things like texture dependencies this is exactly the kind of thing that should be configurable by the user (ex: don't spawn this scene until we have loaded these textures). This is essentially "what should contribute to a given loading screen" or alternatively "what will be present when we 'pop in' this asset and how long will we need to wait before the first render"

Which is the space that Scene::register_dependencies intends to fill

If someone wants to guarantee that an Image asset is loaded before spawning their scene, they should be able to express that

[rare whale]

Here's a concrete use case: a modal dialog box containing a text entry field that uses a special font. When we open the dialog, we would like to ensure that the font is loaded before we begin the opening transition animation.

[golden notch]

oh yeah i totally agree because that kind of model is also easier to reason about, but pedagogically there’s always a risk of presenting the simple
model upfront but then hitting a complexity cliff where for a real production app you need to learn how to do the asynchronous thing. i think/hope we can make the performant architecture friendly and approachable so that people only opt for the fully synchronous model when they have good reason to and build up good prod skills as they learn the engine

[split harness]

Yup everything is async by default

[rare whale]

(At this point in the conversation, were we at a big game studio, we'd get a bunch of clever young engineers suggesting the use of machine learning models to ingest the logs of asset loads and use that to automate the pre-loading of dependencies.)

[split harness]

(with the exception of scene inheritance, which cannot be async)

[silk lava]

The ergonomics for the async route needs to be as close as possible to the sync route otherwise new users will likely choose the latter.
Such as with Events and Messages, Messages may be faster or better semantically, but they miss the ergonomics/feature-set of Events so most users default to Events.

[fallow cloak]

I'm a clever young engineer and we should do that 😃

(no)

[rare whale]

The perception of delay is a fascinating topic, and depends a lot on context. A typical drummer can detect differences in beats of about 1/60th of a second, but that is true because they are listening to it as part of a pattern / groove and comparing the timing to nearby beats. Game players can see the difference between 60fps and 120fps, but that isn't an awareness of discrete events - the perception of jitter is as much spatial as it is temporal.

For something like a ui transition, where the event is happening disconnected from any other rhythm or sequence (other than the muscle movement of the user's fingers), I would guess that one could introduce 1 or 2 frames of delay before it starts to feel less snappy. Where things get bad is when you have cascading delays, a reaction triggers an asset load which triggers etc.

[split harness]

I do agree that this is often true. But this is not necessarily one or two frames of delay we're talking about. And I've personally been able to perceive when something like click and hover events skip a frame or two (in Bevy and elsewhere), and it bugs me.

I think building a UI system around anything other than instantaneous same-frame changes would be a mistake

And unbounded X frames of delay is incredibly scary

[queen oak]

Seeing how nice and fast jackdaw is, is really quite incredible

the comparison to unity is striking

[topaz ginkgo]

Thanks mate! If anything, shows what is possible with the current state of bevy

i will say, I’m really looking forward to getting stuck in with BSN for jackdaw

[dark crater]

I think I must have missed it in the pr, but what syntax was decided on?

[snow wedge]

Punted until later from what I understand

[dark crater]

isn't there still going to be ambiguity etc., then?

[split harness]

The ambiguity has been resolved by removing the [] shorthand in favor of Children []

Later, if we find a child / related syntax we like more, we can switch. But I think what we have currently is good enough for now

[copper dragon]

Generally I like this resolution
Explicit is better than implicit in this use case

[rare whale]

Thinking about this some more: I think that even if the ideal solution is to somehow be able to transitively collect dependencies (and I have no idea how you would do this, especially if you allow scenes in closures), I think that the "atomic children swap for asynchronously resolved scenes" is something we should support anyway. I think it's an important use case, and even if you choose not to go that route, other third-party solutions may want it.

@split harness Also, when we do the "real" documentation for BSN, I would prefer something more top-down instead of bottom-up. That is, start by teaching them how to spawn a scene that someone else has provided; then how to compose a scene using prebuilt components; and then, finally, how to define components of their own. The reason is that this keeps the narrative firmly grounded in concrete use cases instead of having to teach them a bunch of abstractions first.

[split harness]

atomic children swap for asynchronously resolved scenes is something we should support anyway.
On this, I do agree

[split harness]

fwiw I'm planning on adding higher level / usage-driven docs to the Scene trait (and everything already points to that trait as the "learn more" spot)

But this stuff definitely needs book content

[rare whale]

Indeed. There are certain domains within Bevy where even a 10-page-long docs.rs page isn't enough. bevy_reflect seems comprehensively documented, because there is a lot of text there; but there are so many use cases and edge cases that it really deserves an entire book.

[split harness]

Thanks to @tawdry owl's evil specialization spell recommendation, Handle now implements both Default and GetTemplate.
(https://github.com/bevyengine/bevy/pull/23413#discussion_r2961341173)

I think in this context, doing this type of evil is worth it

Today (and yesterday) I worked on resolving comments. Still some left, but I'm pretty much down to documentation and tweaks.

[golden notch]

wow this is truly pretty evil lmao i love it

[split harness]

If specialization ever lands (unlikely imo) then we can remove it 🙂

I remember using specialization in pre-release Bevy six years ago on nightly because surely it would land soon

[golden notch]

i will happily accept specialization never shipping in favor of real compile time reflection which feels like it has the potential to cover 90% of the most useful cases for specialization

maybe a bit uglier

[split harness]

Any easy links to what "compile time reflection as specialization" would look like

[golden notch]

the goals document talks about it a little

but think basically of it as being less open, instead of the compiler selecting the right specialized trait impl for a method call, the library author can just do a bunch of manual (imaginary reflection api):

fn foo<T>(t: T) {
    if const { T == SpecialFastType } {
        t.fast_impl();
    } else if const { T: SomeTrait } {
        t.slow_trait_impl();
    } else { const_panic!("we don't know how to use your thing") }

so it's more tedious for the library author and not open but still allows you to present nicer apis

[split harness]

Gotcha yeah that makes sense

[frosty shell]

there's also try_as_dyn which can be used to accomplish the same thing in a similar way https://github.com/rust-lang/rust/issues/144361#issuecomment-3752705925

with an example provided by our local bushrat

[golden notch]

oh yeah cool, seems a bit more magical to get the same codegen but very similar. nice !

[elder grove]

Woah this is cool

[dark crater]

ah, nice --- thank you!

[split harness]

Implementing GetTemplate for SceneRoot gives us nice usability wins while we wait for the gltf BSN loader:

fn arena() -> impl Scene {
    bsn! {
        Arena
        Children [
            SceneRoot("arena.glb#Scene0")
        ]
    }
}

Ultimately I'd like this to be

fn arena() -> impl Scene {
    bsn! {
        :"arena.glb#Scene0"
        Arena
    }
}

(once we have the gltf BSN loader)

[steel oak]

(I have this working in my dynamic BSN branch) 🙂

[split harness]

SceneRoot templates or gltf inheritance?

Thanks to GetTemplate specialization, I have SceneRoot working on my branch 🙂
(without losing the Default implementation)

[steel oak]

SceneRoot for now. I really want the glTF loader though

Because then I'll be able to implement descendant overrides. I know you don't want them to be post-spawn but that's the easiest thing for me for now

We use ad-hoc descendant overrides of glTFs all over the place and I really want to get them onto a sensible system, even if not perfect

[split harness]

Very on board for something that works now, especially if it’s done on top of the current pieces

[steel oak]

I'm pretty sure it can be evolved into the Right Thing once we have the pieces in place

It's just a template that lets you specify an XPath-alike and a subtemplate. The template's implementation can change over time

[split harness]

Yeah the public interface wont change too much (other than patching the Component instead of the Template)

(which is often the same thing, but not always)

I'm adding Unpin specialization to the GetTemplate derive so we can start deriving GetTemplate on core Bevy types without losing Defaults

[steel oak]

Yeah, I saw that, nice work 🙂

[split harness]

Hehe you haven't seen it being added to GetTemplate yet. Thats a choice I made today

[steel oak]

I just mean what you write in the .bsn file. Right now I'm actually writing yet another ad-hoc system that watches for entities with magical names per artist request to work around the lack of overrides and I hate it

[white lichen]

i dont quite understand what that Unpin trick is doing

[steel oak]

I don't either

I mean, I know what it's intended to do, but I don't know how it works

I can't wait to start migrating my project to dynamic BSN. It's going to fix so many issues it's ridiculous

[fallow cloak]

afaict it’s a hacky negative impl of Unpin that disables the blanket impl of GetTemplate

you could use any trait there but auto traits are nice because they make it opt-out instead of opt-in

[split harness]

Alrighty theres some messiness to discuss. This messiness existed before "GetTemplate unpin specialization", but that does make the situation worse:

• Option<T>, Vec<T>, HashSet<T>, etc all have auto-Template/GetTemplate implementations where Option<T>: Clone + Default. When T does not manually implement T: GetTemplate, we're already golden. Everything works as expected.
• When we want Option<T: GetTemplate> for some manual T: GetTemplate impl (ex: Option<Handle<T>>, Option<Entity>), Option<T> does not behave as desired by default
• We cannot impl<T: GetTemplate> GetTemplate for Option<T> because that conflicts with the blanket impl. And we really want that blanket impl because it means pretty much everything is compatible by default.
• Prior to "GetTemplate unpin specialization", this would fall back to the auto impl: Option<T: Default + Clone>: GetTemplate. This would silently not support the inner template. Ex: for Option<Handle<Image>> the template would also be Option<Handle<Image>>, meaning the template would only accept handle values (ex: no Some("path.png") in bsn!, only Some(handle)). My plan for supporting inner templates was to have manual OptionTemplate / VecTemplate etc, with a #[template(OptionTemplate<T>)]. Obviously we would love for this all to be automatic, but I don't think it fits into the type system unless we give up the blanket impls,which would be quite bad from a UX perspective ... we'd be in a "serde" situation where every type throughout the Rust ecosystem needs Serialize, Deserialize impls.

• With "GetTemplate unpin specialization", this gets weirder. Now when deriving GetTemplate for types that use this specialization (ex: Handle<T> has a manual impl, and I've been playing with adding it automatically to the GetTemplate derive), instead of silently getting Template == Option<Handle<Image>>, we now fail to compile with the inpenetrable error:

#[derive(GetTemplate)]
struct X {
    field: Option<Handle<Image>>,
}

error[E0277]: the size for values of type `[()]` cannot be known at compilation time
   --> examples/scene/bsn.rs:43:10
    |
 43 | #[derive(GetTemplate)]
    |          ^^^^^^^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[()]`
    = help: the following other types implement trait `bevy::prelude::Template`:
              EntityReference
              FnTemplate<F, O>
              HandleTemplate<T>
              OnTemplate<I, E, B, M>
              OptionTemplate<T>
              TemplateTuple<()>
              TemplateTuple<(T0, T1)>
              TemplateTuple<(T0, T1, T2)>
            and 11 others
    = note: required for `bevy::prelude::Handle<bevy::prelude::Image>` to implement `Unpin`

The fix is still:

#[derive(GetTemplate)]
struct X {
    #[template(OptionTemplate<Image>)]
    field: Option<Handle<Image>>,
} 

But we've gone from silently only supporting Option<Handle<Image>> in templates to failing with an error that makes no sense.

Related:
On the topic of improving the ergonomics of #[template(OptionTemplate<Image>)], if we add a second GetTemplate-style trait that doesn't have a blanket impl (for example: BuiltInTemplate), then we could implement that for things like Option<T: GetTemplate>. Then it would just look like this:

#[derive(GetTemplate)]
struct X {
    #[template(built_in)]
    field: Option<Handle<Image>>,
} 

(note that I haven't wired up the #[template(CustomTemplate)] feature yet ... I'll do that before we merge the PR)

Potential names for this concept: template(built_in), template(alternate), template(manual).
I think alternate is simultaneously the most accurate and the least helpful, from the perspective of users.

template(generic) is close, but notably GetTemplate derives (and manual impls) do support generics!

We could also just look for common types like Vec and Option and do this automatically

Although I really don't like using type paths for cases like this

Brittle and magic

[steel oak]

Oh man, I just started wondering whether my animation retargeter could just be a BSN template

That would simplify my work so much

[copper dragon]

Do we have any of the metadata syntax for .bsn designed?
Like versioning, asset bindings/paths, template vs patch, etc?

[steel oak]

Templates are notated with @. That's all I've implemented thus far

[copper dragon]

Its gonna be really fun when someone manages to make a bsn/rust polyglot file

[split harness]

Check out the import versioning here: https://github.com/bevyengine/bevy/discussions/14437

Nothing set in stone. Just an option

[topaz ginkgo]

How does bsn load the components that have been declared? Is it iterative, or can it be parallelised?

[split harness]

The answer depends on what you mean by "load": Scenes are resolved to ResolvedScene, which is a spawnable/reusable scene description, and the ResolvedScene is spawned into the World as an entity. But in general parallelizing scene things is hard, due to the constraints at play.

When resolving scenes, the "scene patches" are applied in order, the order matters, and they can affect arbitrary levels of the hierarchy. Not a very parallelizable problem space. If we know that a given scene patch wont "reach in" to patch a descendant, then we could theoretically parallelize individual entities in the hierarchy.

Spawning anything in Bevy requires &mut World at the moment, which definitely impedes "parallel spawning". We're writing to shared data structures. Theoretically, if we could prepare all of the storage ahead of time (ex: know what Resources / Components / Archetypes / Tables are needed and how many entities in each storage, we could pre-allocate space and write to it in parallel. But the problem here is that scene spawn order is also a logical thing. Each spawn can have hooks/observers, which will benefit from having predictable / consistent execution orders relative to parents / children.

What we can definitely parallelize is resolving scenes that do not depend on each other

[slate pewter]

i'm getting this same error while trying to turn https://bevy.org/examples/3d-rendering/3d-scene/ into bsn! as an exercise.

// old one
fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
) {
    // circular base
    commands.spawn((
        Mesh3d(meshes.add(Circle::new(4.0))),
        MeshMaterial3d(materials.add(Color::WHITE)),
        Transform::from_rotation(Quat::from_rotation_x(-std::f32::consts::FRAC_PI_2)),
    ));
}

// bsn one
fn base_scene(
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>
) -> impl Scene {
    let base_mesh = meshes.add(Circle::new(4.0));
    let base_mat = materials.add(Color::WHITE);

    bsn! {
        Mesh3d(base_mesh)
        MeshMaterial3d(base_mat)
        Transform::from_rotation(Quat::from_rotation_x(-std::f32::consts::FRAC_PI_2))
    }
}```

error[E0277]: the size for values of type [()] cannot be known at compilation time
--> src\main.rs:51:5
|
51 | / bsn! {
52 | | Mesh3d(base_mesh)
53 | | MeshMaterial3d(base_mat)
54 | | Transform::from_rotation(Quat::from_rotation_x(-std::f32::consts::FRAC_PI_2))
55 | | }
| |_____^ doesn't have a size known at compile-time
|
= help: the trait Sized is not implemented for [()]
= help: the following other types implement trait GetTemplate:
bevy::prelude::Entity
bevy::prelude::Handle<T>
= note: required for bevy::prelude::Handle<bevy::prelude::Mesh> to implement Unpin
note: required because it appears within the type bevy::prelude::Mesh3d
--> C:\Users\flfuac.cargo\git\checkouts\bevy-50d7e162b728c6c6\c35e428\crates\bevy_mesh\src\components.rs:98:12
|
98 | pub struct Mesh3d(pub Handle<Mesh>);
| ^^^^^^
= note: required for bevy::prelude::Mesh3d to implement GetTemplate

[split harness]

Gross. Same principle at play as Option<Handle<T>>. Handle "infects" it with its unpin, requiring an explicit GetTemplate impl.

This is common enough of a thing to make me want to rethink these shennanegans

The "solution" is to derive GetTemplate on Mesh3d. But of course thats not something we can expect people to know

Real specialization would solve all of these problems 😭

[thick slate]

Can we use the custom diagnostics to point people in the right direction?

[split harness]

Sadly no dice here.

Just pushed:

• GetTemplate derive now supports #[template(CustomTemplate)], enabling fields to define custom templates
• OptionTemplate: enables support for things like Option<Handle<Image>> via #[template(OptionTemplate<HandleTemplate<Image>>)]
• SceneRoot now derives GetTemplate, enabling bsn! { SceneRoot("level.gltf#Scene0") }

[topaz ginkgo]

this is kind of what i was getting at - however, do we know this at time of creation? presumably we would need a graph/tree of some kind here to work this out

@steel oak had made reference to an editor which edits the bsn - i was curious what your take is, as i'm obviously very interested in exploring this in jackdaw

[steel oak]

For what it's worth I'm aligned with @split harness here. The point of BSN has always been for editors to edit it and not edit components.

The BRP is for editing components.

You can wait for @cart to confirm but it's true 🙂

[topaz ginkgo]

i understand! i'm just curious how that looks and ultimately works in practice, as it's something i had time to sit with as i was looking at other bsn implementations/tracking projects across our ecosystem at the moment

[steel oak]

I don't think anyone has worked on this; it's just too new right now

BSN itself is still undergoing flux

I'm obviously interested but I have a ton of things on my plate

[topaz ginkgo]

yea fair enough, i was thinking a happy medium just for now might be something to do with jackdaw modifying .bsn files, but then as i understand those won't be part of this PR

[split harness]

We had a long conversation about this: #1264881140007702558 message

As it was quite long, here are some milestone markers

#1264881140007702558 message
#1264881140007702558 message

[topaz ginkgo]

awesome stuff, and perfect reading on the commute tomorrow - much appreciated! 🙂

[steel oak]

Actually rereading the conversation I think @split harness is less absolutist than I am about no ECS -> BSN writeback.

Though the disagreement might not have any practical effect.

I think part of my absolutism about no ECS -> BSN is the fact that I work on a the type of project where we use artist-defined prefabs and overrides everywhere. And it really is a totally different model. The #1 thing that's always at the top of mind for me is "how will an artist who works in Maya interact with this system"

[split harness]

Yeah the true takeaway from that conversation should be: start with BSN editing as the source of truth and then expand out into ECS->BSN writeback (perhaps partial writeback) as it makes sense.

[steel oak]

I agree

[split harness]

Just pushed:

• Support for Handle values in HandleTemplate
• GetTemplate derives for Mesh3d and MeshMaterial3d

@slate pewter this makes it possible to spawn the mesh scenes in 3d_scene, although it requires a bit more boilerplate than I like:

let base = meshes.add(Circle::new(4.0));
let material = materials.add(Color::WHITE);

commands.spawn_scene(bsn! {
    Mesh3d({base.clone()})
    MeshMaterial3d<StandardMaterial>({material.clone()})
    Transform::from_rotation(Quat::from_rotation_x(-std::f32::consts::FRAC_PI_2))
});

[steel oak]

Editing meshes and animations in-editor isn't really interesting to me, for example (that doesn't mean I don't think that shouldn't be a supported workflow, mind you). Artists are going to do all that stuff in Maya. My interest is "how do I create a system that allows me, the programmer/tech artist, to cleanly integrate with these standalone glTF scenes that contain art assets only"

[split harness]

The clone() is required due to how the ownership works internally. I suspect we can figure out a way to remove that

The other sad piece of boilerplate isMeshMaterial3d<StandardMaterial>, as type inference breaks down in the context of handle.into()

[steel oak]

That means I think a lot about things like "what if the artist modifies the mesh in Maya and gives me a new FBX file, can I just throw that FBX file into my "bevy-ify" script and keep all the code intact, or did the editor force me to do destructive operations"

[split harness]

Actually maybe not resolvable. Not the first time we've discussed this. The issue here is that base is owned by the generated patch closure , which is repeatable. Meaning each invocation of the patch needs a new owned instance.

This is the relevant generated bsn!

    let base = meshes.add(Circle::new(4.0));
    <MeshMaterial3d<StandardMaterial> as bevy_scene2::PatchGetTemplate>::patch(
        move |value, _context| {
            value.0 = (material.clone()).into();
        },
    ),

(edit: missed a .clone())

[split harness]

Yeah non-destructive BSN patches on top of in-memory Bevy Scenes loaded from FBX/gltf is definitely the intended scene workflow

Not to say users can't do a FBX -> BSN conversion and do direct edits on top. But I personally think that workflow is crazy. People ask for it / use it though in other engines

[steel oak]

Another thing is animation retargeting which is a whole 'nother kettle of fish I haven't even fully thought about

Well, we have a fully functional animation retargeting system (mostly OSS too as bevy_animation_controllers, albeit undocumented)

Some amount of it may be able to be converted into Templates which would be wonderful

But the needs are vast. For example: just yesterday I had an artist give me an animation that didn't match up to the rig because the bones were named things like deformation_grp/Hips in the rig and were prefixed with the character name like Ferris:deformation_grp/Ferris:Hips in the animation

Retargeting systems (ideally a Template of some kind?) need to be able to handle this kind of nonsense. e.g. a template like @RetargetAnimation { animation: "path/to/MyAnimation.gltf#Animation0", strip_prefix: "Ferris:" }

[steel oak]

It might work better for solo projects but not for projects in which the artists have their tools and are set in their ways

[steel oak]

You can see at this point why I'm skeptical of component editors: "import this thing, wire it up to the animation graph, and strip a prefix from all the bones" is such a high-level operation that I can't see how any ECS->BSN writeback could possibly handle it properly. But it's like, the kind of nonsense tech artists deal with every day in the real world.

[rare whale]

@steel oak I am interested in editing of bsn, but possibly in a different way. As I mentioned, my game is a large open world that is a hybrid between manually created and procedurally generated. In my previous implementation, I leaned heavily on JavaScript prototype inheritance via "exemplars" and "aspects": for example, "alpha wolf" inherited properties from "wolf" which inherited from "NPC", and the system would, at runtime, compose together the various aspects that made up that character (the wolf skin, armature, animations, as well as the behaviors: territorial, combatant, canine combat skills). Originally these aspects were defined as JSON objects, but I think it may be possible to translate them into BSN files using BSN's inheritance feature. Since I don't want to spend eternity authoring BSN files in a text editor, I'll need to be able to edit these BSN aspects and compose them together in the game editor.

[steel oak]

I think that's quite compatible 🙂

[steel oak]

I also feel like being able to express your intentions at a high level is important even if for no other reason than dealing with merge conflicts. The low-level text scene format will leak through when dealing with merge conflicts and this is an area where Unity falls down. Apparently Godot 4 has a fairly nice scene format, but even it isn't as flexible as BSN

[slate pewter]

this is super fine thanks for the fix. probably can be revisited in the future with whatever ergonomic clone solution the lang comes up with

[leaden dew]

isn't this possible by changing the negative impl from [()]: Sized to an equally trivial condition using our own trait?

for example:

#[diagnostic::on_unimplemented(message = "Helpful error message here!")]
trait BlahSpecializationError {}
impl<T> Unpin for Blah<T> where for<'dummy> (): BlahSpecializationError {}

error[E0277]: Helpful error message here!
   --> src/spec_test.rs:16:6
    |
 16 |     <Option<Blah<i32>> as Something>::VALUE;
    |      ^^^^^^^^^^^^^^^^^ the trait `BlahSpecializationError` is not implemented for `()`
    |
<rest of the error message is similarly unhelpful as before>

this is also nice because we can provide a different error for each type being specialized to point people in the right direction

[split harness]

That seems very promising. Great idea!

I’ll give it a whirl tomorrow

[split harness]

This doesn't work. Probably needs to be an auto trait (and might need to be Sized specifically)

[split harness]

Presumably you didn't get this working?

[leaden dew]

I got it working but not on anything BSN related so I'm not sure what differences might throw it off

struct Blah<T> {
    data: T,
}
trait Something {
    const VALUE: i32 = 1;
}
//Blanket implementation.
impl<T: Unpin> Something for T {}
//Negative implementation
#[diagnostic::on_unimplemented(message = "Helpful error message here!")]
trait BlahSpecializationError {}
impl<T> Unpin for Blah<T> where for<'dummy> (): BlahSpecializationError {}
// Specialised implementation!
impl<T> Something for Blah<T> {}
fn blah() {
    <Option<Blah<i32>> as Something>::VALUE;
}

this produces the error message I posted above

another version of the same idea I saw online used PhantomPinned: Unpin which is indeed also an auto trait. I don't really understand the evil spell enough to tell whether that's materially relevant though

as far as i can tell the bound just needs to be a trivially false bound, which would normally be a compiler error but the for<'dummy> defers it or something

[split harness]

Ok I understand why yours works and mine doesn't. When defined in a single crate, this all works. When using the trait across crates, it stops working.

[leaden dew]

oogh...

[split harness]

Probably orphan rule-ey

And that explains why Sized works, as nobody can manually implement that

Building on this does probably introduce a degree of risk, as this does feel like a hole that could be closed at any moment.

That plus the arcane-ness of the errors makes me want to pursue other options

Sadly I don't think any option will be as satisfying as "supports both Default and GetTemplate"

[leaden dew]

i can't seem to reproduce the issue on your branch, even across crates 🤔

e.g. this test in bevy_scene2 behaves as expected after the change in bevy_ecs

#[diagnostic::on_unimplemented(message = "This is my super helpful Handle specialization error")]
trait AssetSpecializationError {}
impl<T: Asset> Unpin for Handle<T> where for<'dummy> (): AssetSpecializationError {}

[split harness]

I just pushed a fix for autocomplete of named fields that are impartial / added in the middle of struct:

bsn! {
  Foo {
    bar: 1,
    a // typing here
    baz: 2,
  }
}

Its a case of "the current contents are not valid BSN, which fails parsing, which means Rust Analyzer never has a chance to resolve the final symbols"

The trick is to make the macro successfully parse and pass through the partial a field, despite not having the , separator

It already did this same type of thing for partial fields (that don't have : value), which is what allowed autocomplete to work in the other cases

[tawdry owl]

You can also try making the error trait a supertrait of Sized to make it work across crates

[split harness]

Weird. Can you try it with Handle<T> in bevy_asset?

[split harness]

I suspect this works because you didn't change the GetTemplate macro to use the new Unpin approach?

(constrained on AssetSpecializationError)

[split harness]

I'll give that a try tomorrow

[split harness]

I just generalized this to solve this problem in the case of entity lists:

bsn! {
  Children [
    Node,
    A // typing here
    Node,
  ]
}

This issue is still present for listed entities that are followed by another entity with ():

bsn! {
  Children [
    Node,
    A // typing here
    (Node, Foo),
  ]
}

This is because this is ambiguous with

bsn! {
  Children [
    Node,
    A(Node, Foo),
  ]
}

Perhaps a reason to adopt mandatory#() / #ChildName() syntax for children

[tawdry owl]

Would --- entity separator have this problem?

[split harness]

Yes and no. This is accomplished by a double-parse of an "entity scene" without a separator

That would make it all get parsed as a single entity

Which is actually valid from an autocomplete perspective

But it would have potentially weird interactions with features like#Name

[split harness]

Actually this is slightly wrong.

The middle one should be (Node Foo) (no comma), which is not ambiguous with the A(Node, Foo) tuple

However to know the difference, the parser would need to peek all the way in

And A(Node) is a truly ambiguous case

[topaz ginkgo]

apols for another basic question, but i assume nobody is looking at serializing bsn to a file atm right?

[leaden dew]

ah yes I didn't touch the GetTemplate macro. my assumption was that it'd need a different approach since any specific trait like AssetSpecializationError isn't necessarily in-scope where GetTemplate is being derived

[steel oak]

Yeah I've only implemented parsing, not pretty printing (serializing to a .bsn file)

Pretty printing should be straightforward, but I haven't implemented it yet

[topaz ginkgo]

Already started on it in a branch in jackdaw

[steel oak]

Awesome!!

Please feel free to send PRs to my branch btw

[topaz ginkgo]

Ive actually cherry picked your commit onto my fork of carts branch

Might be best to just work directly from yours tbh

[steel oak]

Once cart's BSN PR lands I'll clean up and PR my branch

I'd really like to get dynamic BSN in soon because that unblocks so much

Probably not for 0.19, but early in the 0.20 cycle

[fervent kindle]

Not trying to be that impatient and demanding guy, but im curious, what is the ETA on the BSN pr?

[split harness]

Ideally in the next couple of days

[split harness]

Just pushed this change

[split harness]

PhantomData<T> is an especially annoying instance of a "cannot be unpinned" error

[lime veldt]

do you keep trying to make the pin specialization hack work?

it sounded like there is no way

[tawdry owl]

What makes PhantomData worse than other cases?

[split harness]

It works, it just has painful quirks that appear to have no mitigation

As soon as I wrap up everything else, I'll make a final call. But I'm leaning toward removing it in favor of something else

Which will likely mean that Handle will be harder to consume from templates (for as long as we keep the Default impl around)

[split harness]

Its prevalence

[tawdry owl]

If it's about the error message, constraining on a Sized supertrait with on_unimplemented was working when I tried it. Aside from that, Autoderef specialization can also probably be used instead of unpin hack, but that has its own quirks.

[dark crater]

how do you mean?

[split harness]

I think currently its fine, but if we (for example) disallowed duplicate #Name declarations for a single entity, this would break:

bsn! {
    Children [
        #Child1
        A
        ---
        // I want to add a #Child2 here
        #Child3
        B
    ]
}

This would parse as two children, where the second child has both #Child2 and #Child3

bsn! {
    Children [
        #Child1
        A
        ---
        #Child2 // I haven't typed the --- yet so this is read as a single entity
        #Child3
        B
    ]
}

[split harness]

Arg spent like 45 minutes doing a graph toposort to ensure scenes in resolve_scenes (fed by the LoadedWithDependencies events) are resolved in dependency order, only to remember after the fact that these events already fire in dependency order :/

Pretty dumb, especially given that I wrote that code

[split harness]

I just pushed changes that give "queued spawning" the better names (ex: spawn_scene / spawn_scene_list are now "queued" and spawn_scene_immediate / spawn_scene_list_immediate are now "immediate)

The rationale there is that queued spawning works by default / can be the thing people use for everything, so it makes sense to push people toward it with the better name.

I've also made queued spawning be "immediate" when there are no dependencies, meaning the changes can be observed right away. I think this makes it more viable as the "use for everything" method.

I resolved a couple of other comments. So everything is resolved except @white lichen's suggestion that we do the bevy_scene rename before merging (in a separate PR)

Kicking that off now

Still need to make a final call on Default specialization

But we're very close to being merge ready!

We could consider deferring Default specialization alternatives as a followup, given that this does work as-is

[rare whale]

The acceptance criteria should not be "is it featureful enough" but rather "is the next step a logical progression" - meaning that early adopters that jump on v0.1 won't have to undo things when they go to v0.2.

[split harness]

Removing the specialization hack would mean that people deriving both Default and GetTemplate would need to choose one or the other

So its pain they'd need to eat, but it is a logical progression (in that the specialization hack is extremely hairy)

Just pushed a merge with main

[raw mesa]

Personally I disagree with this. IMO users should prefer the immediate APIs by default, and use the queued (aka deferred) APIs, and this renaming inverts that dynamic. I can also imagine users being surprised if they have a bsn that works and then they add an asset inheritance, and suddenly they don't see the effects of their bsn immediately

Users often are surprised when SceneRoot doesn't spawn the scene immediately, because the API we give them sounds like it should be immediate, when it's actually deferred

I was a big fan of the "queue" naming for that exact reason - it makes it very obvious this API is deferred and you shouldn't rely on it being ready immediately

[silk lava]

I dont like the change in behavior based on dependencies, an users code could work when expecting it to be immediate from the first iteration, but on the second iteration after changing the asset it becomes deferred and the user becomes confused, because now they need to use the deferred pipeline/path.

[topaz ginkgo]

hey @split harness @steel oak - ive been doing some preliminary testing of editing bsn in jackdaw, rather than the components themselves. one thing i can’t quite get my head around is that in order to actually see the changes to the bsn in the editor, i have to actually sync the bsn AST to the ECS anyway. this seems like an additional unnecessary hop - do you think im approaching this in the right way?

side note, i also have a preliminary .bsn file format which is a “snapshot” of the world on a user save

[white lichen]

@split harness can we have the inline rust expressions generated by {} be auto-wrapped in a { } so that

bsn! {
    {
        #[cfg(feature = "pineapple")]
        bsn!{ Pineapple { size:  MASSIVE } }
        #[cfg(not(feature = "pineapple"))]
        ()
    }
}

can become

bsn! {
    {
        #[cfg(feature = "pineapple")]
        bsn!{ Pineapple { size:  MASSIVE } }
    }
}

since let x: () = {}; so the empty case wouldn't need a gated ()?

[split harness]

Hmm yeah these are fair points. I do agree that the old naming and behavior was more predictable. I think the big thing that the new behavior gets us is that it insulates people from changes (ex: if a scene you call like bsn!{ scene() } didn't have a dependency, and then adds one, then suddenly your spawn starts to fail). I liked that there was one method we could recommend that just always works.

But the new behavior has its own (harder to understand) breaking quirks in that area (if you're relying on the immediate behavior). I'm reasonably convinced that the old approach was better

[split harness]

Yeah doing partial updates definitely requires building a mapping. My plan (and I believe @steel oak's plan as well) was to increase our "incrementality" over time. Start with a full wipeout and respawn, then scope it to entities, then perhaps scope it to individual components.

side note, i also have a preliminary .bsn file format which is a “snapshot” of the world on a user save
Exciting 🙂

The benefit of starting with the decoupling is that we get the full expressive power of scenes / templates. I think the jury is still out on whether or not the final BSN editor should have a principled 1:1 mapping between templates and components

(at the cost of some expressiveness)

You are welcome to explore both avenues

[split harness]

To be clear I do know that full wipeout and respawn is non-viable / ridiculous for something like moving a Transform

[split harness]

Related: I'm actually still back and forth on whether or not the Template<Output: Bundle> constraint should be changed to Template<Output: Component>, as it would make the scene editing side much easier to do, at the cost of some template expressiveness

[rare whale]

I have a lot of these:

impl<
    Value: PartialEq + Send + Sync + 'static,
    SelectorFn: Lens<Value> + Clone + Send + Sync + 'static,
> Template for Switch<Value, SelectorFn>
{
    type Output = ();

[split harness]

Yeah I'm aware / thats the primary category to sort out before such a change. I believe most of those are dual Scene / Template impls. Might be a way to reconcile that

[rare whale]

Yeah the dual impls are annoying

If Output had to be a component, then entities could only have a single effect

[split harness]

Ex: perhaps we add something that isn't a Template to ResolvedScene, which Scene can populate

[rare whale]

This would mean my entire approach would have to be abandoned

[split harness]

Ex: "effects"

[split harness]

Yeah just to be clear we won't leave you high and dry

[rare whale]

Yeah, some other kind of animal but in template position makes sense

For the record, Output = () is the only thing I ever do

[split harness]

I believe you and others have asked for something like that in the past and I've shot it down out of principle 🙂

[rare whale]

I'm not doing anything more exotic with Output

[rare whale]

I know I have been pushing the envelope 🙂

[split harness]

Yeah I'm very familiar with your approach (although I haven't reviewed your most recent changes)

[rare whale]

In a way this kind of resembles the canonical pure-functional approach e.g. monoids: you have values that represent outputs, and values that represent side-effects

If you squint hard enough

[white lichen]

bumping this

[rare whale]

The approach I have been taking is to make the incrementality visible and explicit; that is, there are different strategies (wipeout and replace vs. mutate in place), so I present to the user an API where they choose, in a component-declarative, fine-grained fashion, which strategy they want. That's not to say that this is the best strategy, but it's one that is easily implementable as a separate layer on top of bsn

[split harness]

That seems reasonable. Only downside I can think of is that the generated code will be less legible when inspected

Which is not a major concern in the context of macros

When compared to functionality

[rare whale]

I suppose you could surround it with braces only if you detect an annotation?

[split harness]

Hehe I was just typing up a similar proposal: insert () when {} is empty

[split harness]

Actually wrapping the contents in {} generally is a good idea. RIght now doing arbitrary "scope things" in {} is broken

This is intended to work

Currently requires {{ }} to work

I've started doing the bevy_scene -> bevy_ecs_serialization rename. I think the big open question is what to do with Scene and Handle<Scene> which are both still used for glTF loading

(obviously they need to stick around, but what do we call them)

Scene is just Scene(World), and for the rest of bevy_ecs_serialization my plan was to just work with World directly

[white lichen]

Is it currently legible at all?

[split harness]

But we can't reasonably do Handle<World> for this. And Handle<Scene> / SceneInstance are reasonably high traffic types

[white lichen]

yeah this makes sense

[split harness]

Legible-ish:

bsn! {
    Node {
        width: Val::Percent(100.0),
        height: Val::Percent(100.0),
    }
    Children [
        button("Ok"),
        button("Cancel"),
    ]
}

Becomes:

bevy_scene2::SceneScope((
    <Node as bevy_scene2::PatchFromTemplate>::patch(move |value, _context| {
        value.width = Val::Percent(100.0);
        value.height = Val::Percent(100.0);
    }),
    bevy_scene2::RelatedScenes::<
        <Children as bevy_ecs::relationship::RelationshipTarget>::Relationship,
        _,
    >::new((
        (bevy_scene2::EntityScene((button("Ok"),))),
        (bevy_scene2::EntityScene((button("Cancel"),))),
    )),
))

[rare whale]

Rename Scene to SceneGraph? It still has an overlap in naming, but at least they are no longer identical names

[split harness]

I'd prefer to drop "scene" terminology entirely if possible

[rare whale]

Can you provide some prose that describes the concept (in your understanding) and let me chew on the thesaurus?

[split harness]

The migration would definitely feel better if we had a glTF loader for the new scene system

[split harness]

Scene is a deserialized World asset, which we then clone underneath an entity containing the SceneRoot(Handle<Scene>) component

[magic belfry]

not sure if I'd call this readable

[split harness]

It certainly requires some effort to decode

[buoyant venture]

That screams generated code and not something a human would ever write

[rare whale]

How about just WorldAsset then?

[white lichen]

whats with all the < as Patch> stuff

[rare whale]

Boring but straightforward

[raw mesa]

It's extra awkward because gltf calls them scenes internally, so giving them a different name seems sad

[split harness]

I'll note that the traits were designed to be a bit friendlier than that to author in person.
This generated code:

 <Node as bevy_scene2::PatchFromTemplate>::patch(move |value, _context| {
  value.width = Val::Percent(100.0);
  value.height = Val::Percent(100.0);
})

Can be written this way when PatchFromTemplate is in scope:

 Node::patch(move |value, _context| {
  value.width = Val::Percent(100.0);
  value.height = Val::Percent(100.0);
})

But we can't generate code that way, as it would require the developer to import PatchFromTemplate, and it could conflict with other patch functions in scope

[split harness]

^ its there for this reason

[thick slate]

Can we kick these into bevy_gltf?

[magic belfry]

also have we done anything about requiring {} everywhere for trivial cases like Mesh3d({meshes.add(cube)}) and {Transform::from_translation(v).with_rotation(q)} and if not, is that planned for the mvp

[split harness]

I agree

[thick slate]

What about GltfScene?

[white lichen]

could the macro generate code like

{
    use ::bevy::blah::PatchStuff;
    Node::patch(//etc
    );
}

[split harness]

Not yet. The former is particularly challenging because patches must be repeatable, and patches are implemented by capturing the input value, meaning it must be cloned to be repeatable

[split harness]

Yeah I think it could!

[raw mesa]

I actually like this name decent enough

[split harness]

I'd say no, as there might be non-gltf 3rd party consumers

[split harness]

This is reasonable I think

That leaves SceneRoot

[rare whale]

ECSRoot?

[split harness]

I don't love it. Too general. Every root entity is an "ecs root"

[rare whale]

FragmentRoot

Something implying a subtree

[split harness]

All meaning is lost i think

[white lichen]

Prefab?

[magic belfry]

yeah fwiw I was playing with bsn! a few days ago trying it with Avian's examples, and my experience was pretty bad for general usage, maybe for UI it's nice with all the benefits it provides for that sort of use case (templates, inheritance...)

but the {} spam, constantly breaking autocomplete, and lack of formatting felt pretty bad

[silk lava]

SpawnAt maybe with a generic marker SpawnAt<WorldAsset>

[magic belfry]

the lack of formatting is my biggest pain point and a dealbreaker for me honestly

[split harness]

Introduces a new (desirable) concept name for something we intend to remove shortly, and could be confusing given that we're introducing a new scene system while also introducing the new name

[white lichen]

DeprecatedSceneRoot

lol

[split harness]

Yeah I do feel your pain / theres a reason I haven't started porting all of our examples. There are clearly missing pieces from a UX perspective

[white lichen]

actually its not deprecated yet

[magic belfry]

I know a formatter extension is planned, but before that happens I don't feel like I can use bsn! happily or productively :P

[verbal heron]

SoonToBeDeprecatedSceneRoot :D

[white lichen]

EarlyAccessDeprecatedExperimentalLegacySceneRoot

[raw mesa]

I mean the current Scene and SceneRoot still have a place

Even after bsn

[split harness]

Perhaps this is all pointing to an "experimental" BSN release, where we leave bevy_scene as-is for now

[white lichen]

if Scene is becoming WorldAsset cant SceneRoot just be WorldAssetRoot

[white lichen]

can bsn be an experimental module in bevy_scene? or must it be its own crate

[split harness]

The bevy_scene rename starts the "push" to bevy_scene2, and I do think another release to get usability in order would help (glTF scene ports, formatter, reasonable asset instantiation story, etc)

[split harness]

I'd prefer a separate crate, just from an organizational perspective. Having a separate prelude also makes it easier to prevent them from stepping on each other

[split harness]

The bevy_feathers approach seems emulate-able

bevy_scene2 crate + bevy_scene2_experimental feature

[split harness]

Yeah this would work. Its just a bit unwieldy for a reasonably high-traffic type

[rare whale]

It's not that bad

[split harness]

@white lichen given that I think the bevy_ecs_serialization rename vs experimental designation are coupled, I think it might make sense to merge it in its current form first, then make a call on the final packaging

[white lichen]

sure

i just would like to avoid a PR that moves bevy_scene to something else and also moves something else to bevy_scene

[split harness]

Makes sense. If it comes to that a two-PR process seems reasonable

[rare whale]

There's a good git argument for this: it's better for the break in the commit history to be explicit, rather than git thinking that suddenly cargo.toml completely changed one day

[split harness]

Cool in that case I think there are just three issues to sort out before merging:

• Still need to make a final call on Default specialization
• Always surround {} blocks in {}
• Initial release notes stub (I would very much prefer to flesh this out during the RC cycle)

[rare whale]

Oh, I don't know, can we really trust cart to write clear, comprehensive release notes as part of the release process? 🙂

[white lichen]

release notes can wait for a separate PR imo, i wouldnt block on even a stub. its not like you're gonna forget

i think the PR should get at least one approval from someone though

@thick slate do you feel comfy approving? i saw your reviews were probably the deepest

[topaz ginkgo]

Great! I’ve been playing with Patch updates, because the concept of a delta just makes more sense to me - ie, when I update the Translation.x value in jackdaw, i would expect just that to update (and not a full Transform update)

[split harness]

I believe once we have the ability to define assets directly in BSN the cost:benefit for most uses cases will flip. No longer needing to deal with system parameters / pass things around will be big.

[thick slate]

Yes, can do tomorrow

[topaz ginkgo]

I will say though it’s been tough to get my head around actually modifying the in-memory representation of the bsn, rather than the Components themselves

[thick slate]

@split harness bother me when you think it's ready

[topaz ginkgo]

sneak peak at the .bsn file format i'm working on with the jackdaw's Brush geometry

[thick slate]

Do we plan to support import-style tpye aliases to reduce duplication?

[topaz ginkgo]

This is just a raw scene dump atm, so in theory anything can be done here 🙂

[split harness]

My spec in the "next gen scenes" discussion includes it. I believe it is necessary, both for legibility and versioned imports (enabling migrations)

[thick slate]

Very sensible. This should also have a measurable impact on file size

[split harness]

@white lichen I attempted a quick toy bsn! macro_rules implementation just to see what the tradeoffs would be in practice. I got as far as "multiple struct-style" struct and enum patches, and lists of ONE related entity.

bsn_rules! (
    Foo::Bar { thing: 10, y: 10 }
    Node { width: Val::Px(10.0) }
    Children [
        Node { width: Val::Px(10. ) }
    ]
)

Source:

macro_rules! bsn_rules {
    () => {()};
    ($type_name:ident$(::$variant:ident)? { $($field:ident$(: $value:expr)?),* $(,)? } $($tokens:tt)*) => {
        (<$type_name as PatchFromTemplate>::patch(|value, _context| {
            if let PathResolveHelper::<<$type_name as FromTemplate>::Template>$(::$variant)? { $($field,)*  ..} = value {
                $(*$field$(= $value)?;)*
            }
        }),
        bsn_rules!($($tokens)*))
    };
    ($type_name:ident$(::$variant:ident)? [ $($child:tt)* ] $($tokens:tt)*) => {
        bevy::scene2::RelatedScenes::<
            <$type_name as bevy::ecs::relationship::RelationshipTarget>::Relationship,
            _
        >::new((
            bevy::scene2::EntityScene(bsn_rules!($($child)*))
        ))
    };
}

Things start to break down:

1. When trying to have multiple children, as $($child::tt)* is required to capture all of a single child's tokens. Trying to wrap that in a comma-separated list doesn't work, as commas are also tokens.

2. Tuple-style inputs, which requiring defining identifiers for the matched tuple indices. I believe this is possible token munchers.

My takeaways:

1. Autocomplete isn't magically fixed. It requires solving the same problems we solve in the the proc macro.
2. It does not help the formatting situation. Rustfmt still breaks on BSN syntax because it isn't valid rust.

Plus theres the whole "#Name in value position" almost certainly being impossible

If anyone wants to explore further they can. But I'm convinced that a macro_rules! port isn't going to solve any of the hard problems for us, and it creates new hard problems to deal with

I do like how concise it is though!

And I think the "use irrefutable let patterns to share code between struct patches and enum patches" might help simplify things. I believe it also enables us to not need to rely on naming conventions to extract "enum-ness"

as struct vs enum will use the same code.

I was planning on exploring "destructuring structs in patches" anyway, as I believe that would solve the "suggesting functions when typing fields" problem

[split harness]

As evidenced by this not being a problem currently in bsn!{} for struct-style enum fields

And as evidenced by this not being a problem in the bsn_rules! impl, when irrefutable let is used

[rare whale]

This line of research seems highly crowd-sourceable / delegatable

[split harness]

Agreed. I'm also aware that this isn't the highest priority atm. This was more of a brain treat for me / an itch I wanted to scratch

(but I'm dropping it now to wrap things up)

[dark crater]

in that case, if you kept the old naming, would it make sense to keep the queued method spawning stuff without delsimmediately, or could there be an advantage to queuing things even if not strictly necessary?

[dark crater]

for the formatting extension, does rustfmt have some way to support extensions like that that was planned to be used? Otherwise, was there a plan for how it could be possible to get formatting?

[topaz ginkgo]

so i've got a very basic implementation of writing the .bsn to a file, and then loading that as a scene in jackdaw - there are still some issues with selection highlights and we've lost some of the Brush Groups on save (but these are just serialization issues)

i don't think the grammar supports spaces @steel oak : if i write something like #Brush Group, it just parses it as #Brush (which is actually a different entity type in jackdaw)

[white lichen]

really appreciate you trying thanks 💚

[topaz ginkgo]

much better! we can no save/load .bsn and compose scenes as well! i need to work on bringing down the filesize next, so will implement the import-type aliasing 🙂

[steel oak]

Good question, @split harness how are you supposed to write a #Name with spaces in it?

[split harness]

I know the Dioxus folks have done this for their rsx macro via a RA rustfmt command override, which calls their formatter, which orchestrates rustfmt

[split harness]

Currently unsupported. Also unidiomatic, as names are PascalCase.

But the plan is to support arbitrary input strings via #{name} syntax

[steel oak]

@topaz ginkgo straight from the horse's mouth, it's #{name}

[topaz ginkgo]

Ok I’ll have to change my PR on your dynamic bsn branch

I changed the grammar to basically parse until end of line

[steel oak]

yeah, it comes back to artists as always

[split harness]

We could probably support #”name” too

[steel oak]

you name a Unicode code point, some artist somewhere has put it in a Maya file 😆

and then when you ask "how did that get there" it's like "idk"

[split harness]

Rather than pretend the file format supports arbitrary expressions

[white lichen]

someone's putting the horse before the cart for once

[topaz ginkgo]

My first implementation was to support #”Brush Group” but that felt a bit too hacky to me

[steel oak]

well done

[topaz ginkgo]

Do you have reservations with something like #Brush and #Brush Group - or do you prefer #Brush and #{Brush Group}

My preference is just simply #Brush and #Brush Group for Name

[steel oak]

I defer to @split harness

keep in mind that we want it to be compatible with the macro parsing in rust

and I'm not sure that rust macro parsing lets you match on newlines like that

[white lichen]

does the space have to be there?

[steel oak]

otherwise I wouldn't have a problem with it

[steel oak]

artists are going to name things with spaces in them, there needs to be a way to serialize it

[topaz ginkgo]

Yes 100%

[white lichen]

Name("Brush Group")

why do we have a special cased way to do it that isnt general and then special case the special case lol

[steel oak]

one of the goals is to make it so you can just copy and paste BSN into your .rs file

[white lichen]

just use the general thing lol

[steel oak]

it's a neat feature

but that means .bsn syntax has to be a rust token tree

that's how I explain BSN to unity devs anyway, "it's a readable scene format so you can edit merge conflicts, and also you can just copy and paste it into your rust files to spawn the objects if you don't want to go to the trouble of making a scene file"

[topaz ginkgo]

Ok this makes sense that it should be something like #“Brush Group” i think if # is just a keyword for Name

[steel oak]

yeah

[topaz ginkgo]

I think we should be consistent here though, so #”Brush” and #”Brush Group” should always be resolved

[steel oak]

incidentally, I don't think this is the way most longtime Bevy users think of BSN, but when talking to most game devs taking an editor centric view is the easiest way I find to explain it

if you say "you can take your scene files and copy and paste them into your code and it just works" they get it

[topaz ginkgo]

I will say, i haven’t thought of BSN like this until now

This makes sense why we want the editor to modify BSN directly

[steel oak]

yeah, it's like, in Unity the "manual" way to construct GameObjects is with Instantiate(), AddComponent<Foo>(), etc. which is different from the way they're specified in the editor. Bevy just says, what if they were the same

I mean, you can construct them "manually" in Bevy too, but it does have the inline bsn! { ... } feature which I think is cool

[topaz ginkgo]

Yes i can see why this might be cumbersome

FYI i have a working implementation of modifying the in-memory BSN in jackdaw, rather than Component-based. I’ll commit and push my branch shortly (there are other changes/refactors as well, so it’s kind of polluted)

But hopefully once carts bsn PR lands, and your dynamic bsn PR i can just pin jackdaw to mainline bevy

[topaz ginkgo]

updated the PR to support #"Brush Group" in bsn grammar: https://github.com/pcwalton/bevy/pull/16

[topaz ginkgo]

also @split harness has there been discussions about shared assets in bsn? sorry if i'm out of the loop on this. JSN currently supports this, and am happy to port to BSN. we have a separate "assets" section in JSN where all the shared assets of entities have access to

[topaz ginkgo]

ok apologies for the spam, i'll shut up - but just to be clear, i have the following 3 changes in flight to get jackdaw fully onto bsn:

• Finish off DynamicBsnLoader impl for List and Tuple support on @steel oak's dynamic-bsn branch
• World -> bsn scene writer (DynamicBsnWriter) - bsn-scene-writer branched off dynamic-bsn
• Asset catalog support (BsnAssetCatalogLoader) - bsn-asset-catalog branch off dynamic-bsn

i've done it this way so i can have individual PRs raised as well - this should then get us in a good state in jackdaw to be fully reading/writing BSN.

i should highlight locally the world -> bsn serializer, and asset catalog is encompassed within jackdaw while i test, but i'm extracting these out to be upstreamed

[split harness]

#Brush Group is ambiguous with

#Brush
Group

In a context where whitespace isn't semantically relevant (and it cannot be for BSN)

I personally don't like #{Brush Group} either, as we'll want that syntax to pass in a #{name? variable

So actually neither of those are options 🙂

I think it either needs to be #"Brush Group" or #{"Brush Group"}

[topaz ginkgo]

Yes, agreed ive gone for #”Brush Group” as that kind of tells me syntactically this is a custom string Name

[lime veldt]

next up #"\"foo\""

[split harness]

#Name isn't just about sugar for Name("Brush Group"). It can also be used in value position, and I see no reason why scene assets shouldn't support that too. I do think it gets harder to defend without the "name as entity variable" functionality.

Name("Brush Group") is "just" the component value / does not have access to the #Name functionality

[topaz ginkgo]

yes this makes sense! i just use it for writing the scene, as we use the Name component for filtering in jackdaw

[split harness]

Are you saying that #MyName does not become Name("MyName") in your impl?

[topaz ginkgo]

no that's exactly how it works!

i wanted to pick your brain on an "asset catalogue" @split harness - kind of similar to unreal's .uasset. in JD currently we have a shared assets file (in JSN) that allows other scenes to reference without instantiating each time (so a load asset once, and reuse). I've made a similar implementation for .bsn in my bevy fork but requires some changes to the LoadContext in bevy_asset such that we will automatically generate an assets.bsn which looks like this:

Children [
  #ground06
  StandardMaterial { perceptual_roughness: 0.8 },
  #rock058
  StandardMaterial { metallic: 0.1 }
]

which we can then access by reference in any scene.bsn:

Brush { faces: [ BrushFaceData { material: "assets.bsn#ground06" } ] }

so in essence, the changes i've made are scene loads bsn -> asset_server loads the assets.bsn -> our DynamicBsnLoader parses it and registers it as a sub-asset via reflect -> and the Handle resolves automatically through the labeled assets system.

curious if you think this would be a good start, as this will almost immediately allow jackdaw to save/load textures and materials with bsn?

[slender lion]

from a "what happens today" perspective, I would expect this to cause pain around reloading the root asset repeatedly, since loading a sub-asset loads the full root, then drops anything you aren't using, and a second sub-asset access in another spawn will cause the whole root asset to load again. This also triggers things like all of the other sub-assets "refreshing".

[split harness]

I tried the supertrait approach:

#[diagnostic::on_unimplemented(message = "Helpful message here", label = "label", note = "note")]
pub trait SpecializeTemplate: Sized {}

impl<T: Sized> SpecializeTemplate for T {}

impl<T: Asset> Unpin for Handle<T> where for<'dummy> [()]: SpecializeTemplate {}

(and in the FromTemplate derive)

This does still work from a "specialization hack" perspective. But I'm not getting improved diagnostics:

#[derive(Component, FromTemplate)]
struct Foo {
    image: Option<Handle<Image>>,
}

"Helpful message here" is never printed anywhere, and the same arcane the size for values of type [()] cannot be known at compilation time message is printed

[tawdry owl]

I tried this without the impl<T: Sized> SpecializeTemplate for T {}

[split harness]

Is StandardMaterial a component here? Because if not, and it does a runtime "is this an asset type" check, I think this is too hacky (and cannot be supported in the bsn! macro).

My current plan for this category of thing is:

1. Land Assets as Entities
2. Add the concept of a "shared entity" to BSN, which is spawned once and shared between instances
3. Add the ability to retrieve handles to shared entities (this probably directly intersects with both (1) and (2).
4. Profit

[fallow cloak]

Do you specifically need the Sized supertrait bound, or to use [()]? I think you could put any dummy struct/trait there as long as the dummy struct doesn't impl the trait

pub struct Dummy;

#[diagnostic::on_unimplemented(message = "Helpful message here", label = "label", note = "note")]
pub trait SpecializeTemplate {}

impl<T: Asset> Unpin for Handle<T> where for<'dummy> Dummy: SpecializeTemplate {}

[split harness]

(2) is also an important piece for things like "observers shared between instances", once "observers as relationships" lands

[split harness]

AAAYYYYY yeah that works

We're back to "maybe the UX here is viable"

[split harness]

I'll give this a go

Leaving only "this is a built on a hack that the rust team might close at any time"

[thick slate]

It's okay they'll hit us in a crater run 🙈

[split harness]

But honestly, if it gets us through a single release cycle we still come out on top. Because by then we'll be able to push more aggressively toward BSN and perhaps have an "always strong handles" story in place

I'm leaning toward doing it

[tawdry owl]

Hopefully try_as_dyn happens before that, but the timelines are never certain with these features

[queen oak]

I had to reinvent the concept of a shared entity for some of my async ui experiments, its very pog, please do add

[topaz ginkgo]

Yes there are some runtime checks for asset types, and I do agree it is not great. I think ultimately, I’ve been able to prove that jackdaw can be a bsn editor first. All of the upcoming improvements to Assets and shared entities also makes this easier of a migration for me - so im happy to sit on these changes, because in my view, we’re basically there already

[split harness]

Of course do whatever works (although I wouldn't upstream that approach)! There is added benefit if you did your exploratory designs in the context of what the "entity sharing / assets-as-entities" API might look like. But theres definitely value in just building something that works

[leaden dew]

this was exactly my suggestion (though with the unit type instead of a dummy struct) but apparently there's some issues getting it to work across crate boundaries

if the supertrait works then problem solved anyway!

[split harness]

Ah right yeah I've already tried this haven't I

I'll try it one more time just in case 🙂

[topaz ginkgo]

I believe there has been work on the assets as entities already, right? I am happy to use it as a fact find for us to see what sticks

[split harness]

Yup! https://github.com/bevyengine/bevy/pull/22939

If you're entering that world / have questions sync up with @raw mesa

[raw mesa]

I'm unsure how nicely something like "spawning assets as bsn" will work tbh. We'd need like a Template that internally stores a Scene, and then we need to provide some way to tell the asset system "give me a handle for this asset entity, I promise this is the only handle". Quite gross

But maybe I'm thinking too small haha

[split harness]

I think its doable in a clean way (ex: built from pieces that are not hard-coded to assets / usable for arbitrary entities), provided the "assets as entities" impl can work by just spawning entities normally. (ex: commands.spawn(Mesh::default())).
I'll put together a spec once 0.19 is out

/ after I review assets-as-entities v0 so I can take that into account

[raw mesa]

I don't think this will ever be the case. IMO handles are not going away, and we need to ensure people aren't doing stupid things with creating this handles. Today we do that by holding a tight grip on how users spawn assets (only through asset APIs)

[split harness]

I haven't yet come to that conclusion. I suspect we'll hash this out in your PR when I review it 🙂

(dont have time now sadly)

[raw mesa]

Yeah bsn takes priority for sure haha

[split harness]

@thick slate I've resolved the outstanding issues / have hopefully just fixed CI. Ready for your final approval!

[thick slate]

Alright well, I guess that's what I'm doing for the rest of the day before I clock out for the weekend lol

[split harness]

I do have plans to more fully specify the BSN featureset in the Scene docs, but I'd like to do that during the RC cycle

[thick slate]

lol CI is red :p

[split harness]

Hopefully fixed

Was hoping I'd get the cargo_features.md updated before you noticed 🙂

[split harness]

Gotta resolve some schedule ambiguities 🙂

Lol I was still lazily spawning scenes in Update instaed of SpawnScene

@thick slate ok now should be greeeeen green

🤞

[thick slate]

@split harness approved with notes on follow-ups. Feel free to merge

Do uh let us know when you're going to do the big renames

So then fix-ups don't get clobbered and we can get them through quickly

[white lichen]

(pr doesnt look approved to me :p)

[topaz ginkgo]

i have an approach which doesn't actually need to check the asset type at runtime in order to be compatible with the bsn! macro, so something like this works:

bsn! {
      Brush {
          faces: [
              BrushFaceData {
                  material: "assets.bsn#ground06"
              }
          ]
      }
  }

https://github.com/jbuehler23/bevy/commit/817e391b6 - here if interested to take a peek

[split harness]

I'm most curious about what that looks like from the other side

referencing "inline" assets defined in BSN is definitely intended to look like that, from the consumption side

[split harness]

@rare whale have you done any feathers BSN porting? I'll pick that up now if not

[split harness]

The Next Generation Scenes PR is merged!
https://github.com/bevyengine/bevy/pull/23413

[slender lion]

congrats

[split harness]

Just wrapped up the Feathers port to BSN
https://github.com/bevyengine/bevy/pull/23536

[slate pewter]

https://tenor.com/view/freedom-gif-14887144685173519659

[summer cove]

I am a very different person from who I used to be when the first ideas for BSN were brought up

[dapper sky]

Such a huge milestone!

[lime veldt]

wait I thought with bsn you save lines 😹

[split harness]

We’ve opted to leave the bundle versions in, so despite it being terser in many cases, it’s a bunch of new lines

[lime veldt]

because impl Bundle -> impl Scene would cause much trouble at calling sites?

when migrating

[forest shuttle]

About formatting support within bsn, is that something that could have its own goal?

[elfin surge]

well the examples folder did become 2.54KB smaller according to github

[lime veldt]

neat, where do you see that?

[elfin surge]

just adding up the sizes before and after the pr

annoying to do, but I was curious

[near warren]

So, Feathers port will probably be in 0.19 and .bsn asset format (probably) in 0.20?

[dapper sky]

That's the intention, and also pcwalton has been working on the bsn asset format for his own needs and idk how much that'll be able to be rolled up into a 3rd party crate while we wait.

[urban galleon]

Not having to switch to BSN straight away would save me weeks of upfront work

I have been spending 2 months migrating away from a framework and hopefully will be done end of next week.

So if I have to do a bsn migration upfront that would be a big blow for me.

[rare whale]

There will be some migration work if you are keeping the old widget functions, the names have changed and you may also need to ensure you have the right default font.

[somber rivet]

Waiting for the memes where it's like

Me reading the initial discussions for BSN:

• A scraggly early 20's man with bags under his eyes

Me using BSN:

• A beautiful woman

[summer cove]

LMAO

[thick slate]

Beware the pipeline

[rare whale]

I'm going to wait until this is merged before I do anything

[topaz ginkgo]

I think we need to iron out the design on this, but i do have a basically serializer

[copper dragon]

Just put bevy on a Linux trans meme should be bout same lol

[steel oak]

I don't have that much time for Bevy over the next few days but I plan to clean up and submit the dynamic BSN (a.k.a., BSN assets, .bsn) PR shortly after that

This will be preliminary, not a complete implementation of .bsn as specced, but should be a compatible subset.

I think it's best to implement it piecemeal as that simplifies review, etc

[rigid adder]

Do we still need the default helper now that we have BSN?
I remember the main motivation was to improve the ergonomics of the struct update syntax, which was heavily used. But with BSN, users no longer need to type it at all.
It always felt awkward to me since RA suggests Default::default() when you type ...
Even in the BSN PR, I saw Cart using ..Default::default() in benchmarks.

[slender lion]

I would at the very least leave it for a cycle while bsn gets first-used, and not take it out of 0.19

[magic belfry]

personally I'm not going to use BSN until the DX isn't as awful (i.e. formatter, more robust autocomplete, less {} spam for just specifying components or values)

so I'd definitely wait

[slender lion]

the lack of autoformatting and autocomplete is definitely going to be rough in the near term

plus the internal APIs haven't "switched over" to bsn yet either, so "component-based" is still going to be used quite a bit

[magic belfry]

I did try migrating some examples in Avian and gave up :p it was just a pain and made things uglier since I don't even benefit from things like inheritance there

except maybe for some UI things, which I don't have that much of

[slender lion]

I'm committed to living with that pain personally (makes sense for pedagogy/videos/etc), but wouldn't expect anyone else to make that choice

[magic belfry]

but not having to pass around the asset server etc. will be nice

[coarse patrol]

"I started college when BSN was started. Since then I've graduated, had 4 kids, and changed jobs 3 times"

[somber rivet]

I have gotten married and had a child since BSN started 🙃

[steel oak]

So at my studio we use glXF everywhere, which is just BSN but worse

We need dynamic BSN to migrate off glXF, but once we do I plan to yeet glXF

[steel oak]

so we'll be an early heavy dogfooder of BSN

[magic belfry]

yeah I'm talking about the macro, not dynamic BSN

[steel oak]

I plan on using the macro too, but we'll see

I'm all in on BSN

[dapper sky]

I have complex UI that needs to be made maintainable, I'm moving onto it the moment the rc drops 😅

[magic belfry]

definitely very happy to see your work on dynamic BSN :)

[viscid cradle]

bsn merging feels unreal at this point

[magic belfry]

has been cool following along and seeing the progress there

[viscid cradle]

kinda like the editor

[dapper sky]

maybe earlier, if text outlines have been merged

[steel oak]

Once dynamic BSN lands we may go from "no editor" to "there is an editor" surprisingly fast

[dapper sky]

yeah it's relatively easy to design build an editor once the thing that's getting edited has been tightly defined.

~ relatively easy ~

[steel oak]

indeed jackdaw has been making tons of progress

[white lichen]

Joe is a beast

[viscid cradle]

accelerationism in bevy

like the ai bros say, we are reaching the bsn singularity soon

or watever they say

[steel oak]

it will take some time for people's workflows to catch up

and there will need to be a lot of experimentation in the brave new BSN world

[dapper sky]

Bevy tagline 2025: "Imagine france in the 1780s"

[tribal yacht]

the france

yes quite

[white lichen]

2025 is a bit outdated

[dapper sky]

historical lead-up

[dapper sky]

I'm not a writer or anything :)

[tribal yacht]

I couldn't tell 😉

[steel oak]

for example, I have no idea how artists and designers are going to start using the editor in practice at my studio

we'll have to figure it out!

[white lichen]

a mwriter at mheart

[tribal yacht]

I only write fiction, seems much easier

[viscid cradle]

the editor will be released in 0.19

[tribal yacht]

that's just a lie, there's a difference

[viscid cradle]

aren't all fictions lie ?

[dapper sky]

if I had fewer responsibilities I'd be making editors for many silly little things like a macromedia flash clone.

[tribal yacht]

for complicated literary analysis reasons, there is a difference

[topaz ginkgo]

dawww

i will say, leaning into bsn has been a great side quest for jackdaw 🙂

i can even serialize physics components via avian into .bsn files pretty seamlessly @magic belfry

[native mesa]

this looks fantastic

[urban galleon]

That's easily in line with previous migrations, but throwing away entierly would make 0.19 the hardest bevy migration ever (even beating bundles to required components!)

[wooden vine]

Question:
I'm trying to make a UI constructor thing for a list-like widget.
I need to take a list of N user widgets and create a structure like so:

A
 -child-> Header
      -child-> Button
 -child-> Container
      -children-> the N widgets
and
A
 -subwidget-> the N widgets

subwidget is another relationship

Bundles afaik cannot express this, how could this be done in BSN?

I can't seem to figure it out

[native mesa]

the #scenes-and-assets channel is potentially now a better place for this

[wooden vine]

oh! I didn't have that on my channel list

I moved there.

[queen oak]

should this working group be closed and maybe a new one opened focusing specifically on the serialized format?
going off of how working groups we're originally supposed to work they were supposed to be explicitly scoped, and we do have a scenes-dev channel

[cobalt stone]

Agree @split harness ^

[buoyant venture]

The PR listed a lot of follow up work. I don't think we are at a point where we can just close this

[rigid adder]

Do you think hot reloading should use the serialized format, or is it something separate and better be handled by subsecond?

[thick slate]

Probably both tbh

[magic belfry]

at what point do we close this?

probably at least BSN assets and fixing the worst DX problems (formatter pleasepleaseplease)

but stuff like reactivity is also listed in the PR but should be its own thing imo

[native mesa]

i don't think we can close this yet

for example
https://dev-docs.bevy.org/bevy/scene2/macro.bsn.html

there's no actual syntax docs in repo that i can find

there's still remaining work to make this something that you can just hand to users.

i think we can keep this open, but scoped as "fixing and finishing the current impl" rather than "extending it"

[split harness]

Yeah BSN isn’t quite ready to replace everything yet. I’d also prefer to wait at least a cycle

[buoyant venture]

When cart decides it's time 😅

[topaz ginkgo]

shouldn’t you be on the slopes?! 🙂

[rare whale]

Next Bevy JAM theme is going to be "cart's skiing trip"

[slender lion]

prepares bevy_skifree

[thick slate]

https://github.com/bevyengine/bevy/pull/23561 😄

[thick slate]

XTREME SPORTS

[tribal yacht]

A winter wonderland would honestly be a good theme

[thick slate]

@split harness, I'm adopting https://github.com/bevyengine/bevy/pull/23536 real quick: trivial cleanup to get CI green and then merging, since we have two approvals

[rare whale]

Thank you

[thick slate]

Currently sorting out some nasty merge conflicts 😅

Pushing shortly

[rare whale]

@thick slate OK, so the issue is that CustomCursor is used by both CursorIcon and EntityCursor, both of which are enums that have defaults. In other words, it's an enum inside of an enum variant, where the outer enum has a default but the inner one does not:

pub enum CursorIcon {
    #[cfg(feature = "custom_cursor")]
    /// Custom cursor image.
    Custom(CustomCursor),
    /// System provided cursor icon.
    System(SystemCursorIcon),
}

Adding a default to CustomCursor would be a hack, since you'd either have to provide a fake image or a fake URL.

[thick slate]

Fake image is not that unreasonable (just use blank), but that doesn't seem to resolve matters. See https://github.com/bevyengine/bevy/pull/23536/changes#r3006556689

Putting this down for now; I don't understand this well enough to try a more involved fix

[rare whale]

Ideally, we should fix this at the level of EntityCursor and not touch CustomCursor, but I don't know if that's possible. We shouldn't need to change CursorIcon since that's a window thing.

[thick slate]

Yeah, agreed with that 🙂

[rare whale]

Note that EntityCursor::default() is never used anywhere, we could remove the Default impl and replace it with a FromTemplate. Don't know if that solves the problem though.

[thick slate]

Proposed fix is up at https://github.com/cart/bevy/pull/52

This fixes the compilation errors, but I have low confidence that this is the right fix

[rare whale]

Unfortunately it's 10000 commits ahead, unreviewable.

[thick slate]

Oh duh, I'm targeting the wrong branch I bet...

[rare whale]

Yeah, master seems wrong

[thick slate]

+40/-3 lol

A little more manageable!

[rare whale]

I think this is a good temporary fix. The right fix, in the long run, is to allow BSN to be more flexible about enum defaults

Maybe add a TODO to that effect

[thick slate]

I'm not sure what you mean here. What do you think the correct behavior should be when someone calls CursorIcon::Custom in bsn!?

[rare whale]

First, I'm not sure that it makes sense to ever use CursorIcon in bsn, unless you are using bsn to spawn your game's window entity, which seems odd. For individual entities, EntityCursor is the right component to use, and it already has a default - the default system cursor. It makes no sense to insert an EntityCursor with a default custom cursor. But bsn insists on having every enum variant be defaultable, which is stricter than Rust's normal rules for defaulting.

That is, when I impl Default on an enum, only the default variant needs to be default-constructable - not every variant. But bsn is apparently more strict than this.

[thick slate]

Right, so you're arguing that this restriction is too harsh in practice, and this should simply be a compile / scene error sometimes

[rare whale]

In any case, I vote for merging what we have as long as CI is happy

[thick slate]

Agree with this in principle. This is a minor nit compared to the much more important work of "make feathers use bsn" and "add more feathers widgets"

But I don't want to be too presumptous with @split harness's branch while he's off skiiing 🙂 The earlier fixes were very uncontroversial (thanks reviewers!)

(if this was my branch I'd be full 🚀)

[steel oak]

Working on rebasing my dynamic BSN branch. What did GetTemplate turn into?

[rare whale]

FromTemplate

[steel oak]

I submitted a draft PR for .bsn assets: https://github.com/bevyengine/bevy/pull/23576

[thick slate]

I've opened a 0.20 milestone just for this 🙂

[topaz ginkgo]

I will also have a couple follow ups that I’ve implemented off this branch such as asset cataloging and scene serialization as well

[tawdry owl]

Is there a reason to have a separate parser instead of reusing existing one? Dynamic bsn should be expressible as a subset of macro bsn, right?

[steel oak]

syn isn't really usable outside the Rust compiler I don't think

and even if it were, you wouldn't want to pull in so many rust compiler guts into your game

Maybe it is. I've just never heard of anyone using syn for anything other than parsing Rust code

[tawdry owl]

It can parse normal strings just fine, it's why proc_macro2 exists in the first place as far as I understand

[steel oak]

But also like, what's the point? The macro produces Rust code, but that's not what you want in the dynamic setting

[tawdry owl]

The main reason to not use syn for parsing is because it's limited to valid rust tokens, but this is not a problem in this case

[tawdry owl]

Intermediate representation can be shared to not have 2 implementations

[steel oak]

You could have a BSN AST shared between the two but I assume that @split harness didn't write it that way for a reason

Maybe that was not a good assumption, in any case cart should weigh in

(no rush of course)

[steel oak]

At the very least perhaps we could share the lexer?

Because I didn't love having to write a lexer in nom

Ok yeah, I think you can use syn as a lexer

Looking at it closer, the only reason we couldn't use BsnEntry is that BsnEntry is a singly linked data structure with no way to refer to intermediate nodes, which is bad for editors which want to maintain bidirectional links between BSN AST nodes and the objects. We would probably need to ECS-ify it if we want to share data structures.

[tawdry owl]

Yeah, that makes sense. I didn't look too much into it, was just curious if it's possible to not have 2 parallel implementations of the same thing

[steel oak]

I'm going to wait until cart weighs in on the AST question before doing more work on dynamic BSN

[split harness]

We haven't finished the serialized asset format, which was a major part of the formation criteria and is a key piece of the puzzle. I think there is still value in keeping the working group open for a bit longer. I don't think we're ready to close out the Goal yet either, for that reason. I think the formatter, new features, UX improvements, etc would not justify keeping the Working Group / Goal open.

• @cobalt stone @magic belfry

But also we should do whatever serves us best.

If anyone feels a particular kind of way let me know

[topaz ginkgo]

once @steel oak has his dynamic bsn PR finished, id like to work out the 2 bits I’ve experimented with in jackdaw - the bsn file serialization, and a shared bsn asset catalogue - so would be worth keeping the working group open

[split harness]

The rationale behind requiring defaults is that it allows "enum variant patching" in the same style as "struct patching" (requiring Default is also how we accomplish struct patching).

If we lift that requirement, it would mean you would need to specify every field on the enum when setting it to that value. We could certainly do that. It would certainly simplify some things.

[split harness]

I think both of these are additive features on top of the baseline featureset. I know the distinction is arbitrary, but there is value in drawing a line somewhere.

[topaz ginkgo]

oh i totally agree with that - i was just mentioning because it might be worth chatting about them on here only

[split harness]

I think it probably makes sense to switch to threads (or perhaps create new goals / working groups)

[topaz ginkgo]

Fine by me!

[rare whale]

Unfortunately, it distorts the design of enums. CustomCursor doesn't implement Default because the default doesn't represent anything meaningful. It's like saying the default for a URL is an empty string, but that's not actually a valid URL.

[lime veldt]

default url is 127.0.0.1

[split harness]

Haha we've talked about this before. You asked essentially the same question and I pushed you in the direction of a separate parser. My rationale was that they have different featuresets (ex: bsn! supports arbitrary Rust code in {} and in some value positions, .bsn uses versioned imports driven by reflection data, etc) . Sharing implementations introduces new error pathways (ex: accidentally pretending to support something). Our error messages will also need to be different for each of these contexts.

On top of that, bsn! is built to be a "permissive" parser to support autocomplete, where .bsn should be stricter in the interest of ensuring everything is well formed.

@tawdry owl

The previous chat: #1264881140007702558 message

@steel oak's point about not shipping syn in binaries is also extremely valid

[topaz ginkgo]

I’ve actually found that enums which don’t implement Default make it a bit more verbose to work with in bsn. I was looking at some of the avian types, and those don’t implement Default, and I’ve had to fall back to reflection-based component editing on those instead of the patching in jackdaw when a user changes a collider

[split harness]

And we could almost certainly make ours faster

[tawdry owl]

Why is shipping syn bad? It's not a big crate by itself, right? I guess it has enough to parse the entirety of rust (which we don't need), but I'm not sure if it's that much of a problem

[rare whale]

In this case, CustomCursor isn't used in BSN directly - it's not a component. EntityCursor is a component, and it does have a default - but one of its variants (the non-default one) contains a CustomCursor.

[split harness]

I haven't done the cost:benefit here. All I can say is that I know that syn would be bigger than whatever we build

[tawdry owl]

well, I guess bsn is simple enough to not make having 2 parsers too much of a pain, but it's still somewhat annoying to have to maintain feature-parity with 2 different frameworks for parsing.

It also means that dynamic bsn isn't strictly a subset of macro bsn, which I thought was the goal, although maybe I'm mistaken here?

[split harness]

I think full / strict copy-paste-ability should be a nongoal for the following reasons:

1. .bsn files will use versioned imports, in the style of something like use bevy{0.19}::prelude::*. bsn! cannot / should not support this as versioning is nonsensical in that context. Supporting it and just dropping the version number seems wrong to me.
2. .bsn will use the reflected type paths, which serve as a canonical name for the type. bsn! is not constrained by this, and based on the current import / crate context, the paths might not line up.

That being said, for .bsn files using short type names with versioned imports, I think it is useful / cool to support copy-pasting everything but the imports

[steel oak]

OK, cool, so I don't have to rewrite anything 🙂

[dusk aurora]

I got bsn fmt working: https://github.com/bevyengine/bevy/pull/23586

still a draft but looks good so far. The only major constraint is comments in the bsn for now.

I think with both of these features (errors, fmt) I would start using bsn personally, so it would be cool if we could get this out in some shape or form

[split harness]

Wow that was fast

[magic belfry]

would love getting these in 0.19 as experimental (or otherwise) to make bsn! actually usable and give people a better first impression

as it stands, I wouldn't use BSN or recommend others to use it much, but with these I'd be way more comfortable using it

[dusk aurora]

I also added tests to test formatted diffs, but that part is honestly not super great imo yet.

It could be much more granular/atomic tests but it catches most things and is close to the example so I stuck to it.
I also played around with more bsn mock data and more tests but ultimately decided to go with more of an integration test route first.

[split harness]

I would like this, but I think we need to be very very careful here, as formatters have the risk of being destructive to peoples' code. A bug here could be quite bad for people

[dusk aurora]

It only prints the replacements if you use stdin, so it's compatible with editors, but you are right this is a concern.

definitely an area that will need reviewing

I can add instructions for vscode and I'm hoping enough people use it that we can find issues like that in time.

[magic belfry]

Yeah fair, though with an experimental status I wouldn't worry too much about that as long as we have appropriate warnings. But yeah should still be careful and test and review it properly

I feel like it's also important to get it in peoples' hands to really test it

[dusk aurora]

This did happen to me earlier already so I made sure to double check for this

[buoyant venture]

Shouldn't something like that be part of the #1278871953721262090 ?

[dusk aurora]

Yea I wrote some notes about that

I do think so but it should be its own crate

like bevy_lint

[buoyant venture]

Ah, right, I should have read the PR description first 😅

[dusk aurora]

otherwise it force-breaks editor plugins of users

or at least if you ship it as a standalone bevy_fmt (cargo fmt + bsn fmt)

so that coupling should not be forced

[steel oak]

Yeah honestly I'm not surprised that landing BSN has unleashed the floodgates of things built on it, up to and including the editor

[dusk aurora]

Yea to be fair I was like hard focussing on these 2 things for 4 days now

and I was quite deep already

thematically

[buoyant venture]

That's what happens when you have a bevy of contributors working on something

[steel oak]

We are the #2 OSS game engine on github after all (iirc)

[dusk aurora]

Also the design itself did work out rather well in the end

the only thing is syn + comments

rn

that I can't get done (it is doable but yea...)

[buoyant venture]

Just checked, we still are (if you ignore imgui but I don't understand why it has the game-engine tag to begin with)

[rare whale]

For the moment, I just want the bsn/feathers PR to get unblocked

[steel oak]

One nice thing about using nom and lalrpop for the asset format as opposed to having a hand-coded parser is that it means we have a declarative official definition of the BSN grammar right there in the code

So if we wanted to build a spec it'd be easy to

And if you want to write tooling targeting BSN (which my studio very much does) then you can just open the LALRPOP file and the grammar is right there

[split harness]

Yup!

[split harness]

Agreed

[split harness]

Noted!

[amber pebble]

Since this might be tied to a specific Bevy version (in case of syntax changes etc) we could call this binary from the Bevy CLI, with automatic version detection and installation

But I guess for the editor integration there is also value in doing the version detection directly in the format binary and have it also be usable across versions

[thick slate]

Started on crate docs today; should have a PR up tomorrow

[cobalt stone]

for BSN?

[native mesa]

Don’t forget to add syntax docs for the DSL!

Actually we might want to consider writing a spec at some point

No one likes a language thats defined by a single canonical implementation… rust side

[thick slate]

Yeah, for bevy_scene_2

Crate-level docs only for now

[split harness]

Taking a look at the Feathers port stuff yall did while I was away

[tawdry owl]

To add a bit to the tooling conversation, another bit of tooling that would make bsn nicer to use is an LSP that would suggest only components in the bsn! macro (and dynamic bsn), preferring required components first. This would improve discoverability of required components and make it on par with what we had with bundles.

[magic belfry]

a lot of required components are often primarily internal components that are not meant to be added or edited by users manually, so it's not really the best heuristic for that imo

ex: any "computed" component (InheritedVisibility, ViewVisibility, ComputedNode, etc. in Bevy, ComputedMass, ComputedAngularInertia, ComputedCenterOfMass in Avian)

or various ID components or required marker components

[tawdry owl]

Yeah, computed and marker components should probably be their own category, at least docs and completion-wise

Which should also probably help world->bsn serialization? Computed can be ignored when writing back, although I'm not sure if the complexity of tagging component types would be worth it.

[thick slate]

Crate docs for y'all: https://github.com/bevyengine/bevy/pull/23604

[lime veldt]

finally I get to learn what this is all about instead of pretending

kept my fresh eyes sealed for a proper review

[thick slate]

squarely in the target audience 😄

[thick slate]

Tracking the rename work for the milestone: https://github.com/bevyengine/bevy/issues/23606

[rare whale]

@split harness @thick slate Now that I have read the docs, I think that FromTemplate is misnamed - it inverts the normal meaning of Rust's From / Into pattern. I suggest one of the following:

• IntoTemplate
• UseTemplate or UsesTemplate
• HasTemplate
• TemplatedBy

[thick slate]

See, I kind of like the way it mirrors FromWorld still

But no strong feelings here from me

[rare whale]

I read the words FromTemplate as meaning "constructs the object from a template", but what it actually does is the opposite: creates a template given the type of the target object.

[split harness]

FromTemplate is "correct" in my mind.

The From trait is From<Y> for X
If you could derive that , it would be #[derive(From<Y>)] struct X;, which can be read as "we can create X from Y"
Translating to templates, this would be #[derive(From<SpriteTemplate>)] struct Sprite , which can be read as, "we can create Sprite from a SpriteTemplate"
We express this as the FromTemplate trait, which when implemented for Sprite reads as "we can create Sprite from a Template", where the Template in question is defined by FromTemplate::Template, which is SpriteTemplate.

[rare whale]

All right, I will unpick this battle 🙂 There are more important things to worry about.

[steel oak]

Here's a fun little puzzle: The AssetPath -> HandleTemplate is currently driven by a From impl. We need that to be reflectable in asset BSN, and there is no ReflectFrom. ReflectFrom is a little weird too because it has a type parameter, unlike other Reflectable traits. You can't just #[reflect(From)] because the derive macro wouldn't know from what. And even specifying a single type to convert from is insufficient for the case of HandleTemplate because it's a blanket impl<I: Into<AssetPath<'static>>, T: Asset> From<I> for HandleTemplate<T> .
Anyone have any ideas as to how to solve this?
One possibility would be to have something like type_registration.register_conversion<TFrom, TTo>() where TTo: From<TFrom> and register all the transitive conversions between asset paths and handle templates manually.

[split harness]

Yeah I've thought about this before. Functionally this is just about building those edges somewhere, and we'll want it to be extendable by devs. I think you're on the right track. Define each conversion manually in a registry

[split harness]

No crazy graph theory / pathfinding stuff. Just "do we directly know how to convert A into B"

I don't think theres a good way to try to bring the whole Rust type/trait conversion behavior / path into Bevy Reflect. I'd prefer to take the dumb/easy path, at the cost of some additional manual wiring

Once we fill out the initial conversions for core types, I don't think the difference will be noticeable in the vast majority of cases

[split harness]

Just responded / pushed the fix I think is best: https://github.com/bevyengine/bevy/pull/23536#discussion_r3018036339

Happy to discuss though. This won't be the last time we see this pattern

[thick slate]

Perfect. Your solution was much cleaner than mine in the end

[split harness]

@rare whale when testing this I discovered that the custom_cursor feature doesn't enable bevy_feathers/custom_cursor. We should wire that up in bevy_internal via bevy_feathers?/custom_cursor

[rare whale]

There is still a CI failure with ColorChannel, should be an easy fix

[thick slate]

I wanted to prove to myself that a couple features worked when writing docs, so I wrote tests. Might as well PR them: https://github.com/bevyengine/bevy/pull/23611

[split harness]

I've resolved feedback on the Feathers Port / fixed up CI

Triggered a merge

@buoyant venture I responded to your question about if / else in bsn: https://github.com/bevyengine/bevy/pull/23536#discussion_r3020038308

@dusk aurora I'm starting a review of https://github.com/bevyengine/bevy/pull/23561

(working late tonight because I spent some time playing slay the spire 2 with my brother during work hours 😈 )

[split harness]

Quite liking the changes so far. The cleanup is nice, and I'm stoked that you solved the "also suggests function names" problem when typing struct fields

@dusk aurora wow the improved error handling is also great. bravo

[dusk aurora]

the short circuit behavior with ? isn't really used anymore but I think it might be nice to have this option for some errors in the future. We could also have a trait for each node but I didn't wanna take it that far yet.

just for additional context

or just when something else unrecoverable happens

[silver bramble]

Is this type of implementation currently not supported in bsn? I saw that there is a specialization trick, but not sure how it works or if it could be applied here.

If not, thats fine, I know bsn is still in the early phase, just wanted to know if I missed something.

#[derive(Clone, VariantDefaults)]
pub enum Primitive {
    Sphere {
        radius: f32,
        color: Color,
    },
    Circle {
        radius: f32,
        color: Color,
    }
}


impl Template for Primitive {
    type Output = (Mesh3d, MeshMaterial3d<StandardMaterial>);

    fn build_template(&self, context: &mut TemplateContext) -> Result<Self::Output> {
        let mut meshes = context.resource_mut::<Assets<Mesh>>();
        let mut materials = context.resource_mut::<Assets<StandardMaterial>>();


        let mesh = match self {
            Primitive::Sphere { radius, .. } => {meshes.add(Sphere::new(radius))}
            Primitive::Circle { radius, .. } => {meshes.add(Circle::new(radius))}
        };

        let color = match self {
            Primitive::Sphere { color, .. } => {color}
            Primitive::Circle { color, .. } => {color}
        };

        let material = materials.add(StandardMaterial {
            base_color: *color,
            ..default()
        });


        Ok((Mesh3d(mesh), MeshMaterial3d(material)))
    }

    fn clone_template(&self) -> Self {
        match self {
            Primitive::Sphere { radius, color } => {
                Primitive::Sphere {radius: radius.clone(), color: color.clone()}
            }
            Primitive::Circle { radius, color } => {
                Primitive::Circle {radius: radius.clone(), color: color.clone()}
            }
        }
    }
}

[thick slate]

Yes please yes please

[rare whale]

@split harness How do I convert an EntityReference to an Entity? I need to do this:

Scrollbar {
    orientation: ControlOrientation::Vertical,
    target: #inner,
    min_thumb_length: 6.0,
}

Where target is an Entity.

Similarly, I can't pass a name as a parameter to a template:

(
    scrollbar(#inner, ControlOrientation::Vertical)
    Node {
        position_type: PositionType::Absolute,
        right: px(0),
        top: px(0),
        bottom: px(0),
        width: px(6),
    }
),

This gives me an error, "expected one of ! or [, found inner"

[split harness]

Yup this should work, as written, with bsn!{ @Primitive::Sphere { radius: 10 } } (notice the @ because this is a direct template type).
The only "problem" with this implementation is that it will create new meshes for each instance of the template. Theres a world where you'd want to add a Resource to cache the different permutations behind their parameters.

[split harness]

This should be as simple as deriving FromTemplate for Scrollbar to allow the target field to be templated

[split harness]

This one is not supported yet (but it definitely should be). I believe functions in that position currently read their parameters as raw rust code, so its trying to read it as #inner, which is not valid Rust.

Definitely a case we want to support. It will require some smarts to support both arbitrary rust and #Name references

But that is essentially the same case as field: #Name, which currently supports #Name syntax but not arbitrary inline Rust expressions (just values). We want to support both in both cases, and solving one should solve the other

I'm also of course very interested in "#Name in closures", which will require a completely different strategy (templates that take entity references and return closures with the resolved entity)

[round totem]

Can attribute or derive macros be expressed inside a bsn! invocation? (I'm guessing not)

(Just wanting to make sure)

[rare whale]

@split harness I'm calling parent.queue_spawn_related_scenes::<Children>(bsn_list!(...etc...) and nothing is showing up, what could be wrong?

[split harness]

They can wherever raw rust is supported (ex: inside of {} in value position). Given that bsn! generally isn't defining types, I'm not sure why you'd want this generally

[round totem]

I'm moreso trying to think of a syntax for bsn files that denote metadata

[split harness]

Hmmm yeah thats going to require some debugging. If you have a repro you'd like me to check out let me know

[round totem]

Pcwalton's pull has a simple bsn file in it with none of that Metadata and it got me wondering how we could do that

[split harness]

For now, .bsn files cannot define new types, and that will be true for the forseeable future

[split harness]

My default thought is it has dependencies (or thinks it has dependencies)

(which aren't loaded)

[rare whale]

OK I've got a branch which demonstrates a number of things: https://github.com/viridia/bevy/pull/new/moar_widgets - this is porting over to main all of the widgets I created on your bsn branch

You can repro by running the feathers example and clicking on the menu button

[round totem]

Im moreso thinking of a way to let the parser know what "version" of bsn syntax its using. Sort of like how you specify in Cargo.toml the Rust version the rustc compiler should parse

Or how CMakeLists.txt specifies its version syntax (but hopefully less gross)

[split harness]

I'll give this a look after wrapping up the bsn! macro code quality pr

[round totem]

It doesn't need to be fleshed out but I think we'd be better off at least reserving a syntax for meta specifications in bsn files

Since bsn is rust-ish inspired I wonder if we could reserve something like #![] for setting bsn meta attributes

[split harness]

The planned versioned import syntax can potentially be used for this. (ex use bevy{0.19}::prelude::* could serve as an indicator that we are using 0.19 bsn syntax)

Although thats admittedly a bit of a hack

My problem with reserving syntax is that it requires doing design for something that we haven't built yet / committed to build.

[round totem]

#![bevy_version(0.19)]

[split harness]

Yup I understand the scenario now

[rare whale]

@split harness I also want to talk a bit about the menubutton API. The main idea here is that when the user clicks the button, a signal is sent to the app to spawn the menu items (because we don't want to have a bunch of hidden menu items hanging around when the menu popup is not visible). This could be done either as a callback, or as an observer. Neither is ideal:

• Callback
  • This would look like: menu(|parent| { parent.queue_spawn_related... })
  • Not possible to use this in asset bsn
• Observer
  • This would listen for a MenuOpen event, and would be declared using the normal on syntax
  • I can't pass an EntityCommands in an Event, so the observer has to have a lot more access to the internals of the menu system - it means that the observer has to do additional housekeeping and boilerplate

[round totem]

(And whatever goes in the bevy_version() attribute would be any legal semver string)

[white lichen]

when this has come up before ive suggested disambiguating the way cargo does, like [email protected]

use [email protected]::prelude::*

[split harness]

I believe thats essentially what I mentioned here

Other than the syntax

Not opposed to that syntax

[white lichen]

we had pretty much this exact sequence of messages a year or two ago lol

you propose {}, i suggest @, you agree

[round totem]

Other custom syntaxes could be enabled with #![] like in nightly rust

[split harness]

I'll have to add this to the stack. Hard to have an opinion without loading up on context / committing time

[thick slate]

I like this syntax a lot

[split harness]

Maybe this time it will stick 🙃

[white lichen]

lol

almost exactly 2 years ago

[round totem]

I like the #![] idea because it'll allow users to experiment with adding custom stuff to the syntax and enable them like in nightly rust

Like nightly experimental features in Rust

#![bsn_feature(foo)]

If/when bsn's syntax gets expanded and experimented on in the future this could be a boon

[white lichen]

i dont see why this should be combined with bevy versioning

[round totem]

Could do [email protected] or bevy@anysemverstring+1234 like propose before

And then do #![] for toggling experimental syntax that isn't part of bevy core yet

[split harness]

Do you mean you don't see why [email protected]::prelude::* should be responsible for defining BSN syntax versioning, or you don't see why it shouldn't be?

[white lichen]

i dont see why feature gating experimental stuff is related to versioning

i was talking about the #![bsn_feature(foo)] proposal

i dont see how thats related to bevy versioning

[round totem]

Versioning and experimental features are both meta attributes to the parser

[split harness]

Its relevant because [email protected]::prelude::* is about bringing in Bevy 0.19 types, which could be done in the context of any BSN format version

And defining the BSN syntax format shouldn't necessarily involve importing bevy types

[white lichen]

i would think bsn format versions are more like "Is Children shorthand supported" type things

[split harness]

Yup

[white lichen]

and the specific bevy version types involved are not really bsn related

[split harness]

Yup sounds like we're on the same page

[white lichen]

cool

[split harness]

So something like #![bsn_version(1.2)] is "necessary" in that context

[white lichen]

right, but thats got no bearing on which bevy types are available per se

[split harness]

However I kind of like the idea that [email protected]::prelude::* can imply a given bsn version, when none is specified, in the interest of terseness

[split harness]

Fully agreed

[white lichen]

maybe we want to call them bsn editions, not versions? like rust editions

not sure if thats semantically closer

they're language changes, not crate changes if that makes sense

[split harness]

Yeah its worth discussing. Rust editions are kind of just spec version numbers with specialized / slightly nonstandard semantics

[white lichen]

like, in this analogy bevy is the stdlib, and adding stuff to is like stablizing stuff. adding if let matches is a language change and wont be accepted by previous editions

[split harness]

Less scary than saying "rust 2"

[white lichen]

hrmm yeah

maybe its not a worthwhile analogy

[split harness]

I'm personally tabling this conversation for later 🙂

Feel free to come up with thoughts / specs

[white lichen]

~~i'll suggest @ again in 2 years when it comes up again ~~

[split harness]

Curious what people think about the "duplicate component error" added in this PR:
https://github.com/bevyengine/bevy/pull/23561#discussion_r3024485660

(respond there plz)

[split harness]

Kicked off a merge of https://github.com/bevyengine/bevy/pull/23561

[split harness]

Starting my investigation of this

[split harness]

Seems like something specific to menu_popup. If you add the following to the spawned bsn_list! a red square shows up:

parent.queue_spawn_related_scenes::<Children>(bsn_list![
    (
        Node { width: px(100.), height: px(100.)}
        BackgroundColor(RED)
    ),
    (
        :menu_popup
        Children [
            // children here
        ]
    )
]);

@rare whale hehe removing Visibility::Hidden seems to do the trick 🙂

[split harness]

Rather than spawn the menu items lazily when clicked, why not just make this structural / hidden by default? Ex:

(
    menu()
    Children [
        (
            :menu_button(MenuButtonProps::default())
            Children [
                (Text("Menu") ThemedText)
            ]
        ),
        (
            :menu_popup
            Children [(
                :menu_item
                on(|_: On<Activate>| {
                    info!("Menu item clicked!");
                })
                Children [
                    (Text("MenuItem") ThemedText)
                ]
            )]
        )
    ]
)

That allows the menu items to have their dependencies propagated up to the root if they have them. It also lets things like entity references work across boundaries, and improves legibility in general

It also ensures all of the items are "ready to go", which might improve responsiveness

[thick slate]

@split harness do you want to try and land the formatter for 0.19? If so, I'll review @dusk aurora's https://github.com/bevyengine/bevy/pull/23586 tomorrow. Otherwise, plenty of misc release prep stuff for me to do.

[dusk aurora]

I think I need to update the branch first but I can probably do it tomorrow

not sure if it can be merged tomorrow though

[thick slate]

Yeah, that's totally fine. I'm expecting a couple days of back-and-forth most likely

[split harness]

Imo it should be a stretch goal if we have time. We can always link to the PR as a WIP for people that are interested

[dusk aurora]

yea you can totally create the binary locally

[split harness]

I'm working on the bevy_scene rename atm

[thick slate]

kk. IMO we cut the release candidate ASAP then, and then work on this in the background while that marinates

[split harness]

After the rename 🙂

[thick slate]

Right, yes. I remember wanting to do the renames from #23606 but not wanting to clobber the couple of open important BSN PRs

I think we're good now that the macro code quality PR is in

[split harness]

https://github.com/bevyengine/bevy/pull/23619

[rare whale]

Oh, it's because they changed how popup layouts work last week

[rare whale]

The reason I don't like this is because (a) some menus are quite large (e.g. the font selector in inkscape has a menu item for every font), and (b) an editor might have a large number of menu buttons. Also (c) some menu items might incur an overhead even when invisible, for example if their label is dynamically computed. Because of this, I prefer menus, dialogs, and other dynamic content to not exist rather than be hidden.

[split harness]

Riiight I remember this conversation. From my perspective, the cost:benefit plays out in favor of "always spawned menus" in the average case, which is a small number of menu items that are just some text and (perhaps) some icons.

Pros of laziness:

• very expensive menus that have expenses when they are invisible don't incur overhead
• we get some "dynamicness" for free, as we can automatically re-compute a new menu each time it is shown
• we don't wait to prepare the dependencies of menus when spawning the rest of the scene. for expensive menus with many asset dependencies (icons, fonts, inherited scenes), time to first render is faster.

Cons of laziness:

• we don't get "free" preloads of assets which are defined as "required scene dependencies" (ex: inherited scenes, fonts, image icons, etc). Everything is loaded lazily on click, meaning skipped frames and/or pop-in. For "big" assets,/scene dependencies this could be many frames.
• we need to re-compute on every click, which is more expensive than just hiding the root of the menu.
• in most cases, we don't do any rendering or processing of hidden nodes, so the overhead should be extremely minimal. The "don't do work for hidden items" case is generally very easy to optimize.
• developers can't directly define their menus inside the rest of the BSN scene. They need to define a closure that is passed as an argument, which then calls a command, which then defines a macro with the scene contents.
• developers can't use Entity references (ex: #Name syntax) to connect menu items to other items in the scene

[rare whale]

I guess we need both

[split harness]

Agreed. I think the cost:benefit is different depending on the context

[rare whale]

I agree that for small use cases, being able to define the items inline is very convenient

[split harness]

I think for most things, hiding but skipping expensive work is the move

Ex: we don't really want to be recreating state each time we click between tabs

Much better to flip a switch that says "you aren't focused right now" and make it the responsibility of the UI internals (and custom logic) to ensure it doesn't do work in that case

The thing with "lazy menu icon loading" vs "immediate / blocking menu icon loading (aka part of a loading screen)" is that we already plan to support that distinction via HandleTemplate

If we do "always spawned menus", the developer will have both options

If we do "lazy menus", the developer is forced to be lazy, no matter what the configuration of the HandleTemplate is

[split harness]

Although this will notably kick off the configured "lazy" icon load immediately after spawn, even if the menu isn't visible

But I will assert that this is generally preferable to someone clicking a menu and icons popping in randomly (or not at all)

(because the load kicks off right when the click happens)

[exotic pecan]

This just convinced me to change part of my UI

Not all of it, mind you

(I’m not using BSN yet, so, mostly focusing on the points related to asset usage here)

[rare whale]

@split harness I'm still having trouble figuring out how to make a clean API for scrollbars in BSN. The basic structure is that there's a Scrollbar component in the headless widgets that includes an entity id for the region being scrolled - that is, an entity whose ScrollPosition is kept in sync with the state of the scrollbar. Then feathers wraps this in a scrollbar scene function which adds a thumb child and a bunch of styling components.

Earlier you suggested implementing FromTemplate for Scrollbar but that doesn't really help because users aren't calling that directly; they are calling the feathers wrapper. I mean, I could remove Scrollbar from the wrapper and have the users call it directly, but that's a really ugly API - hi, here's a function which outputs all the components needed for scrollbars except for the actual Scrollbar component.

So we need to solve two problems: first, the user needs to be able to pass in the entity id to the wrapper; and second, inside the wrapper, we need to be able to take the passed in parameter and put it in the component. It's unclear (to me) whether that parameter needs to be an Entity or an EntityReference - that is, does it get resolved by the caller or the callee?

Note that if we had struct template support, we could work around the first since we could make scrollbar a struct instead of a function. That doesn't help with the second problem though.

An additional problem is that I can't actually figure out how to implement Template for Scrollbar, there are only 3 examples of impls in the code and they are quite different from my use case.

[silver bramble]

The problem I have is that if I implement default I get:

error[E0119]: conflicting implementations of trait `Template` for type `Primitive`
   --> src/main.rs:100:1
    |
100 | impl Template for Primitive {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: conflicting implementation in crate `bevy_ecs`:
            - impl<T> Template for T
              where T: Clone, T: std::default::Default, T: Unpin;

But if I don't implement it I get:

error[E0277]: the trait bound `Primitive: std::default::Default` is not satisfied
  --> src/main.rs:67:18
   |
67 |       let sphere = bsn! {
   |  __________________^
68 | |         #base
69 | |         @Primitive::Circle {
70 | |             radius: 1.0,
...  |
73 | |     };
   | |_____^ the trait `std::default::Default` is not implemented for `Primitive`

So not sure how I work around that.

[dapper sky]

I assume we don't have @ImageNode { image: "image.png" } yet? 😅

as an alternative to asset server everywhere

[topaz ginkgo]

I’ve been working on an asset catalogue for jackdaw, and hoping to get a PR up soon that should help with this!

[split harness]

Looks like we have a bug in our Template / @ syntax codegen for enums

Its trying to do FromTemplate on the enum variants, which is very wrong in the context of raw templates

[split harness]

It would just be ImageNode { image: "image.png" }. We haven't done derive(FromTemplate) on that type yet, so it doesn't currently work. Still need to do that more globally. Planning on picking it up after the rename

But 0.19 should launch with handle templates working everywhere

The bevy_scene rename is such a slog. So many "scene" things :/

[split harness]

I'll need to hold off on thinking about this in the short term

[dapper sky]

That makes sense! it came up pretty frequently in the bavy jam.

that and a desire for some kind of bsn_for_each :')

[pseudo fern]

Bsn gets so fun when you start doing map().collect()

[rare whale]

@split harness Another question about APIs: how should I design widgets with slots? Let's take as example the text input widget, which looks like this:

• feathers_text_input (the frame entity)
  • left_adornments (icons such as a search magnifying glass, etc.)
  • the actual TextInput
  • right_adornments (more icons such as x to clear)
    Where both "left adornments" and "right_adornments" are bsn_list parameters

[split harness]

I can't get in the weeds here right now, but for things that are "init only" (ex: things that are very unlikely to need adding / removing after init), parameterizing seems fine (ex: accept a left_adornments: impl SceneList parameter, or if we land struct-style, struct FeathersTextInput<L: SceneList> { left_adornments: L }.

For things that are likely to be dynamic (ex: lists, collections, etc), until we land reactivity I think we should embrace direct BSN / structural APIs as much as possible, as abstracting over that structure will ultimately make it harder for developers to change it dynamically.

[split harness]

I do believe FeathersTextInput<L: SceneList> { left_adornments: L } might be problematic with the current BSN impl, as type inference breaks down in the context of FromTemplate, meaning the actual type must be expressed. This could be worked around with left_adornments: Box<dyn SceneList>. Shouldn't be a problem with implicit .into()

[rare whale]

In that case, let me add to the stack of requests: some sugar for declaring a Box<dyn SceneList> inline

[dapper sky]

And alternatively, Mesh2d(<mesh data source of some kind. Primitive shapes, or obj files>)

[split harness]

https://github.com/bevyengine/bevy/pull/23630

[topaz ginkgo]

https://github.com/bevyengine/bevy/issues/23637 - i raised an issue with some of the pieces i added into Jackdaw in order to get us into the world of editing BSN. i'd like to open a few PR's (hopefully small and easy to review!) with a final PR that encompasses an API for an editor to use

[topaz ginkgo]

First PR is up for scene serialisation: https://github.com/bevyengine/bevy/pull/23639

[rare whale]

Would Option<Box<dyn SceneList>> also work? Most of the time the list is going to be empty and I don't want to pay the overhead of allocating memory.

This won't compile:

Children [
    {props.adorn_left}
]

the trait bound `std::option::Option<std::boxed::Box<dyn bevy_scene2::SceneList>>: bevy_scene2::SceneList` is not satisfied

[rare whale]

@split harness @crude pawn Also, we have a problem that TextFont needs to implement FromTemplate (it's currently on team Clone+Default). InheritableFont does; but that's a feathers thing, and we need for TextFont to be usable for non-feathers people.

[topaz ginkgo]

https://github.com/bevyengine/bevy/pull/23648 - this is the initial draft of the asset catalogue port from jackdaw. i will caveat and say i am certainly not an assets expert, so this may not be the best approach - also depends on @steel oak dynamic-bsn changes!

[split harness]

Rad! It would definitely be nice if we could deprecate bevy_world_serialization in 0.20

[split harness]

This is awesome! Just to set expectations I'm very likely to pump the breaks on this space until after Assets as Entities lands, as I think that will be important for a clean implementation

[split harness]

If you implement SceneList for Option<S: SceneList>, I think this will work (as Box<dyn SceneList> already impls SceneList)

[split harness]

Yup planning on doing a FromTemplate pass over the relevant Bevy types today-ish

[topaz ginkgo]

That’s totally cool, i don’t think it’s a showstopper for me in jackdaw/editor land 🙂

[split harness]

I think this is in a merge-able state. Another review would be nice 🙂
I also have the bevy_scene2 -> bevy_scene + adding to the bevy::prelude changes queued up

[split harness]

https://github.com/bevyengine/bevy/pull/23668

[thick slate]

Merging 🙂 Don't forget the migration guides for this work!

[split harness]

Scene2 doesn’t need a migration guide, and the bevy_scene to bevy_world_serialization PR already included a migration guide

[thick slate]

Oh perfect; I must have missed that migration guide. All good then

[rare whale]

@split harness Back on the topic of "slots", I'm still struggling to come up with a workaround. Let's take MenuButton as an example. The behavior we want is:

• Menu button requires a label, which can be either text or an icon
• The dropdown chevron is automatically placed next to the label
  Right now the way I am doing it is an ugly hack: You call the :menu_button scene function, and then you immediately follow it with Children [ (your label) ]. Since children are appended, this means that the label and the chevron are in the wrong order, so I fix that by setting the flex direction to row_reverse.

This creates a problem if we ever want to change the hierarchical structure of the button - right now it assumes that (a) the label is a direct child of the button, which might not be the case for a future implementation, and (b) the label is always appended as the last child. If we decided that we needed an extra layer for layout purposes, there's never going tobe a Grandchildren [] relation in bsn.

I experimented instead with adding a label parameter to MenuButtonProps, but couldn't get that to compile.

Now, I also want to give users the option to replace the chevron with a custom icon (the bevy editor designs make extensive use of custom drop-down icons).

I wanted to add an optional icon field to MenuButtonProps, which would let users override the icon, but this creates another problem: if we declare the type of icon as Option<I> where I is a generic param bound by SceneList, it means that in the majority of cases, where you want to pass None, Rust can't infer the type of I.

Another approach would be to make the menu_button function take positional parameters (this lets me use impl SceneList) but then I lose the ability to have defaults, it means now you have to explicitly pass in None if you don't want a custom chevron.

[split harness]

Option<Box<dyn SceneList>> would solve that problem no?

[split harness]

Its an abominable hack that doesn't hold up in the context of repeated templates (ex: spawning multiple times or inheritance). But I did make this work:

/// set up a simple 3D scene
fn setup(mut commands: Commands) {
    commands.spawn_scene_list(bsn_list![
        // circular base
        (
            Mesh3d(asset_value(Circle::new(4.0)))
            MeshMaterial3d<StandardMaterial>(asset_value(Color::WHITE))
            Transform::from_rotation(Quat::from_rotation_x(-FRAC_PI_2))
        ),
        // cube
        (
            Mesh3d(asset_value(Cuboid::new(1.0, 1.0, 1.0)))
            MeshMaterial3d<StandardMaterial>(asset_value(Color::srgb_u8(124, 144, 255)))
            Transform::from_xyz(0.0, 0.5, 0.0)
        ),
        // light
        (
            PointLight {
                shadow_maps_enabled: true,
            }
            Transform::from_xyz(4.0, 8.0, 4.0)
        ),
        // camera
        (
            Camera3d
            template_value(Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y))
        ),
    ]);
}

(a port of 3d_scene)

The "hack" being adding this variant: HandleTemplate::Value(Arc<Mutex<Option<T>>>)

Swapping the Option with an enum AssetOrHandle<A: Asset> { Asset(A), Handle(Handle<A>) } would make it work in the context of repeated templates, but thats still quite the hack

[cobalt stone]

The other thing I want is something like PartialDefault

Where some fields have sensible defaults, and other fields require you to fill out in the BSN

See https://github.com/bevyengine/bevy/issues/23688

[thick slate]

Very similar to @rare whale's requests for enums where some variants can be defaulted but others can't

[split harness]

This would likely require new syntax if we're enforcing it at compile time, as the default Struct { x } syntax semantically implies a default template

Im not sure we can encode "maybe implied default maybe partial default, based on the type" in the Rust type system

I mean we can from a "encoding the variant" perspective. But I don't think we could then use it to drive the necessary behaivors

I think in the majority of cases, we should just make things work by default

Required fields don't feel particularly necessary in this case.

[split harness]

Ex: instead of crashing in the renderer, this could log a helpful error and/or fill in an "error" skybox value (pink)

[split harness]

This is essentially the "default field values" proposal: https://github.com/rust-lang/rust/issues/132162

And is notably the behavior difference between Foo { ..Default::default() } and Foo { .. } in that proposal

Aka they also require different syntax

[cobalt stone]

Can we always rely on PartialDefault, and just impl PartialDefault for T: Default?

[rare whale]

the trait bound `std::boxed::Box<dyn bevy_scene::SceneList>: bevy_scene::SceneList` is not satisfied

[split harness]

Perhaps. Although I'd like to see what your PartialDefault trait looks like.

[split harness]

Ah yeah still need to add the impls for the boxed Scene and SceneList traits

Definitely want those

[cobalt stone]

Yeah idk what it would look like. Some magic derive macro that returns PartialMyType, where all fields are wrapped in Option?

[split harness]

Yeah probably something like that. Maybe better to wait for Rust's answer to the problem. Although I doubt we could use it in the context of asset BSN

I feel like our own ParitalDefault impl would be dirty / make an already quite complicated typing situation in BSN even more complicated

I'm not sure the cost:benefit is there relative to just making our types default-friendly

/ default template friendly in the cases where the actual component field is not default-able

[split harness]

Also checking all of those fields for is_some() on init to enforce PartialDefault also feels like we've lost the plot a little bit

Although I think we'd only need to eat that check for required fields

[split harness]

I actually think this would fit into the existing XTemplate -build_template-> X system

Ex: label a field with #[template(required)], which uses the RequiredTemplate<T> wrapper (which is like an Option but returns a template error when it isn't set)

The issue there is that it doesn't give us compile-time errors, so its just a standard way to have an error at runtime

And in general, i think it is better for that type of thing to be handled on a case-by-case basis, for more descriptive messages / the ability to define fallback behaviors

[cobalt stone]

that seems fair to me

We'd maybe want #[template(default(foo))] then as well

In order to not rely on Default

[rare whale]

This branch has a number of feathers widgets (menu, listbox, scrollbar) that are pretty much complete except for bad scene function ergonomics: https://github.com/viridia/bevy/tree/moar_widgets - if you feel like hacking on something

[split harness]

https://github.com/bevyengine/bevy/pull/23696

[split harness]

I have #[template(built_in)] queued up. I'll PR once this merges

[split harness]

Just put out a PR to impl Scene for Box<dyn Scene>. Same for SceneList:
https://github.com/bevyengine/bevy/pull/23698

[rare whale]

Will that be enough to make Option<Box<dyn Scene>> work as well?

[split harness]

Yup! For a gracious definition of "well":

It still requires field: Some({Box::new(bsn! { Node })})

Or in your case, more likely some_widget(Some(Box::new(bsn!{ Node })))

[rare whale]

It's only a flesh wound!

[split harness]

https://github.com/bevyengine/bevy/pull/23699

[thick slate]

You'll never guess what my review says 😉

[rare whale]

@split harness Working on trying out your latest changes. It works, but one thing I quickly ran into was that Box<dyn Scene> can't derive Default, so I have to manually implement it; also, this means that the default allocates memory:

impl Default for MenuButtonProps {
    fn default() -> Self {
        Self {
            label: Box::new(bsn_list!()),
            corners: Default::default(),
        }
    }
}

Also, assuming we can get to a place of comfort here, I'd like to revise the APIs for button, checkbox, and the other label-bearing widgets to use similar patterns - passing in the label as a parameter rather than man-handling the widget's children.

[raw mesa]

Box::new(()) probably doesn't allocate memory AFAIK. ZSTs never need to be accessed so I've been told the optimizer doesn't do the allocation.

In fact I'm using that assumption in my migration of BSN to assets as entities, so if that's not true... That's a problem

(I need to hokey-pokey a SceneListPatch in order to spawn it, but doing that fully would mean removing the AssetData<SceneListPatch> component, using it, then inserting it back onto the entity. So instead I just make a fake dummy instance for SceneListPatch and replace the data during the hokey instead)

Actually I guess even if it does allocate, it's still better than the two archetype moves

[rare whale]

Too bad we can't point all empty bsn_lists to a global immutable singleton 🙂

[split harness]

In theory we could have some BoxSceneList(Box<dyn SceneList>) type that implements Default (or alternatively, make it an enum to avoid the risk of allocation). This would also have the benefit of allowing us to support impl<T: SceneList> From<T> for BoxSceneList, which Box<dyn SceneList> doesn't have on its own.

Making it play nice with Into would likely (eventually) yield fruit in the context of defining this in field-value-position in BSN (which has implicit into)

The big downside is that its another "special" type people need to think about + learn, rather than just using the natural tools Rust gives us (Box / traits)

[rare whale]

Well, you know my philosophy when it comes to DSLs: first make it work, and then make it pretty

@split harness One thing I hope to get out of the review for the menu button is feedback on the DX; I expect that there will be opinions about what the API should look like. For example, menu items look like this now:

(
    :menu_button(MenuButtonProps {
        label: Box::new(bsn_list!(
            (Text("Menu") ThemedText),
        )),
        ..default()
    })
),

For the moment, the "optional chevron" feature is handled by having a different scene function menu_button_without_arrow. I thought about instead having a boolean in the menu button props to indicate whether to include the standard arrow, but I realized I wasn't sure what was the canonical way to do conditional logic (non-reactive) in bsn blocks.

To support the editor, we're going to need variants of menu_button, for example some dropdown buttons in the mocks are smaller and have the "tool button" form factor.

We might also need to implement a combo box, which combines the functionality of text input and dropdown menu

And then there's autocomplete, which in my experience is one of the more difficult widgets to implement.

[split harness]

I think we should consider treating something like a "label slot" as a single bsn entity / entry rather than a bsn_list:

1. It slims things down slightly
2. It would allow us to resolve the label to a single Entity and perhaps expose that as a property on the MenuButton component for easier / direct access to that logical piece.

Cases that really require multiple entities can create their own nesting. I think bsn_list is best used for "collection" abstractions.

Ex: (2) would allow let node = nodes.get(menu_button.label)?;

[rare whale]

I'd prefer not to do this; buttons are flexboxes, and it's common to have text + icon button labels.

[split harness]

Might be better to call it labels then

Or it could be a more generic button.content

I dont have strong opinions here

[split harness]

The parse logic in template(SOME_PATH) was a bit broken after adding template(built_in). Just pushed a fix here: https://github.com/bevyengine/bevy/pull/23699

[steel oak]

At some point I'd like some feedback on my .bsn asset PR to know if it's on the right track

There's still some stuff it's blocked on like ReflectConvert but other than that a lot of it is just docs & polish, but I don't want to spend time polishing something if there are concerns about the approach

[rare whale]

@split harness So, it looks like there's no way to have a conditional child without boxing being involved at some level.

[lime veldt]

Because it would return different types at the branching? Can't do bsn something alike the SpawnIter<core::option::IntoIter> thing one can do at bundle level?

[rare whale]

Yeah, I was wondering if there was a SpawnIter or SpawnWith escape hatch I could use.

Even this doesn't compile:

Children [
    {props.label},
    {
        if props.arrow {
            Box::new(bsn_list!(
                Node {
                    flex_grow: 1.0,
                },
                :icon(icons::CHEVRON_DOWN),

            ))
        } else {
            Box::new(bsn_list!())
        }
    }
]

[lime veldt]

might need casting so it is Box<dyn Foo> instead of Box<Bar1> and Box<Bar2> at each branch

[rare whale]

Interesting. I tried Box::<dyn SceneList>, but new complains about it not being Sized.

Maybe a custom template function? I need to look at the docs again.

[lime veldt]

try Box::new(...) as Box<dyn Foo>

it is weird it does not work like you did it but it should work that way

[rare whale]

Holy crap that actually works

That being said, I bet you could write a ConditionalScene impl of the trait