Remove remaining internal use of `!Send` resources #18386

joshua-holmes · 2025-03-18T03:09:29Z

Objective

Remaining work for and closes #17682. First half of work for that issue was completed in PR 17730. However, the rest of the work ended up getting blocked because we needed a way of forcing systems to run on the main thread without the use of !Send resources. That was unblocked by PR 18301.

This work should finish unblocking the resources-as-components effort.

Testing

Ran several examples using my Linux machine, just to make sure things are working as expected and no surprises pop up.

alice-i-cecile · 2025-03-18T03:15:59Z

I'm going to want @maniwani's eyes on this, hence the X-Controversial label.

crates/bevy_winit/src/state.rs

joshua-holmes · 2025-03-20T00:02:47Z

The work on this PR is complete, but we are waiting for NonSendMarker PR to get merged because this PR depends on that one. Once it's merged, I will rebase and fix up the git history so the commits related to creating NonSendMarker do not appear in this PR.

crates/bevy_winit/src/lib.rs

crates/bevy_winit/src/state.rs

crates/bevy_winit/src/accessibility.rs

crates/bevy_winit/src/lib.rs

alice-i-cecile · 2025-03-23T21:37:57Z

Merging the linked PR for you :)

Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>

…DAPTERS` Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>

alice-i-cecile · 2025-05-05T23:26:37Z

Thank you so much for tackling this! I'm really happy that this is finally done, now we just need to remove the old NonSend API surface and revive the resources-as-components work.

alice-i-cecile · 2025-05-06T06:11:33Z

Back to Waiting-on-Author until CI issues are resolved :(

# Objective Remaining work for and closes bevyengine#17682. First half of work for that issue was completed in [PR 17730](bevyengine#17730). However, the rest of the work ended up getting blocked because we needed a way of forcing systems to run on the main thread without the use of `!Send` resources. That was unblocked by [PR 18301](bevyengine#18301). This work should finish unblocking the resources-as-components effort. # Testing Ran several examples using my Linux machine, just to make sure things are working as expected and no surprises pop up. --------- Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> Co-authored-by: François Mockers <francois.mockers@vleue.com>

IceSentry · 2025-06-08T18:15:19Z

Where's the migration guide for this PR? Also, why is WINIT_WINDOWS private? That's a huge breaking change.

joshua-holmes · 2025-06-10T04:52:42Z

There are no changes that Bevy users should need to implement on their own. One can still use !Send resources. This PR only removes Bevy's internal use of !Send resources, so a migration guide does not seem necessary.

@IceSentry can you elaborate why WINIT_WINDOWS should be public? If we need to use it somewhere else in Bevy, I would suggest making it public as a part of the PR. My understanding is that a Bevy user would never need to use this directly. I believe we talked about this in the Discord. Maybe @alice-i-cecile can shed some light on this though

IceSentry · 2025-06-10T05:42:34Z

There are no changes that Bevy users should need to implement on their own. One can still use !Send resources. This PR only removes Bevy's internal use of !Send resources, so a migration guide does not seem necessary.

Removing something that used to be public is a breaking change and needs a migration guide to inform users that this change happened even if it's just telling them they can't use it anymore. Also, if people were using those NonSend resources and still want to access them they need to know how to do it without having to dig through the codebase. Knowing that you need to add a special marker and access a global variable is not obvious.

can you elaborate why WINIT_WINDOWS should be public?

I was working on a custom renderer and I needed to access the raw WinitWindow.

I was away for a while so I may have missed the discussion on discord.

I recognize this is a very niche use case, but I was hit with it and it took me a while to figure out.

alice-i-cecile · 2025-06-10T16:30:31Z

Indeed: this 100% needs to be made public again and have a proper migration guide. Sorry for not spotting that in review.

joshua-holmes · 2025-06-11T00:57:37Z

I see. Yes, this makes sense. Apologies for missing this. I'll work on a migration guide and make the resource replacements public.

After removing `!Send` resources, `GILRS` and `WINIT_WINDOWS` were not made public, which is a breaking change. This was brought up in a [comment on that PR](#18386 (comment)). This PR makes them public. Fixes #19540.

hymm self-requested a review March 18, 2025 18:17

hymm reviewed Mar 18, 2025

View reviewed changes

crates/bevy_winit/src/state.rs Outdated Show resolved Hide resolved

hymm reviewed Mar 18, 2025

View reviewed changes

crates/bevy_winit/src/state.rs Outdated Show resolved Hide resolved

joshua-holmes changed the title ~~Remove remaining internal use of !Send~~ Remove remaining internal use of !Send resources Mar 19, 2025

chescock approved these changes Mar 20, 2025

View reviewed changes

crates/bevy_winit/src/lib.rs Outdated Show resolved Hide resolved

crates/bevy_winit/src/state.rs Outdated Show resolved Hide resolved

crates/bevy_winit/src/accessibility.rs Outdated Show resolved Hide resolved

crates/bevy_winit/src/lib.rs Outdated Show resolved Hide resolved

joshua-holmes force-pushed the no-more-non-send branch 3 times, most recently from 8b6d428 to ec82c5f Compare March 20, 2025 01:21

maniwani approved these changes Mar 23, 2025

View reviewed changes

alice-i-cecile added this to the 0.17 milestone Mar 23, 2025

joshua-holmes force-pushed the no-more-non-send branch from ad60838 to ec82c5f Compare March 24, 2025 20:40

joshua-holmes and others added 9 commits March 24, 2025 14:17

Remove WinitWindows non-send res

899926d

Replace AccessKitAdapter non-send resource

707bc1e

Remove unnecessary NonSendMarkers

dc1b015

Expose visible to scope outside of .with()

e67d675

Simplify the use of .with()

3f014d1

Destructure windows

bfa24cc

Remove unnecessary mutability

bcc56ac

Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>

Remove unnecessary NonSendMarker

aa14237

Use ::default() when initializing WINIT_WINDOWS and `ACCESS_KIT_A…

7bce0d2

…DAPTERS` Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>

joshua-holmes force-pushed the no-more-non-send branch from ec82c5f to 7bce0d2 Compare March 24, 2025 21:18

Import NonSendMarker

f6f681e

joshua-holmes marked this pull request as ready for review March 24, 2025 21:24

alice-i-cecile added the X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers label May 5, 2025

alice-i-cecile added this pull request to the merge queue May 5, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 5, 2025

alice-i-cecile added this pull request to the merge queue May 6, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 6, 2025

alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels May 6, 2025

Fix Android compilation bugs

3838f85

alice-i-cecile added this pull request to the merge queue May 6, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 6, 2025

mockersf added 5 commits May 6, 2025 23:19

remove options from thread locals

ac162fb

fix compilation on android

955fe3e

manual run of systems outside of borrowed thread locals

b8d7c67

expect, not allow

0b84ea8

cfg_attr the expect

2aacd73

alice-i-cecile added this pull request to the merge queue May 6, 2025

Merged via the queue into bevyengine:main with commit 770f10b May 6, 2025
32 checks passed

maniwani mentioned this pull request May 20, 2025

Remote entity reservation v9 #18670

Open

IceSentry mentioned this pull request Jun 8, 2025

Missing migration guide for WINIT_WINDOWS #19540

Closed

joshua-holmes mentioned this pull request Jun 11, 2025

Make GILRS and WINIT_WINDOWS public #19575

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Remove remaining internal use of `!Send` resources #18386

Remove remaining internal use of `!Send` resources #18386

joshua-holmes commented Mar 18, 2025 •

edited by alice-i-cecile

Loading

Uh oh!

alice-i-cecile commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!

joshua-holmes commented Mar 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alice-i-cecile commented Mar 23, 2025

Uh oh!

alice-i-cecile commented May 5, 2025

Uh oh!

Uh oh!

Uh oh!

alice-i-cecile commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

IceSentry commented Jun 8, 2025

Uh oh!

joshua-holmes commented Jun 10, 2025

Uh oh!

IceSentry commented Jun 10, 2025

Uh oh!

alice-i-cecile commented Jun 10, 2025 •

edited

Loading

Uh oh!

joshua-holmes commented Jun 11, 2025

Uh oh!

Uh oh!

Uh oh!

Remove remaining internal use of !Send resources #18386

Remove remaining internal use of !Send resources #18386

Conversation

joshua-holmes commented Mar 18, 2025 • edited by alice-i-cecile Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Testing

Uh oh!

alice-i-cecile commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!

joshua-holmes commented Mar 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alice-i-cecile commented Mar 23, 2025

Uh oh!

alice-i-cecile commented May 5, 2025

Uh oh!

Uh oh!

Uh oh!

alice-i-cecile commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

IceSentry commented Jun 8, 2025

Uh oh!

joshua-holmes commented Jun 10, 2025

Uh oh!

IceSentry commented Jun 10, 2025

Uh oh!

alice-i-cecile commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshua-holmes commented Jun 11, 2025

Uh oh!

Uh oh!

Remove remaining internal use of `!Send` resources #18386

Remove remaining internal use of `!Send` resources #18386

joshua-holmes commented Mar 18, 2025 •

edited by alice-i-cecile

Loading

alice-i-cecile commented Jun 10, 2025 •

edited

Loading