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

Generalize DiGraph/UnGraph 📊 #18247

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ItsDoot
Copy link
Contributor

@ItsDoot ItsDoot commented Mar 11, 2025

Objective

As the first step towards more modular schedule graph structures, we first need to generalize the foundational pieces: the Graph data structure and related algorithms.

One might ask: "what are 'more modular schedule graph structures'?" I've come across quite a few use cases that can almost be fulfilled with a Schedule, but have different-enough requirements that render Schedule incompatible as-is:

  • My WIP utility AI library needs a graph of read-only systems while linking system output values as inputs to other systems.
  • bevy_render's RenderGraph is already somewhat Schedule-like, but requires all nodes to be read-only and currently only allows a single query to be performed.
  • bevy_render's Extract schedule could be statically enforced to be read-only, which would allow its executor to skip access conflict checks.
  • Occasionally users in the Discord will show interest in statically-dispatched schedules, where a fork-join sequence can be modeled at compile time.

Therefore, by providing modular and generic building blocks for building Schedule-shaped things, we can satisfy each of the above and more. And since most of these are likely to make use of DiGraph or UnGraph, generalizing them is a good start.

One might ask: "what does this give us over petgraph?" A couple things:

  • petgraph is not currently no_std, which is a prerequisite to pull it in now that bevy_ecs is no_std.
  • The associated types on GraphNodeId allows us to use memory-efficient forms for edge and neighbor information, which currently saves us 8 bytes for each edge pair and each neighbor+direction.

Solution

Replaces the hardcoded usage of NodeId with an implementable GraphNodeId trait in DiGraph/UnGraph and associated algorithms. Algorithms implemented as part of ScheduleGraph will be tackled in a future PR in order to keep this one small.

Testing

Current tests are being reused.


Migration Guide

  • Bare DiGraph or UnGraph usage must be changed to DiGraph<NodeId> or UnGraph<NodeId>, respectively.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 11, 2025
@ItsDoot ItsDoot added C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Code-Quality A section of code that is hard to understand or change labels Mar 11, 2025
@alice-i-cecile
Copy link
Member

Can you say more about why this will be useful for your long-term plans? IIRC we did the opposite thing when vendoring petgraph 😂

@ItsDoot
Copy link
Contributor Author

ItsDoot commented Mar 11, 2025

Can you say more about why this will be useful for your long-term plans? IIRC we did the opposite thing when vendoring petgraph 😂

Context added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants