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.

Sign up for GitHub

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

Jump to bottom

Determine plugin build order with a dependency graph #7839

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Determine plugin build order with a dependency graph #7839

wants to merge 1 commit into from

Conversation

maniwani
Copy link
Contributor

@maniwani maniwani commented Feb 28, 2023 •
edited
Loading

Objective

  • Fix Plugin Dependencies #69 (👈👈😎).
  • Provide a different way for one plugin to spoof/substitute another (i.e. not "these plugins have the same string name").

Solution

  • Think of plugin as the ECS equivalent of a crate.
  • Use TypeId to differentiate plugins.
  • Remove ability to override uniqueness. Only one plugin with a given TypeId to be added to an App.
    • Can maybe silence the error if two instances are configured the same.
  • Defer building plugins until after they've all been added.
  • Have plugins declare which plugins they depend on (or substitute) via TypeId.
  • Construct a directed graph from those relationships.
  • Topologically sort the graph and build plugins in that order.

Migration Guide

TODO

@maniwani maniwani added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-App Bevy apps and plugins M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Feb 28, 2023
@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Feb 28, 2023
@alice-i-cecile alice-i-cecile added this to the 0.11 milestone Feb 28, 2023
@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@alice-i-cecile alice-i-cecile removed this from the 0.11 milestone Jun 19, 2023
@NthTensor
Copy link
Contributor

Great work! I'm working on getting this updated and passing CI so it can be evaluated for merge. Looks largely feature complete. Were there any missing peaces I should know about?

@maniwani maniwani added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Jan 5, 2024
@NthTensor NthTensor mentioned this pull request Jan 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-App Bevy apps and plugins 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-Adopt-Me The original PR author has no intent to complete this work. Pick me up! X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Plugin Dependencies
3 participants
@maniwani @NthTensor @alice-i-cecile