Skip to content

Conversation

@alice-i-cecile
Copy link
Member

Covers the issue reported in bevyengine/bevy#18778.

Co-authored-by: JaySpruce <[email protected]>
@alice-i-cecile alice-i-cecile enabled auto-merge April 9, 2025 20:01
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review Ready for a maintainer to consider for merging and removed S-Needs-Review labels Apr 9, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 9, 2025
Merged via the queue into bevyengine:main with commit cc24e41 Apr 9, 2025
10 checks passed
@alice-i-cecile alice-i-cecile deleted the never-closures branch April 9, 2025 20:41
github-merge-queue bot pushed a commit to bevyengine/bevy that referenced this pull request Apr 14, 2025
…raits (#18804)

# Objective

After #17967, closures which always panic no longer satisfy various Bevy
traits. Principally, this affects observers, systems and commands.

While this may seem pointless (systems which always panic are kind of
useless), it is distinctly annoying when using the `todo!` macro, or
when writing tests that should panic.

Fixes #18778.

## Solution

- Add failing tests to demonstrate the problem
- Add the trick from
[`never_say_never`](https://docs.rs/never-say-never/latest/never_say_never/)
to name the `!` type on stable Rust
- Write looots of docs explaining what the heck is going on and why
we've done this terrible thing

## To do

Unfortunately I couldn't figure out how to avoid conflicting impls, and
I am out of time for today, the week and uh the week after that.
Vacation! If you feel like finishing this for me, please submit PRs to
my branch and I can review and press the button for it while I'm off.

Unless you're Cart, in which case you have write permissions to my
branch!

- [ ] fix for commands
- [ ] fix for systems
- [ ] fix for observers
- [ ] revert bevyengine/bevy-website#2092

## Testing

I've added a compile test for these failure cases and a few adjacent
non-failing cases (with explicit return types).

---------

Co-authored-by: Carter Anderson <[email protected]>
mockersf pushed a commit to bevyengine/bevy that referenced this pull request Apr 14, 2025
…raits (#18804)

# Objective

After #17967, closures which always panic no longer satisfy various Bevy
traits. Principally, this affects observers, systems and commands.

While this may seem pointless (systems which always panic are kind of
useless), it is distinctly annoying when using the `todo!` macro, or
when writing tests that should panic.

Fixes #18778.

## Solution

- Add failing tests to demonstrate the problem
- Add the trick from
[`never_say_never`](https://docs.rs/never-say-never/latest/never_say_never/)
to name the `!` type on stable Rust
- Write looots of docs explaining what the heck is going on and why
we've done this terrible thing

## To do

Unfortunately I couldn't figure out how to avoid conflicting impls, and
I am out of time for today, the week and uh the week after that.
Vacation! If you feel like finishing this for me, please submit PRs to
my branch and I can review and press the button for it while I'm off.

Unless you're Cart, in which case you have write permissions to my
branch!

- [ ] fix for commands
- [ ] fix for systems
- [ ] fix for observers
- [ ] revert bevyengine/bevy-website#2092

## Testing

I've added a compile test for these failure cases and a few adjacent
non-failing cases (with explicit return types).

---------

Co-authored-by: Carter Anderson <[email protected]>
jf908 pushed a commit to jf908/bevy that referenced this pull request May 12, 2025
…raits (bevyengine#18804)

# Objective

After bevyengine#17967, closures which always panic no longer satisfy various Bevy
traits. Principally, this affects observers, systems and commands.

While this may seem pointless (systems which always panic are kind of
useless), it is distinctly annoying when using the `todo!` macro, or
when writing tests that should panic.

Fixes bevyengine#18778.

## Solution

- Add failing tests to demonstrate the problem
- Add the trick from
[`never_say_never`](https://docs.rs/never-say-never/latest/never_say_never/)
to name the `!` type on stable Rust
- Write looots of docs explaining what the heck is going on and why
we've done this terrible thing

## To do

Unfortunately I couldn't figure out how to avoid conflicting impls, and
I am out of time for today, the week and uh the week after that.
Vacation! If you feel like finishing this for me, please submit PRs to
my branch and I can review and press the button for it while I'm off.

Unless you're Cart, in which case you have write permissions to my
branch!

- [ ] fix for commands
- [ ] fix for systems
- [ ] fix for observers
- [ ] revert bevyengine/bevy-website#2092

## Testing

I've added a compile test for these failure cases and a few adjacent
non-failing cases (with explicit return types).

---------

Co-authored-by: Carter Anderson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Migration Guides S-Ready-For-Final-Review Ready for a maintainer to consider for merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants