Skip to content

System order ambiguities should be grouped into related sets #7443

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

Open
alice-i-cecile opened this issue Jan 31, 2023 · 3 comments
Open

System order ambiguities should be grouped into related sets #7443

alice-i-cecile opened this issue Jan 31, 2023 · 3 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Blocked This cannot move forward until something else changes

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

When multiple systems are ambiguous with each other, each pair is currently reported independently.

This is challenging to read and reason about, as the correct fix often involves thinking about the group as a whole.

What solution would you like?

If a group of systems are all ambiguous with each other, report this as a single ambiguity.

What alternative(s) have you considered?

We could instead group agglomeratively: if A is ambiguous with both B and C (but B and C are fine), group them.

This is probably incorrect: those errors tend to be due to several independent problems.

Additional context

Please don't do this until #7267 is merged for the sake of my sanity.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events S-Blocked This cannot move forward until something else changes C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jan 31, 2023
@alice-i-cecile alice-i-cecile moved this to Blocked in Scheduling Jan 31, 2023
@joseph-gio
Copy link
Member

I attempted this before in #6071, though I ran into issues with non-deterministic grouping. This code might be helpful to anyone trying this in the future.

@maniwani
Copy link
Contributor

maniwani commented Feb 1, 2023

To group by component, we'd have to take the ambiguities, insert them as edges into a collection of undirected graphs (one per contested component type), and then run a maximal clique detection algorithm on each graph.

Then, knowing all groups of "systems that conflict on component T at same time", we could either:

  • just print them to the console
  • try to merge them further first
    • sort and hash each Vec<NodeId> to build a HashMap<Vec<NodeId>, Vec<ComponentId>>

@maniwani
Copy link
Contributor

maniwani commented Feb 1, 2023

Grouping by system is another option. We can skip "bar conflicts with foo" if we already logged "foo conflicts with bar".

foo
conflicts with:
    bar on [A, B, C]
    baz on [World]

bar
conflicts with:
    baz on [World]

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 S-Blocked This cannot move forward until something else changes
Projects
Status: Needs Implementation
Development

No branches or pull requests

3 participants