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.

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

Thirsty example broken in 0.19.0 #97

Closed
ekalosak opened this issue May 15, 2024 · 11 comments
Closed

Thirsty example broken in 0.19.0 #97

ekalosak opened this issue May 15, 2024 · 11 comments

Comments

@ekalosak
Copy link
Contributor

The thirst example is broken using:

  • big brain 0.19.0
  • bevy 0.13.2
  • stable-x86_64-unknown-linux-gnu - rustc 1.78.0 (9b00956e5 2024-04-29)

Minimum reproducible example

https://github.com/zkat/big-brain/blob/main/examples/thirst.rs

Error text:

error[E0053]: method `build` has an incompatible type for trait
   --> examples/bigbrain.rs:108:35
    |
108 | #[derive(Clone, Component, Debug, ScorerBuilder)]
    |                                   ^^^^^^^^^^^^^ expected `Commands<'_, '_>`, found a different `Commands<'_, '_>`
    |
    = note: expected signature `fn(&Thirsty, &mut bevy_ecs::system::commands::Commands<'_, '_>, bevy_ecs::entity::Entity, bevy_ecs::entity::Entity)`
               found signature `fn(&Thirsty, &mut bevy::prelude::Commands<'_, '_>, bevy::prelude::Entity, bevy::prelude::Entity)`
    = note: this error originates in the derive macro `ScorerBuilder` (in Nightly builds, run with -Z macro-backtrace for more info)

Potential change implicated

#93 (comment)

@stargazing-dino
Copy link
Contributor

So is the error that we're expecting a Command defined in a different module or something?

If that's the case, we should be able to change this line to use the bevy_ecs import one and that'd resolve it no?

@ekalosak
Copy link
Contributor Author

ekalosak commented May 15, 2024

It's weird, right?

Somehow the bevy::prelude::Command is not the bevy::ecs::system::Command.

This project's derive (https://github.com/zkat/big-brain/blob/main/derive/src/action.rs#L58) uses that latter. The code I used to replicate (clean project, those two dependencies, verbatim copy of thirsty.rs) uses the bevy::prelude::* which is the former.

Also, worth noting that cloning the project and checking the example (ostensibly with dev-dependencies) does not cause this issue, and the example works in that context.

More clues and notes

bevy_ecs prelude

https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/lib.rs
re-publishes its ecs::system::Command etc

@ekalosak
Copy link
Contributor Author

e.g. this, @stargazing-dino #99 ?

@stargazing-dino
Copy link
Contributor

stargazing-dino commented May 15, 2024

Also, worth noting that cloning the project and checking the example (ostensibly with dev-dependencies) does not cause this issue, and the example works in that context.

Super weird.

Yeah, just ran it and didn't get any errors myself. also upgraded to latest stable rust and still nothing.

Your PR looks correct though. Left an approval. You'll need @zkat to run CI to verify tho

@ekalosak
Copy link
Contributor Author

I should refine my instructions for minimal replication, sorry for the ambiguity:

cargo new foo && cd foo
cargo add bevy big-brain
# copy the thirst example to examples/bigbrain.rs
cargo check —examples bigbrain

Crucially, the example needs to be built using the big brain crate as an external dependency to cause the error. I’m guessing it has something to do with the difference between big brains dependencies and its dev-dependencies, but that’s only a hunch. I’m still not sure what the root cause is and would appreciate any insight, if the proposed fix is not sufficient.

@stargazing-dino
Copy link
Contributor

Yup, I reporduced locally. #99 fixes it for me ^^

@ekalosak
Copy link
Contributor Author

ekalosak commented May 16, 2024

Could be more of a legitimate RCA: https://users.rust-lang.org/t/are-re-exported-types-distinct-from-the-original-types/63911/2

Installing Bevy 0.13 could introduce the “interesting” error where the compiler claims bevy::ecs::system::Command and bevy::prelude::Command are different upon encountering the Bevy 0.12 one that big-brain 0.19 uses viz the 0.13 one in the user code.

The change in #99 appears to fix. But maybe just releasing 0.20 with Bevy 0.13 would as well..? What is bothersome is that, were the preceding logic correct, #99 shouldn’t have fixed…

@ekalosak
Copy link
Contributor Author

More on this issue: https://github.com/rust-lang/rfcs/blob/master/text/1977-public-private-dependencies.md

Continuing from above: or, duh, because the thing that fixes in #99 is the bump to Bevy 0.13 not the silly change to the import path for ecs::Command

@zkat
Copy link
Owner

zkat commented May 17, 2024

that's... all really weird.

@ekalosak
Copy link
Contributor Author

ekalosak commented May 17, 2024

Yeah but it's weird because it's my rambling, I think. Root cause is I believe quite simple. The root cause is that latest (0.19) Big Brain is using Bevy 0.12 and user code is using Bevy 0.13. Thus the compiler finds "different Commands<', '>"

It just doesn't specify how they're different - the import path is a red herring - they are semantically versioned different.

Will be trivially "fixed" with releasing Big Brain 0.20 - would be great to have a better compiler message but at least TIL.

@zkat
Copy link
Owner

zkat commented May 17, 2024

0.20.0 is released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants