Skip to content
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

Add schematics RFC #64

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Add schematics RFC #64

wants to merge 16 commits into from

Conversation

MDeiml
Copy link

@MDeiml MDeiml commented Oct 19, 2022

Rendered

Unresolved questions:

  • How to synchronize the other direction
  • How to be able to have "requirements" in schematics. I propose something like Archetype Invariants bevy#1481 which could also be used outside this RFC
  • How to serialize the schematic world
  • How to check if:
    • Every component gets converted
    • Conversions don't conflict

See also bevyengine/bevy#3877

@SamPruden
Copy link

It's great to see this moving! I'm glad to see my initial proposal is well received. This looks like a great start!

Thoughts in no particular order:

  • This fairly tightly couples a schematic entity to having a single corresponding runtime entity. I think that's probably reasonable, but I just wanted to call it out explicitly. The lowest level version of this feature should probably support scenarios where that correspondence doesn't exist, even if they're very rare.
  • Having the iterator provide the commands is much neater than the convoluted idea I was developing.
  • The resource situation is a little tricky. Any resources touched by the system would need to become dependencies of the conversions, and trigger reconversion if they change.
  • The spawn_or_select_child idea with labels is nice.
  • This doesn't tackle the problem of multiple schematics coordinating in non trivial ways.
  • Do we want schematic systems to be able to mutate the schematic world? This feels very dangerous. A bug in a system could corrupt the editor save file in ways that may not be immediately noticed. It should be possible to do this from code somehow, but it should be very hard to do this by accident.
  • This is covered by "stability against changes", but I would emphasise that one of the goals is to allow significant technical changes to be made without concern for breaking compatibility with already authored content.
  • I was originally including the customisation of schematic component UI as part of this proposal, but you're absolutely right to separate it out from this RFC. If schematics are just components, that can be a completely orthogonal feature.
  • This design directly mutates the runtime world. My instincts say that this is a bad idea. When it comes to the parts which aren't tackled yet (conflict resolution and coordination, which I think are actually the hardest parts apart from readback) I think it's going to be necessary to maintain an intermediate constraint specification structure representing the runtime world. I'm concerned that if we try to add those later, we'll end up having to scrap the work we've already done. I can't see a way around a three pass (collect, resolve, apply) approach to schematic conversion, although I'd be very happy to be proved wrong.

@MDeiml I'd be very interested to hear your thoughts on immediate conversion vs collect-resolve-apply.

One thing I would emphasise is that we should be thinking of this as a feature aimed at a non-technical audience. Obviously implementing a schematic is a technical process, but a designer consuming them should not be assumed to have any technical experience. For this reason, things like invariant enforcement with good error descriptions are essential.

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

Thanks for your feedback! This was really fast. Gonna reorder your points to organize my answer better

  • This fairly tightly couples a schematic entity to having a single corresponding runtime entity. I think that's probably reasonable, but I just wanted to call it out explicitly. The lowest level version of this feature should probably support scenarios where that correspondence doesn't exist, even if they're very rare.

You could just leave the corresponding entity empty. Maybe we could even spawn the entity lazily for the first inserted component. Loosing this correspondence would mean one extra call to spawn_or_select if you want a corresponding entity, which is what you want 99% of the time. Having this corresponding entity makes it just way more easy to use IMO.

  • The resource situation is a little tricky. Any resources touched by the system would need to become dependencies of the conversions, and trigger reconversion if they change.

That is tricky indeed. There is no Changed or ChangeTrackers. Also most of the time the behavior of Changed wouldn't even be practical here. I don't think there's a way around giving responsibility to the developer here.

That being said in most cases this should be fine. A typical case where you would Res in the first place is getting a handle to an asset. In this case changes to the AssetServer wouldn't really matter.

  • Do we want schematic systems to be able to mutate the schematic world? This feels very dangerous. A bug in a system could corrupt the editor save file in ways that may not be immediately noticed. It should be possible to do this from code somehow, but it should be very hard to do this by accident.

Similar to the last point I think. This might make sense in some cases, but we would really need to trust the developer to do it right. Imagine for example a schematic that generates some randomness like a random id when it is converted. It then wants to remember this id somewhere, which can only be in the schematic world.

The other thing to consider here is that schematics as I described them are just systems. Currently it is not possible to restrict systems to be read-only at the schedule level. SchematicQuery is intended to be read-only (didn't explicitly say that I think). If you still think this is important I suggest forbidding mutating queries as system params so that systems that want to mutate components need to be exclusive (i.e. take &mut World).

  • This is covered by "stability against changes", but I would emphasise that one of the goals is to allow significant technical changes to be made without concern for breaking compatibility with already authored content.

Sounds good. I also think I didn't stress the "stable interface" part enough. Feel free to suggest specific changes to the document btw :)

One thing I would emphasise is that we should be thinking of this as a feature aimed at a non-technical audience. Obviously implementing a schematic is a technical process, but a designer consuming them should not be assumed to have any technical experience. For this reason, things like invariant enforcement with good error descriptions are essential.

Completely agree. There should also be error detection for e.g. not every schematic component being read, runtime components not being converted back, ...

  • This doesn't tackle the problem of multiple schematics coordinating in non trivial ways.
  • This design directly mutates the runtime world. My instincts say that this is a bad idea. When it comes to the parts which aren't tackled yet (conflict resolution and coordination, which I think are actually the hardest parts apart from readback) I think it's going to be necessary to maintain an intermediate constraint specification structure representing the runtime world. I'm concerned that if we try to add those later, we'll end up having to scrap the work we've already done. I can't see a way around a three pass (collect, resolve, apply) approach to schematic conversion, although I'd be very happy to be proved wrong.

@MDeiml I'd be very interested to hear your thoughts on immediate conversion vs collect-resolve-apply.

I agree on both points. This question is not yet resolved with what we got so far. Gonna make an extra comment to keep things organized ;)

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

Now to the question of schematics interacting / resolving conflicts. My intuition here is the following:

(almost) All runtime data needs to be converted to data in the schematic world somehow. Otherwise the scene format that is created using schematics would not really be "complete". You could also think about this from the other direction: Every change you want to make to the runtime world should be achievable through changing something in the schematic world. That's the whole point of having schematics as a editor representation.

Now IMO it would be good to ensure at least to some degree that there is no duplicate data in the schematic world. As an example imagine an entity that has both a "mesh renderer" component and a "particle system" component. Both of these depend on a "visibility" component. My argument is that (in the schematic world) "visibility" should neither be saved in the "particle system" component nor in the "mesh renderer" component, as otherwise we would save this data twice in the schematic world. For me the better design here would be to have a "visibility" component in the schematic world that is seperate from "mesh renderer" and "particle system".

For me what these design choices naturally lead to, is that every piece of data in the runtime world has one and only one corresponding piece of data in the schematic world. Now the term "piece of data" is not very exact, but my suggestion would be to interpret that as "component" as components (in the runtime world) already should be "atmoic" i.e. be so small that breaking them apart into multiple pieces makes them kind of useless. A component that can be separated into multiple pieces IMO are bad design. Summarizing what I say is that "every component in the runtime world has one and only one corresponding component in the schematic world".

Of course this immediately makes the process of conversion quite a bit easier. There is no need for "conflict resolution" any more other than checking that every component in the runtime world is only touched by one schematic (one system). As such there's also no need for "collect-resolve-apply". (Checking what I just mentioned should be possible by just looking at the commands that are produced by each system)

On the other hand it doesn't solve the problem of enforcing envariants, i.e. every entity with a "mesh renderer" component should also have a "visibility" component (in the schematic world). My suggestion here would be to solve this problem completely in the schematic world. We could for example look for the "mesh renderer / visibility" invariant in the schematic world and then resolve it there or give out an error message, whatever is more appropriate. See also bevyengine/bevy#1481.

Sry for the wall of text. Happy to hear your opinion on this.

@SamPruden
Copy link

This was really fast. Gonna reorder your points to organize my answer better

A fair subtle roast of my organisational habits.

That is tricky indeed. There is no Changed or ChangeTrackers. Also most of the time the behavior of Changed wouldn't even be practical here. I don't think there's a way around giving responsibility to the developer here.

Is there any fundamental reason why there couldn't be change trackers for resources? On the "mutable access is an assumed change" principle of course. I can imagine "schematic resources" also being used for things like per-level configurations in the editor. In that case, we definitely would need to automatically rerun conversion when they changed. For me, giving responsibility to the developer is a last resort - the API should be as simple to use and as hard to break as possible.

The other thing to consider here is that schematics as I described them are just systems. Currently it is not possible to restrict systems to be read-only at the schedule level. SchematicQuery is intended to be read-only (didn't explicitly say that I think). If you still think this is important I suggest forbidding mutating queries as system params so that systems that want to mutate components need to be exclusive (i.e. take &mut World).

Ah, SchematicQuery being read-only does help! Requiring something explicit like .add_mutating_schematic() seems reasonable.

Your second comment will receive a second reply. Spoilers; we're going to disagree on that one.

@alice-i-cecile
Copy link
Member

FYI, resources do use change tracking: they're accessible via the .is_changed() and .is_added() methods.

@SamPruden
Copy link

SamPruden commented Oct 20, 2022

One-to-one schematic components to runtime components fundamentally contradicts the design goals for me, for a number of reasons.

Schematic components are intended to be logical units of functionality from the perspective of a designer. Often, that logical unit will be significantly larger than the atomic components used at runtime. My goto example is a Rigidbody Schematic. The designer wants one big block they can add to an object, with neat checkboxes for things like "Is Kinematic". Dynamic and kinematic rigidbodies likely have quite different sets of runtime components, and certainly break into many atomic parts.

One-to-one is also incompatible with the goal of being flexible to changes (both small and large) in the runtime data layout. You may design the schematic as being one-to-one, then want to break the runtime data into multiple components later.

I see two broad categories of cooperation between schematics:

  • Multiple schematics each adding the same component. A trivial example might be that multiple types of rendering schematic all want to add a DoesRenderingTag component. If there's one thing I know about ECS, it's that people find clever patterns you might not think of up front, and we don't want to get in their way.
  • Schematics requiring a component to be added by another schematic, but not adding it themselves. An example might be that the Rigidbody Schematic complains about needing a Transform component to be added, and ideally suggests "Try adding a Transform Schematic". (That would require the editor to know which schematics are capable of adding which components, which is an idea that I'm exploring.)

On the principle that we should empower users as much as possible, my inclination is towards a powerful underlying schematic "engine", capable of tracking all of these dependencies and managing constraints. We then build simple APIs for the common case on top of that engine. It's harder but (IMO) it's better.

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

Sry, I was not very clear. I proposed a "one-to-many" structure. I'll make a concrete example cause it's much easier to talk about it that way.

struct MeshRendererSchematic {
    mesh: Handle<Mesh>,
    material: Handle<Material>,
}

struct VisibilitySchematic {
    visible: bool
}

struct TransformSchematic {
    translation: Vec3,
    rotation: Vec3,
    scale: Vec3,
}

fn main() {
    App::new()
    // ...
        .add_schematic(mesh_render_schematic)
        .add_schematic(visibility_schematic)
        .add_schematic(transform_schematic)
        // results in `VisibilitySchematic` automatically being added
        .add_archetype_invariant(MeshRenderSchematic::implies(VisibilitySchematic))
        // results in an error message if entity doesn't have a `TransformSchematic`
        // (`implies` would also fit better here, just for example purposes)
        .add_archetype_invariant(MeshRenderSchematic::requires(TransformSchematic))
    // ...
        .run();
}

fn mesh_render_schematic(query: SchematicQuery<MeshRendererSchematic>) {
    for (mesh_renderer_schematic, commands) in query {
        commands.insert_or_update(mesh_renderer_schematic.mesh);
        commands.insert_or_udpate(mesh_renderer_schematic.material);
    }
}

fn transform_schematic(query: SchematicQuery<TransformSchematic>) {
    for (transform_schematic, commands) in query {
        commands.insert_or_update(Transform {
            translation: transform_schematic.translation,
            rotation: Quaternion::from_euler(
                EulerRot::default(),
                transform_schematic.rotation.x,
                transform_schematic.rotation.y,
                transform_schematic.rotation.z,
            ),
            scale: transform_schematic.scale,
        });
        // This could maybe even be an `ArchetypeInvariant` on the runtime world
        commands.insert_or_update(GlobalTransform::default());
    }
}

fn visibility_schematic(query: SchematicQuery<VisibilitySchematic>) {
    // you get the point
}

I guess this even allows a "many-to-many" relationship between components. Just not a "many-to-many" relationship between schematic systems and runtime components.

Are we getting closer to agreeing? xD

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

FYI, resources do use change tracking: they're accessible via the .is_changed() and .is_added() methods.

That's great. Still means that developers need to check this manually. Do you think that's ok? Otherwise we could maybe do some magic and have something like SchematicParam which is different from SystemParam and automatically has change tracking, but that would maybe limit the user and add a lot of complexity to the implementation. We would also need IntoSchematic, FunctionSchematic, ... basically duplicate the whole ecs::system module.

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

On the principle that we should empower users as much as possible, my inclination is towards a powerful underlying schematic "engine", capable of tracking all of these dependencies and managing constraints. We then build simple APIs for the common case on top of that engine. It's harder but (IMO) it's better.

To summarize, we're disagreeing in two points (probably):

  1. "one-to-many" vs "many-to-many" for schematic systems <-> runtime components
  2. Resolving conflicts during the conversion process (what you call "schematic engine" I guess) vs resolving conflicts inside the schematic world (what I call "archetype invariants")

@SamPruden
Copy link

SamPruden commented Oct 20, 2022

Ah, our positions are much closer than I thought then! The "every component in the runtime world has one and only one corresponding component in the schematic world" line threw me off.

I've certainly been imagining that something like schematic implies/requires relationships would be possible, so we're agreed on that.

However, this brings up the question of whether we're expecting people to only have one canonical schematic to achieve a certain goal, and whether we're comfortable with those schematics being tightly coupled to each other.

If there is one single TransformSchematic, then doing a basic requires on it makes sense. But, if we want the user to be able to build their own MyTransformSchematic, that should also be compatible. Using the archetype invariant method, the MeshRenderSchematic is tightly coupled to the TransformSchematic, and incompatible with MyTransformSchematic. This probably doesn't make much sense for transforms, but for more complex things it could happen.

The other approach is to define compatibility purely on the basis of the interpreted runtime data. This approach is purer, more versatile, and (I think) strictly more powerful. However, it does require a significantly more complex implementation.

One issue with the runtime data compatibility approach is that it's possible for the compatibility between schematics to change accidentally if their implementation changes. Personally, I think I'm happy saying that if programmers mess that up then they've introduced a bug, go fix it.

I'll make a follow up comment with a very rough sketch of how I imagine schematic systems may look, attempting to deal with neatly piping through the necessary information for change tracking, constraint resolution, and nice editor error messages and suggestions.

@SamPruden
Copy link

SamPruden commented Oct 20, 2022

Here's a very rough vision for the collect-resolve-apply constraint based approach.

I'm going to use the language of "interpreter functions" instead of "interpreter systems" to avoid ambiguity. Maybe they would be registered as systems, or maybe they would be managed through some schematic engine layer.

Common interpreters

fn interpret_transform(schem: &TransformSchematic, commands: SchematicCommands<TransformComponent>) {
    commands.require_exact(TransformComponent(schem.pos, schem.rot, schem.scale));
}

Note that this takes a single schematic component, not a query . The function will be invoked once for each schematic that needs interpretation - it's a map. The engine knows which schematic component is being interpreted, and tracks dependencies for the purpose of knowing which things to invalidate when schematic data changes. The only way for this automated tracking to be broken/cheated would be for the function to touch global data.

The commands are in a declarative constraint format. require_exact is very straightforward, but it could also be require_any, require_any_or_default, or even require_matching_predicate_or_default.

SchematicCommands<TransformComponent> includes the output component/s in the function signature. This is to allow the engine to know which schematics can produce which components. The primary motivation for this is to allow the editor to say "This schematic requires a TransformComponent. Try adding a TransformSchematic or a MyTransformSchematic."

It would be an error to constrain a component that isn't included as a SchematicCommands type parameter. Sadly this would have to be a runtime error for now (an issue Bevy already has with query.get_component()), although there's a Rust fix for that in the pipeline, see my comment on the RFC: rust-lang/rust#29864 (comment)

We can also allow things like resource parameters here, with dependency tracking of them done in the same way as tracking of schematic components themselves. I.e. some outer system doing the tracking work and invoking interpreters.

Query interpreters

Occasionally, we may want a conversion process to depend on arbitrary data in the schematic world.

fn some_crazy_interpreter(
    query: SchematicQuery<(TransformSchematic, SomeOtherSchematic)>,
    Res<SomeResource>,
    SchematicCommands<(TransformComponent, SomeOtherComponent)>
) {
    // Iterate and constrain, using arbitrary data.
}

In this version, all commands would be given a dependency on all schematic components in the query. You can use the data in arbitrary ways, but any change to any of the schematics will trigger reinterpretation of all of them. (We could give advanced APIs to override these dependencies with manual ones if people want to tune performance.)

The resolution pass logic is actually relatively simple (check exact requirements first, then soft ones - if there's a conflict, throw an error), but I'll leave that to later.

@SamPruden
Copy link

Note that this takes a single schematic component, not a query .

To motivate why I prefer this over for (a, commands) in query:

Using the query approach, it's possible to cheat the tracking by storing information from previous iterations in a local. I don't believe that there's any way to 100% stop people from ever breaking the tracking, but forcing them to write to global data to break it feels more robust to me. It's more rusty.

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

Also seems like a good design.

  • I really like your naming using require. Seems a lot more concise than my insert_or_update.

  • I'm not sure I understand the generic argument in SchematicCommands. Can you add more parameters to common interpreter functions? Otherwise I don't think this is needed.

  • For the query interpreters I quite liked my design of having iteration also return the commands. With the function signature where SchematicQuery and SchematicCommands are separate code would have to look like

    for (entity, component) in query {
        let commands = schematic_commands.get(component);
        // ...do stuff with `commands`
    }

I think we should talk about the duplicate API of "schematic functions" and "schematic systems" though. Tbh I think there's good reasons for and against both (feel free to add points, I'll edit this comment to stay up to date):

Pros:

  • Less verbose in 99% of all cases (this is a big win)
  • Harder to break correspondence

Cons:

  • Duplicate API (biggest disadvantage IMO)
  • Might lead users to write less efficient systems (not sure if this is a concern) if the schematic needs memory access / calculation that is common to all schematic components of the same type
  • Schematics are not just systems, so there needs to be some translation between the two
  • Breaking change tracking is possible using SchematicQuery, but I don't think it would happen unintentionally. People writing schematics are technical users, so probably it's fine to trust them a bit. I guess what could make the API a lot more clear is to require syntax like for (component, commands) in query.iter_changed() with an for (component, commands) in query.iter_every_single_component_you_might_not_want_to_do_this() alternative.

Finally regarding resolving conflicts in the schematic world vs as part of conversion: (The complexity of implementation should be roughly similar.)

Pro "schematic world" / archetype invariants:

  • Can be used for things other than schematics
  • Persistent, i.e. the result of the resolution is saved as part of the schematic world
  • Apparent to user. Components don't get added quietly but instead would pop up in the editor. This is similar to Unity's [RequireComponent(...)] I think.
  • Invariants can be added to schematic world as well as runtime world as opposed to only in the runtime world

Pro "collect-resolve-apply":

  • Better integrated syntactically with interpreters / schematics. Uses the require syntax inside schematic functions as opposed to add_invariant in setup code
  • As such probably easier to understand for the coder
  • More customizable, as it is defined using code rather than structs

Btw I'm amazed at how quickly this is moving :). Seems like together we'll be able to come up with a pretty nice design here.

@SamPruden
Copy link

  • I'm not sure I understand the generic argument in SchematicCommands. Can you add more parameters to common interpreter functions? Otherwise I don't think this is needed.

You can also do a query style SchematicCommands<(ComponentA, ComponentB)>.

The intent is to allow the engine to know which interpreter functions can create which components.

This is useful for editor UX reasons. If we have a schematic with a require::<TransformComponent>() constraint (i.e. it requires a transform component to be present, but doesn't add one itself), the editor can help the designer by saying "I know that TransformSchematic produces that component that you need! Why don't you try that?"

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

If we have a schematic with a require::<TransformComponent>() constraint

I'm still don't understand completely, I'm sorry. The call to requrie::<TransformComponent> contains all the information needed, does it not? What additional information can be gathered from the generic in the function parameter? Or is there a case I'm not thinking of where the informatino from the require call can not be accessed?

@SamPruden
Copy link

SamPruden commented Oct 20, 2022

The call to requrie::<TransformComponent> contains all the information needed, does it not?

We want the engine to be able to work this out without running the interpretation function.

There may be no TransformSchematics currently in the scene, so that interpreter has never run. Nevertheless the editor would like to know that a TransformSchematic is probably a good way to satisfy a .require::<TransformComponent>() constraint.

It works this out by seeing that there's a registered interpreter which takes TransformSchematic as input and potentially produces TransformComponent as output.

It's an attempt to recapture the niceness of add_archetype_invariant automatically adding the TransformSchematic when it's needed.

Under my proposed approach, user experience would be:

  1. Designer drags a MeshRendererSchematic onto object that lacks a transform.
  2. Get clear error message saying "MeshRenderer requires a TransformComponent. Try either TransformSchematic or MyTransformSchematic.
  3. User clicks on one of those suggestions to get it immediately added.
  4. Error goes away.

@SamPruden
Copy link

SamPruden commented Oct 20, 2022

For the query interpreters I quite liked my design of having iteration also return the commands. With the function signature where SchematicQuery and SchematicCommands are separate code would have to look like

Yep, I also thought that was very pretty.

As a matter of personal style, I always like to encode constraints into the type system as heavily as possible. I wil happily go to extra effort to move documented restrictions and runtime errors into compiler errors. Given that personal leaning - which I think Rust shares - I prefer the more locked down approach.

I don't think it's a dealbreaker either way, it's a surface level difference.

Might lead users to write less efficient systems (not sure if this is a concern) if the schematic needs memory access / calculation that is common to all schematic components of the same type

Hmm, that seems like a rare but very fair concern.

@SamPruden
Copy link

Schematics are not just systems, so there needs to be some translation between the two

I think maybe this is a good thing. In fact, maybe we want to encapsulate the schematic World and only allow access through schematic APIs.

If we want to enforce proper use of dependency/change tracking, we probably want to disallow people from doing untracked queries, which they would be able to do if it were just a plain system. This is my personal tendency towards locking things down (without restricting power) again.

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

As a matter of personal style, I always like to encode constraints into the type system as heavily as possible

Oh, I'm not sure I explained myself good here. My comment was regarding your query interpreters where SchematicQuery and SchematicCommands are separate. Having SchematicQuery return the SchematicCommands seems to be a bit more safe to me. The generic parameter from SchematicCommands would then need to be in SchematicQuery I guess. But also this is not a very important point.

Btw thanks for clarifying my question about the generic parameter, I understand it now. It's solving a problem though, that only comes up with the collect-resolve-apply approach, not with archetype invariants.

Otherwise I think we reached a point where we just have different but equally valid opinions, so we should maybe write down both designs with their pros and cons and then get feedback from other people. Do you agree?

We could add two sections for archetype invariants and your collect-resolve-apply approach with a comment explaining that this is a "pick one out of two" situation. Also add a section for your "common interpreters" with a comment explaining the feature is optional.

@SamPruden
Copy link

Having SchematicQuery return the SchematicCommands seems to be a bit more safe to me.

Oh I see. Yes, I think this is just a matter of API style, they seem equivalent in all meaningful ways.

It's solving a problem though, that only comes up with the collect-resolve-apply approach, not with archetype invariants.

Agreed.

Otherwise I think we reached a point where we just have different but equally valid opinions, so we should maybe write down both designs with their pros and cons and then get feedback from other people. Do you agree?

There's a big scary monster that we haven't addressed yet, and that's readback.

If we assume that it's possible to drop in and out of a "play mode" in the editor, we would ideally like changes to runtime state to somehow be reflected back into the schematics. Schematics are the interface that designers are used to using to interact with that data, and we'd like them to be able to stay in that familiar territory for basic observation of runtime state. We also want to be able to do things like edit the TransformSchematic live in the editor during gameplay to move an object while testing gameplay.

I don't believe an automated solution is possible in the general case, because we can't (and don't want to) enforce that the schematic -> runtime mapping is bijective. We could trivially create an interpreter that ignores a schematic field, and now there's no way to recreate the schematic from the runtime data.

I expect that this will require some type of opt-in manually implemented "schematic inference" functionality. It won't be possible for everything, but for something like TransformSchematic it would be easy to implement.

A big question here is whether we attempt to fold that API into the existing interpreters (so an interpreter also specifies how to invert itself as part of the command) or whether the solution is to have a completely separate set of "inference systems" that go the other way.

We should probably do some thinking about whether this part of the problem impacts the choice between the two designs before we solicit feedback on them.

@MDeiml
Copy link
Author

MDeiml commented Oct 20, 2022

We should probably do some thinking about whether this part of the problem impacts the choice between the two designs before we solicit feedback on them.

Good point. I think I'll still write down both solutions for future reference.

I don't believe an automated solution is possible in the general case

Agreed

I expect that this will require some type of opt-in manually implemented "schematic inference" functionality

I also think that's the best tactic here. It would be nice though if the editor could output something like "TransformSchematic is not being read back" in an appropriate place, so there should be a mechanism to detect schematics that don't have this implemented.

A big question here is whether we attempt to fold that API into the existing interpreters (so an interpreter also specifies how to invert itself as part of the command) or whether the solution is to have a completely separate set of "inference systems" that go the other way.

Good question. Intuitively I'd suggest something like:

  • Common interpreters are implemented as a trait on a unit struct
  • Inference for those interpreters is another trait (that extends the interpreter trait) that you would implement on the same struct. This way both can be added together
  • Query interpreters are a bit more crazy, basically just systems, so probably it's ok if their corresponding inference is not grouped with them

Thinking about this now (as you predicted) makes me more keen to include common interpreters for that exact reason. 😅

@SamPruden
Copy link

SamPruden commented Oct 20, 2022

Edit note: Originally I called them Edit and Play schematics. This has been changed to forward and backward.

One possible solution to this is to have two classes of schematics, forward and backward. Forward schematics are the ones already discussed in this thread. Backward schematics are the ones that get inferred from the runtime state during play. Often, one schematic will hold both roles - but there's room for both forward only and backward only schematics.

  • Forward only shematics are those with interpretation but not inference.

  • Backward only schematics are those with inference but not interpretation. They would be used to display (and potentially edit) runtime data that's not part of the editor/serialized state. They only exist while in play mode.

Inference functions would be like inverse interpreters. They can perform queries over the runtime world, and infer schematics into the edit world. (Editing gets a bit complicated...)

In the case of something like TransformSchematic, the same identical schematic would have both interpretation and inference, meaning there's no UX difference between edit and play.

Every time I've tried to solve this readback problem previously, I've ended up concluding that my solution was fatally flawed in some way. So I wonder what's wrong with this one - any ideas?

@SamPruden
Copy link

Good point. I think I'll still write down both solutions for future reference.

I'm happy to do the write up of mine if you want to do yours - it's probably best if each is presented by its strongest advocate.

Thinking about this now (as you predicted) makes me more keen to include common interpreters for that exact reason. 😅

Amusingly I actually don't think this is a good reason! In the inference model proposed above, I'm quite happy for them to be implemented independently, even if they're commonly side-by-side in code.

An advantage of that inference approach is that it's not tied too closely to the schematic that generated the data. We should assume that runtime data will drift away from what the schematic specified (including components being added and removed) and our play UI should attempt to be as tolerant of that as possible. A little conceptual separation here may not be a bad thing.

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

Because the main purpose of the schematic is to be a stable representation of the data

I don't really agree, but I guess you invented the term, sooo. In my opinion the main purpose is to be the representation that is used for UI. I guess we just don't agree here, but in the end it's also the code authors decision what they use it for and not ours. We can just make suggestions.

The custom runtime UI has no stability requirement and can be changed at any time

I see that point. So in the end there would be four representations: schematic, schematic UI (which hopefully is not that different), runtime and runtime UI (which hopefully is the same as schematic UI).

Changing runtime UI without changing schematics means though, that the UI would suddenly behave different when entering play mode. Maybe in a way that is not noticeable to the user. That seems dangerous to me.

there's no conversion process

There is a conversion process to UI components. Imagine for example that rotation in the UI is given in euler angles. There is a tiny error in floating point arithmetic when converting quaternions to euler angles and back. This error would appear both for readback and custom runtime UI.

What would the inference function look like in this case?

The author could either choose to round the float (there is an error, but the error would not multiply over time, also obviously the error would only come up if the designer would actually edit the schematic during runtime). Alternatively the author could choose to change the schematic. I would strongly suggest the second option. Otherwise the value couldn't be set to e.g. 1.5 during the actual editing, so outside play mode.


Updated pro / contra (just meant to keep this structured):

Pro readback:

  • No duplicate effort for UI (I think we agree)
  • More consistent experience (Not sure you agree)
  • get more out of derives I guess runtime UI could also be derived with a bit of extra effort
  • With the "merging" I suggested, readback is only optional (Not sure if you like that solution)

Pro runtime UI:

  • Easier to implement for us (IMO we're just leaving the problem to whoever implements UI)
  • Allows schematic representation to drift from runtime representation (I don't think that's a good thing)
  • Easier to implement for users (I don't agree)

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

I feel like we're at a point where it might be possible that we can't agree. So I'd offer a different way to look at it. The scope of this RFC is schematics, not UI. Obviously we need to make sure that a UI can actually built from what we're creating, but we don't need to think about the specifics.

This means that we shouldn't focus on whether custom runtime UI is useful. I actually agree that it is. The question is rather if readback is useful, if it is implementable and if it can cause unsafety / frustration:

I think we might agree that it is useful. I claimed that it is also implementable, but obviously that question will be answered as soon as I write the "implementation" section in the RFC for this.

So the remaining question is, whether it can cause unsafety or frustration. I take it that your answer to this is "yes"?

@SamPruden
Copy link

SamPruden commented Oct 22, 2022

I don't really agree, but I guess you invented the term, sooo. In my opinion the main purpose is to be the representation that is used for UI. I guess we just don't agree here, but in the end it's also the code authors decision what they use it for and not ours. We can just make suggestions.

TBH I don't understand how instability could ever be acceptable here.

I think we're agreed that schematics are the format in which assets/scenes are serialized. In that case, how can it be acceptable for that format to be unstable? That would break compatibility with assets already built - which is a huge deal!

There's some amount of tolerance here if we e.g. use Serde for serialization and use its field attributes to handle simple migrations, but that's fairly brittle and we want it to happen as little as possible.

So in the end there would be four representations: schematic, schematic UI (which hopefully is not that different), runtime and runtime UI (which hopefully is the same as schematic UI).

I don't quite think of the UIs as "representations" because the data is never stored in them - they're more like mapping functions from (state, user input) -> modified state, but I suppose that's only an incidental semantic difference.

However, customising the UI is optional in both places, so it's two representations + more if you care about making this one pretty.

The simple case of just combining multiple runtime components into one UI could be automated too - it would just have to inline their default representations next to each other in one panel. A simple attribute or one-liner could take care of that.

Changing runtime UI without changing schematics means though, that the UI would suddenly behave different when entering play mode. Maybe in a way that is not noticeable to the user. That seems dangerous to me.

I think it's on the programmer not to do anything too silly here.

There is a conversion process to UI components. Imagine for example that rotation in the UI is given in euler angles. There is a tiny error in floating point arithmetic when converting quaternions to euler angles and back. This error would appear both for readback and custom runtime UI.

This is a great point I'd never thought of - the imgui pattern might accumulate precision error in some cases! However, this is how Unity (and I think Unreal and Godot?) do it today and I've never heard of anybody complaining about it being a problem, so I guess it's not a big deal in practice.

The author could either choose to round the float (there is an error, but the error would not multiply over time, also obviously the error would only come up if the designer would actually edit the schematic during runtime). Alternatively the author could choose to change the schematic. I would strongly suggest the second option. Otherwise the value couldn't be set to e.g. 1.5 during the actual editing, so outside play mode.

Scenario: My game is on a grid, and it's only allowable to set those positions to grid-aligned values. However, they're floats at runtime because they can be animated between grid positions.

Updated pro / contra (just meant to keep this structured):

Good summary!

No duplicate effort for UI (I think we agree)
More consistent experience (Not sure you agree)

Agree ish. However, because schematic data and runtime data are sometimes fundamentally different, forced consistency may be bad.

With the "merging" I suggested, readback is only optional (Not sure if you like that solution)

Equally true for the UI route, where customisation only happens if you don't want to accept the defaults. Mixing defaults and customised is basically the same experience as merging.

@SamPruden
Copy link

I feel like we're at a point where it might be possible that we can't agree.

I fear we may be.

So the remaining question is, whether it can cause unsafety or frustration. I take it that your answer to this is "yes"?

Yes.

I contest that any solution that you come up with, I will be able to give you plausible real-world scenarios that readback will not be able to handle satisfactorily, but that will be handled by the UI route. You're welcome to take that as a challenge. 😂

@SamPruden
Copy link

I think I'm going to put together a "rival" RFC - of course intended in the spirit of cooperation not rivalry! That way we can get a clear look at both approaches side by side, and perhaps bring others in to comment at that stage. I may play around with a prototype implementation too, as I find that's usually a good way to be quickly confronted by all of the ways in which I'm an idiot.

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

Sure, that will help a lot with discussion.

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

You're welcome to take that as a challenge

I'm going to, but first we need to solve a different question that I forgot just now and that is probably more important. I'm sorry 🤦

That is the question of "stable format", which we both agree is one of the motivations of what we're doing. You (if I understand correctly) see this as the most important motivations and interpret stable as "stable, unless stability is impossible". I see usage in editors as the most important motivation and interpret stable as "stable, unless the runtime representation changes enough". I'm gonna go out on a limb here and define my position a bit stricter as "stable, unless schematics are no longer surjective" ("surjective" meaning that every sensible runtime state can at least be reached through schematics).

I'd like to give some context: Unity AFAIK does not have stable formats in your sense. If you change the attributes of a custom MonoBehaviour, then data is lost. In the example of changing euler angles to quaternions for rotation for instance, the data would just be lost. Also, if you open a project with a newer version of Unity it warns you that data could be lost.

Do I understand correctly that you would want bevy to behave differently?

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

I feel like if we don't agree on what's motivation schematics, then there's no way we'll agree on a design 😆

@SamPruden
Copy link

SamPruden commented Oct 22, 2022

That is the question of "stable format", which we both agree is one of the motivations of what we're doing. You (if I understand correctly) see this as the most important motivations and interpret stable as "stable, unless stability is impossible".

Stability unless stability is impossible, in which case friction should be minimized.

If a game feature gets radically redesigned, then stability from old version assets to new is probably nonsense.

If a feature's implementation gets changed (i.e. new/different systems - operating on a different ECS layout) stability should be maintained. I can write a whole new physics engine and keep the same rigidbody schematic, as long as the engine is exposing the same concepts.

Serde style migration attributes on the schematic are acceptable as a last resort.

every sensible runtime state can at least be reached through schematics

I would say it's every runtime state that the programmers want the designers to be able to create.

I actually think it's perfectly sensible to create a LockedToGroundTransform schematic, specifically intended to apply an artificial limitation to the transform. (That could function in a couple of different ways, either by being the transform, or overriding its sibling TransformSchematic). It would still be dealing with a plain TransformComponent.

In programming terms, schematics are a tool for encapsulation. Schematics are the public interface to the state - and programmers can choose what to make public, and what to keep private. Some states can only be reached "privately" - i.e. through progression of game logic. That's a good thing, because many states would be nonsensical - e.g. unnormalized direction vectors.

The only reason you need to relax the stability from full to surjective is to accomodate the problems with the readback approach.

Unity AFAIK does not have stable formats in your sense.

Sort of.

In the case of Monobehaviours, they don't. That's bad and I want to fix it. However, it's a slightly less bad problem for them.

Monobehaviours, being managed classes, are not particularly sensitive to data layout. Being stuck with an existing layout of serialized fields for backward compatibility is usually okay - you just do the equivalent of "interpreting" that data in OnEnable() to get it into the format you want to consume it in.

In Bevy we're data and performance oriented all the way down. Data layout is very sensitive, and is also something we want to be able to change freely, either as we change our designs or improve performance. Being stuck for backward compatibility reasons is a much bigger problem.

When it comes to their ECS prototype, they sort of do have a stable layer. They use Monobehaviours to serialize their assets, and an interpretation pass to turn that into ECS data. Generally, people will avoid breaking the stability of those Monobehaviours for exactly this reason.

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

The reason why this is important, is because this dictates if "you can implement readback for every sensible schematic". If the schematic component covers every combination of components that are meant to come up during play, in mathematical terms if they are surjective on to the set of states encountered during runtime, then (again mathematically speaking) there exists an inverse function.

If on the other hand a situation like example with f32 and i32 that you gave is "allowed" / encouraged, then this is not the case. Notice that here the schematic that uses i32 is not "surjective", i.e. does not cover all cases that are encountered during runtime. (If the f32 values encountered during runtime are exclusively whole numbers, then it is possible to implement readback).

I hope I'm not misinterpreting your answer, but our difference of opinion boils down to:

  • "Schematics should be able to represent everything encountered during runtime" (I actually don't think this has to be true for every schematic, but I'm gonna ignore that for simplicity)

vs

  • "Schematics should be able to represent everything encountered when editing levels"

I feel like both are very much valid. It's probably not surprising then, that I see schematics as something that can also be used during runtime while you disagree.

@SamPruden
Copy link

SamPruden commented Oct 22, 2022

Excellent summary!

My stance is that only supporting surjectivity is an extra restriction, and we shouldn't make decisions that constrain our users without very clear justification.

The fact that we disagree over whether the i32/f32 example makes sense illustrates that some people may want to be able to do that and IMO we should cater to them.

I'm not persuaded that there's anything that the readback approach does so dramatically better than the UI approach as to justify these restrictions.

I think the path forward is that we both write up and/or prototype our respective versions, then come back and compare how they look in practice - perhaps challenging each other to show how we'd deal with some challenging scenarios.

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

Now since we won't come to agree on the topic of "stable data", I'm gonna adopt your stance on that for discussions sake.

I contest that any solution that you come up with, I will be able to give you plausible real-world scenarios that readback will not be able to handle satisfactorily, but that will be handled by the UI route. You're welcome to take that as a challenge.

This of course means that I agree that there are schematics where you couldn't implement readback in a satisfactory way. But I'd still like to have readback as an optional feature of schematics, since I believe that the situation where it can't be implemented are very rare. A editor UI would find a way to deal with these rare cases, whether through "custom runtime UI", "merging" or some solution we haven't thought of yet.

I'd expect, that if someone learns of a feature in bevy, which helps in converting one representation into another, then a natural question would be "Is there also a way to convert back?". I'd like the answer to be "yes". Even if not, it makes sense to ensure that our design at least has the possibility for conversions the other way.

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

challenging each other to show how we'd deal with some challenging scenarios

Haha, not sure how productive that would be, but sounds fun. I imagine that - having different goals as well as different scopes (I guess you're also going to talk about UI in your RFC) - we just wouldn't agree on which is better xD

@SamPruden
Copy link

Github says our discussion is too robust. I got locked out for 10 minutes by a rate limit! 😂

Yep I agree with all of that. My challenge to myself is to make the UI proposal good enough to persuade you that readback is obsolete. I reckon I've got a 50% shot at that.

I'm going to take some time to collect my thoughts and come back with a draft RFC and/or prototype - probably in a couple of days if I had to guess.

@MDeiml
Copy link
Author

MDeiml commented Oct 22, 2022

Didn't even know github had a rate limit for discussions xD

Sure, take your time.

@Diddykonga
Copy link

I'm very unknowledgeable about the whole of the conversation/discussion, but I figured I would give my view on the subject anyways.

Schematics - Editor or Artist representation of game data/systems.
Runtime - Runtime or Programmer representation of game data/systems.

Two easy examples brought up so far was:

  1. Schematic - Integer grid-based positions for the Editor/Artists.
    Runtime - Float animation-based positions for the Runtime/Graphics.
    
  2. Schematic - Vector3 euler-based rotations for the Editor/Artists.
    Runtime - Quaternion imaginary-based rotations for the Runtime/Graphics.
    

The idea of stability was brought up for Schematics and while they would/should provide stability from changes to the Runtime representation, Schematics themselves would/should not be stable because an Artist/Editor-user may need to change that representation as they see fit.

How Archetype Invariants + Entity Kinds would work with Schematics I am unaware, perhaps these are Runtime constraints only.

@SamPruden
Copy link

Schematics themselves would/should not be stable because an Artist/Editor-user may need to change that representation as they see fit.

Just to check that I'm understanding you @Diddykonga, do you mean that a designer should be able to choose e.g. whether to input a rotation as Euler angles or as a quaternion?

If so, I think that's best dealt with one of two ways:

  1. The UI lets you type the numbers in either format, but it always serializes is one canonical format.
  2. Represent the rotation as a enum Rotation { Euler(Vec3), Quaternion(Vec4) }.

Both of these methods could be used with either myself or @MDeiml's preferred approaches.

@Diddykonga
Copy link

Diddykonga commented Oct 24, 2022

Just to check that I'm understanding you @Diddykonga, do you mean that a designer should be able to choose e.g. whether to input a rotation as Euler angles or as a quaternion?

My apologies, what I meant is that there would always be one for editor and one for runtime, but that both should have mutability/migration-ability, which was why I said it is unstable. Although I may be misunderstanding what stable is in this context ¯_(ツ)_/¯

So as long as the Editor Schematic is Euler Angles, that's what a designer/artist would use, but should also be able to change it without changing the Runtime Schematic, to support designer/artist workflows more precisely.

@MDeiml
Copy link
Author

MDeiml commented Oct 24, 2022

I think you understood stability right. "Stability" would mean that the actual data structures used for the schematic representation don't change at all under any circumstances.

If I understand correctly you're actually mentioning another point that we haven't yet talked about. Which is that artists / designers might want to intentionally change the data representation they see in the UI.

An example would be if the developer / non-designer initially adds rotation to the UI as quaternions. In this situation a designer would probably complain that they don't understand quaternions and would rather like to input euler angles. The developer would then need to change the schematics to meet that request.

So I guess optimizing for designer workflow is contrary to the schematic representation being stable...

@SamPruden
Copy link

SamPruden commented Oct 24, 2022

In practice, what matters is long term stability. I want to be able to make runtime implementation changes without worrying about breaking assets that were built months ago. One would hope that the way that example would resolve itself is that the arist would complain as soon as the schematic was given to them, and changes would be made before a backlog of content was built on the bad version. Stability only matters once the team start relying on it.

Technically stable doesn't have to mean that the schematic struct layout can't change at all - the real requirement is that any previous version can sensibly deserialize into the current version.

If we assume that serialization is being done by serde or something with similar capabilities, then I believe that in the absolute worst case it should be possible to provide a manual implementation of Deserialize for the schematic type that deals with arbitrary migrations - we should probably include a version tag in the serialization format to facilitate this.

Having to do that is tedious. Supporting deserialization from multiple prior versions of the same struct is error prone, and doing so responsibily probably requires writing unit tests for the migration. Nobody wants to do that for their supposedly quick & seamless schematics, but the fact that this is possible means that even the worst case isn't a catastrophe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants