Skip to content

Remove OnUpdate system set#8260

Merged
cart merged 5 commits intobevyengine:mainfrom
lewiszlw:remove-OnUpdate
Apr 4, 2023
Merged

Remove OnUpdate system set#8260
cart merged 5 commits intobevyengine:mainfrom
lewiszlw:remove-OnUpdate

Conversation

@lewiszlw
Copy link
Member

@lewiszlw lewiszlw commented Mar 30, 2023 •
edited
Loading

Objective

Solution

  • Replace OnUpdate with run_if(in_state(xxx)).

Migration Guide

  • Replace OnUpdate with run_if(in_state(xxx)).

@github-actions
Copy link
Contributor

github-actions bot commented Mar 30, 2023

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

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR labels Mar 30, 2023
alice-i-cecile
alice-i-cecile reviewed Mar 30, 2023
View reviewed changes
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Mar 30, 2023

Let's merge #7676 first, then this will no longer be controversial.

@alice-i-cecile alice-i-cecile removed the X-Controversial There is active debate or serious implications around merging this PR label Mar 30, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 30, 2023
cart
cart approved these changes Mar 30, 2023
View reviewed changes
Copy link
Member

@cart cart left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Lets hold off until #7676 is merged (its currently in the merge queue). After that merge, we should adapt the distributed_run_if cases to use run_if

@alice-i-cecile alice-i-cecile removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 30, 2023
alice-i-cecile
alice-i-cecile reviewed Mar 30, 2023
View reviewed changes
alice-i-cecile
alice-i-cecile reviewed Mar 30, 2023
View reviewed changes
alice-i-cecile
alice-i-cecile reviewed Mar 30, 2023
View reviewed changes
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Mar 30, 2023 •
edited
Loading

@lewiszlw could you look over the changes I made to remove distributive_run_if + the merge conflict resolution? :)

@lewiszlw
Copy link
Member Author

lewiszlw commented Mar 31, 2023

Yeah, definitely agreed.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 31, 2023
@cart cart added this pull request to the merge queue Apr 3, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 3, 2023
@cart cart added this pull request to the merge queue Apr 4, 2023
Merged via the queue into bevyengine:main with commit 5c7abb0 Apr 4, 2023
@joseph-gio joseph-gio mentioned this pull request Apr 12, 2023
Closed
@hymm hymm mentioned this pull request Apr 16, 2023
Closed
@andriyDev andriyDev mentioned this pull request Jun 12, 2023
Closed
This was referenced Jun 28, 2023
Closed
Merged
@Selene-Amanita Selene-Amanita added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jul 10, 2023
Estus-Dev pushed a commit to Estus-Dev/bevy that referenced this pull request Jul 10, 2023
@lewiszlw @alice-i-cecile
Remove OnUpdate system set (bevyengine#8260)
4441265 
# Objective

- Fixes bevyengine#8239.

## Solution

- Replace `OnUpdate` with `run_if(in_state(xxx))`.

---

## Migration Guide

- Replace `OnUpdate` with `run_if(in_state(xxx))`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@cart cart cart approved these changes

@maniwani maniwani Awaiting requested review from maniwani

+1 more reviewer

@joseph-gio joseph-gio joseph-gio approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove OnUpdate system set

6 participants

@lewiszlw @alice-i-cecile @cart @joseph-gio @james7132 @Selene-Amanita