Skip to content

Conversation

@alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Jan 14, 2022

RENDERED

This RFC is now ready for final review. Feedback is welcome from users at all levels! Do you think this design easy-to-understand, useful and correct? How could the proposed API be improved?

TL; DR

  • Systems: now stored in the World
  • Exclusive systems: no longer special-cased
  • System sets: now just pure metadata, define order and conditions of groups of systems
  • Schedules: now the executable instances of system sets, also stored in the World
  • Run criteria: replaced by combineable, non-mutating, bool-returning functions
  • States: now powered by new exclusive system + schedule combo instead of run criteria
  • Fixed timestep: now powered by new exclusive system + schedule combo instead of run criteria
  • Commands: now applied with an exclusive system
  • Stages: yeet
  • All of the awful bugs with stages, states, and run criteria: replaced with new, yet-to-be-discovered bugs!

Plus, some shiny new toys: running systems in commands and modifying schedules at runtime!

Prototype

iyes_loopless is a Bevy 0.6-compatible third-party implementation of the core ideas of how this RFC handles run criteria and states. Go try it out!

Context

This is the culmination of many months of discussion, work, debate and mind-melting from @maniwani (Joy), @hymm (WrongShoe) and myself; they should be considered equal coauthors on this RFC. If you're curious about the background, check out bevyengine/bevy#2801 to get a sense of the tangled mess we're trying to clean up.

Special thanks to @Ratysz and the rest of the ECS crew for all of the feedback and guidance throughout the process.

Meta

What is the best way to handle the migration process from an organizational perspective?

  • Get an RFC approved, and then merge a massive PR developed on a off-org branch?
    • Straightforward, but very hard to review
    • Risks divergence and merge conflicts
  • Use an official branch with delegated authority?
    • Easier to review, requires delegation, ultimately creates a large PR that needs to be reviewed
    • Risks divergence and merge conflicts
  • Develop bevy_schedule3 on the main branch as a partial fork of bevy_ecs?
    • Some annoying machinery to set up
    • Requires more delegation and trust to ECS team
    • Avoids divergence and merge conflicts
    • Clutters the main branch

Copy link

@inodentry inodentry left a comment

Choose a reason for hiding this comment

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

It is amazing that the "stageless" discussions are finally boiling down to something cohesive and concrete! Great work everyone (and alice for writing it up)!

Most of my comments are pretty minor, or asking for clarification, not objections. There are a few things that I feel are important to discuss.

@inodentry
Copy link

I worry that this may end up clunky and frustrating to use without automatic management/insertion of the commands and states flushing systems. I'd love to be proven wrong.

@alice-i-cecile
Copy link
Member Author

I worry that this may end up clunky and frustrating to use without automatic management/insertion of the commands and states flushing systems.

In all honesty, I agree. I wanted to start with the more straightforward, explicit design for the first draft. I suspect that automatic inference will both be less frustrating and faster for end users though.

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

First section read through; all seems reasonably sensible so far.

Up next: System ordering

Since GitHub's UX is so terrible, please ping me in discord if you reply; I probably won't be able to see it otherwise :(

@maniwani
Copy link

I worry that this may end up clunky and frustrating to use without automatic management/insertion of the flushing systems.

Frustration over stages sacrificing potential concurrency was a big motivator for this effort. IMO explicit flush systems are hands down better than implicit stage flushes.

To clarify, this isn't "manual insertion forever." Automatic insertion can build on top of this. It's just going to require more system annotation (or type system magic), so the schedule builder can know which components any system can affect with commands.

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

I'm not planning on reviewing implementation today; I'm fairly sure this can be implemented.

Copy link

@hymm hymm left a comment

Choose a reason for hiding this comment

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

Regarding automatic sync points:

Alternative proposal for sync points

In this model sync points are still a special exclusive system in the executor. We would reserve a ArchetypeComponentId to mark the world state as dirty when a system uses commands. This would ideally when commands are actually issued, but if it's just when a system runs that has a Commands that would be ok. Once commands are synced the system should use another ArchetypeComponentId to mark the archetypes are dirty. This would ideally happen only if archetypes have been changed, but might need to be done whenever an exclusive system is run. Parallel systems are not allowed to run when archetypes are dirty. The function to update archetype would need to be run on all systems before the bit is cleared. generalization of this could be used for reactive systems, and would be needed if users want to customize the behavior of syncing commands.

System ordering

I feel like the default behavior should be to automatically insert a sync requirement between systems when the first system issues commands. This would obviously require "automatic" sync points. Then add some type of opt out behavior only if users seem to need it.

Edit: Overall not sure about how I feel about this RFC. It does solve a lot of things, but I think I still like my ideas for structured concurrency better. But I guess I need to write an RFC for that. It would be similar to this one in a lot of places, but different in a few key things.

Copy link

@Nilirad Nilirad left a comment

Choose a reason for hiding this comment

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

I limited myself to only review the user-facing explanation. In general, it is magnificent. Good job to everyone that contributed to this design!

The system ordering API looks a a bit difficult to read though. Maybe, since we're propending towards graph visualization tools in the future, using a graph-based API would probably be a wise choice, like the one proposed in bevyengine/bevy#2381.

Prefer System Tuples, replace in_schedule, aggressive add_systems merging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project
Archived in project

Development

Successfully merging this pull request may close these issues.