-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Next Generation Bevy Scenes #20158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Next Generation Bevy Scenes #20158
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -145,6 +145,7 @@ default = [ | |
"bevy_picking", | ||
"bevy_render", | ||
"bevy_scene", | ||
"bevy_scene2", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @3xau1o I'm converting your top level comment to a thread (please read the PR description)
BSN does not currently support reactivity (please read the PR description). There have been many investigations into both fine grained and coarse reactive solutions (both diff-ing and signal-based) / we are well versed in the space at this point. The goal for this phase is to (if possible) build BSN in such a way that it can support both paradigms. Then we can iterate / have an ecosystem (potentially even cross-compatible) where ideas can compete. From there if a winner arises, we can bless it as the "go-to" / default solution. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can confirm there's multiple people in the community who I know are planning to experiment with different approaches. I'm looking forward to building a coarse-grained diffing system, and I'll bet @viridia will build a fine-grained solution. I'm really pleased with this reactivity-agnostic approach, let's us avoid a big bikeshed and do more incremental exploratory work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On one hand:
On the other hand:
Can a diff-based system be the foundation of an efficient implementation of fine-grained reactivity? The suggestion seems to be that this is supposed to be a neutral foundation, but I'd argue that it is not. You can't build a (true) reactive system on top of a diff-based system without many compromises. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you may be confusing two different things: Inheritance and reactivity. Patches are just for inheritance. Personally, I don't see how they relate to reactivity (or incrementalization) at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NthTensor Uhh... Gotcha. They're more like layers then, right? Maybe the confusing "patch" terminology could be avoided. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
this is only possible if BSN is only a DSL syntax like JSX which is used in both Reactjs and Solidjs there is also the need to realize that
they're so different, mostly opposite, that's why Vue.js vapor was mostly a rewrite instead of an upgrade from Vue.js 3 vdom, Vue Vapor and Vue3 are not compatible, they only use the same vue syntax, same as React and Solidjs use JSX There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think you might be mixing up unrelated things. The compilation you're speaking of in e.g. SolidJS is compiling away the framework into plain JS functions. This has no relevance to the Bevy / Rust case... |
||
"bevy_sprite", | ||
"bevy_sprite_picking_backend", | ||
"bevy_state", | ||
|
@@ -167,6 +168,7 @@ default = [ | |
"x11", | ||
"debug", | ||
"zstd_rust", | ||
"experimental_bevy_feathers", | ||
] | ||
|
||
# Recommended defaults for no_std applications | ||
|
@@ -249,6 +251,9 @@ bevy_render = ["bevy_internal/bevy_render", "bevy_color"] | |
# Provides scene functionality | ||
bevy_scene = ["bevy_internal/bevy_scene", "bevy_asset"] | ||
|
||
# Provides scene functionality | ||
bevy_scene2 = ["bevy_internal/bevy_scene2", "bevy_asset"] | ||
|
||
# Provides raytraced lighting (experimental) | ||
bevy_solari = [ | ||
"bevy_internal/bevy_solari", | ||
|
@@ -598,8 +603,9 @@ flate2 = "1.0" | |
serde = { version = "1", features = ["derive"] } | ||
serde_json = "1.0.140" | ||
bytemuck = "1" | ||
bevy_render = { path = "crates/bevy_render", version = "0.17.0-dev", default-features = false } | ||
# The following explicit dependencies are needed for proc macros to work inside of examples as they are part of the bevy crate itself. | ||
bevy_render = { path = "crates/bevy_render", version = "0.17.0-dev", default-features = false } | ||
bevy_scene2 = { path = "crates/bevy_scene2", version = "0.17.0-dev", default-features = false } | ||
bevy_ecs = { path = "crates/bevy_ecs", version = "0.17.0-dev", default-features = false } | ||
bevy_state = { path = "crates/bevy_state", version = "0.17.0-dev", default-features = false } | ||
bevy_asset = { path = "crates/bevy_asset", version = "0.17.0-dev", default-features = false } | ||
|
@@ -619,6 +625,7 @@ anyhow = "1" | |
macro_rules_attribute = "0.2" | ||
accesskit = "0.19" | ||
nonmax = "0.5" | ||
variadics_please = "1" | ||
|
||
[target.'cfg(not(target_family = "wasm"))'.dev-dependencies] | ||
smol = "2" | ||
|
@@ -2788,6 +2795,14 @@ description = "Demonstrates loading from and saving scenes to files" | |
category = "Scene" | ||
wasm = false | ||
|
||
[[example]] | ||
name = "bsn" | ||
path = "examples/scene/bsn.rs" | ||
|
||
[[example]] | ||
name = "ui_scene" | ||
path = "examples/scene/ui_scene.rs" | ||
|
||
# Shaders | ||
[[package.metadata.example_category]] | ||
name = "Shaders" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
use bevy_ecs::system::{Commands, SystemId, SystemInput}; | ||
use bevy_ecs::system::{Commands, IntoSystem, SystemId, SystemInput}; | ||
use bevy_ecs::template::{GetTemplate, Template}; | ||
use bevy_ecs::world::{DeferredWorld, World}; | ||
use std::marker::PhantomData; | ||
|
||
/// A callback defines how we want to be notified when a widget changes state. Unlike an event | ||
/// or observer, callbacks are intended for "point-to-point" communication that cuts across the | ||
|
@@ -27,15 +29,119 @@ use bevy_ecs::world::{DeferredWorld, World}; | |
/// // Later, when we want to execute the callback: | ||
/// app.world_mut().commands().notify(&callback); | ||
/// ``` | ||
#[derive(Default, Debug)] | ||
#[derive(Debug)] | ||
pub enum Callback<I: SystemInput = ()> { | ||
/// Invoke a one-shot system | ||
System(SystemId<I>), | ||
/// Ignore this notification | ||
Ignore, | ||
} | ||
|
||
impl<I: SystemInput> Copy for Callback<I> {} | ||
impl<I: SystemInput> Clone for Callback<I> { | ||
fn clone(&self) -> Self { | ||
match self { | ||
Self::System(arg0) => Self::System(arg0.clone()), | ||
Self::Ignore => Self::Ignore, | ||
} | ||
} | ||
} | ||
|
||
impl<In: SystemInput + 'static> GetTemplate for Callback<In> { | ||
type Template = CallbackTemplate<In>; | ||
} | ||
|
||
#[derive(Default)] | ||
pub enum CallbackTemplate<In: SystemInput = ()> { | ||
System(Box<dyn RegisterSystem<In>>), | ||
SystemId(SystemId<In>), | ||
#[default] | ||
Ignore, | ||
} | ||
|
||
impl<In: SystemInput + 'static> CallbackTemplate<In> { | ||
pub fn clone(&self) -> CallbackTemplate<In> { | ||
match self { | ||
CallbackTemplate::System(register_system) => { | ||
CallbackTemplate::System(register_system.box_clone()) | ||
} | ||
CallbackTemplate::SystemId(system_id) => CallbackTemplate::SystemId(*system_id), | ||
CallbackTemplate::Ignore => CallbackTemplate::Ignore, | ||
} | ||
} | ||
} | ||
|
||
pub trait RegisterSystem<In: SystemInput>: Send + Sync + 'static { | ||
fn register_system(&mut self, world: &mut World) -> SystemId<In>; | ||
fn box_clone(&self) -> Box<dyn RegisterSystem<In>>; | ||
} | ||
|
||
pub struct IntoWrapper<I, In, Marker> { | ||
into_system: Option<I>, | ||
marker: PhantomData<fn() -> (In, Marker)>, | ||
} | ||
|
||
pub fn callback< | ||
I: IntoSystem<In, (), Marker> + Send + Sync + Clone + 'static, | ||
In: SystemInput + 'static, | ||
Marker: 'static, | ||
>( | ||
system: I, | ||
) -> CallbackTemplate<In> { | ||
CallbackTemplate::from(IntoWrapper { | ||
into_system: Some(system), | ||
marker: PhantomData, | ||
}) | ||
} | ||
|
||
impl< | ||
I: IntoSystem<In, (), Marker> + Clone + Send + Sync + 'static, | ||
In: SystemInput + 'static, | ||
Marker: 'static, | ||
> RegisterSystem<In> for IntoWrapper<I, In, Marker> | ||
{ | ||
fn register_system(&mut self, world: &mut World) -> SystemId<In> { | ||
world.register_system(self.into_system.take().unwrap()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this leak? We need a way to unregister the callback when the owner is despawned. This is why I had proposed using cached one-shots for inline callbacks, but we have to solve the type-erasure problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does leak in its current form. This style of "shared resource" cleanup feels like it should be using RAII, as we discussed previously. I'm not sure how the a cached one-shot would solve this problem, as it would still leak in that context as it doesn't use RAII? From my perspective the only difference between this approach and register_system_cached is where the cache lives (inside the template, which is shared across instances, or inside world).
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because cached one-shots eventually get removed automatically, the For the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I understand the disconnect. We wouldn't use |
||
} | ||
|
||
fn box_clone(&self) -> Box<dyn RegisterSystem<In>> { | ||
Box::new(IntoWrapper { | ||
into_system: self.into_system.clone(), | ||
marker: PhantomData, | ||
}) | ||
} | ||
} | ||
|
||
impl< | ||
I: IntoSystem<In, (), Marker> + Clone + Send + Sync + 'static, | ||
In: SystemInput + 'static, | ||
Marker: 'static, | ||
> From<IntoWrapper<I, In, Marker>> for CallbackTemplate<In> | ||
{ | ||
fn from(value: IntoWrapper<I, In, Marker>) -> Self { | ||
CallbackTemplate::System(Box::new(value)) | ||
} | ||
} | ||
|
||
impl<In: SystemInput + 'static> Template for CallbackTemplate<In> { | ||
type Output = Callback<In>; | ||
|
||
fn build( | ||
&mut self, | ||
entity: &mut bevy_ecs::world::EntityWorldMut, | ||
) -> bevy_ecs::error::Result<Self::Output> { | ||
Ok(match self { | ||
CallbackTemplate::System(register) => { | ||
let id = entity.world_scope(move |world| register.register_system(world)); | ||
*self = CallbackTemplate::SystemId(id); | ||
Callback::System(id) | ||
} | ||
CallbackTemplate::SystemId(id) => Callback::System(*id), | ||
CallbackTemplate::Ignore => Callback::Ignore, | ||
}) | ||
} | ||
} | ||
|
||
/// Trait used to invoke a [`Callback`], unifying the API across callers. | ||
pub trait Notify { | ||
/// Invoke the callback with no arguments. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ValorZard I'm converting your top-level comment into a thread (please see the PR description):
The idea is that
.bsn
will be the subset ofbsn!
that can be represented in a static file. In the immediate short term, that will not include things like theon
function, as we cannot include arbitrary Rust code in asset files.The primary goal of
.bsn
will be to represent static component values editable in the visual Bevy Editor. In the future we might be able to support more dynamic / script-like scenarios. But that is unlikely to happen in the short term.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drat. I thought I did the comment thing correctly.
but otherwise that makes sense. So .bsn and bsn! Aren’t fully equivalent. Interesting.
I guess Bevy could support something like Godot’s callables in the future maybe, and have some sort of id sorted in the .bsn file that could be converted to a function? Idk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, function reflection + a registry should make stringly-typed callbacks defined in assets very feasible.