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

Merge bevy_app into bevy_ecs #3786

Open
alice-i-cecile opened this issue Jan 27, 2022 · 8 comments
Open

Merge bevy_app into bevy_ecs #3786

alice-i-cecile opened this issue Jan 27, 2022 · 8 comments
Labels
A-App Bevy apps and plugins A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Docs An addition or correction to our documentation X-Controversial There is active debate or serious implications around merging this PR

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jan 27, 2022

bevy_app is a thin layer over bevy_ecs, and is nearly impossible to use on its own.
Moreover, most standalone uses of bevy_ecs would be better suited by using the tools provided with bevy_app; particularly if we had better documentation around custom runners and manually controlling the App loop.

For example: bevy_app re-exports all of bevy_ecs::event.

This is pointless, because bevy_ecs is an explicit dependency of all users of bevy_app.
Furthermore, it creates confusion in the main Bevy docs, since the bevy_app version is listed.

This re-export can and should just be removed.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change A-App Bevy apps and plugins labels Jan 27, 2022
@DJMcNab
Copy link
Member

DJMcNab commented Jan 27, 2022

The alternative is to make bevy_app export bevy_ecs somehow; bevy_app is meaningless without bevy_ecs anyways.

In some ways, I still think merging them probably makes sense; I can't recall a single usage of bevy_ecs without bevy_app which hasn't been misguided.

@CGMossa
Copy link
Contributor

CGMossa commented Jan 27, 2022

There are people that want to simply use bevy_ecs on its own.
There are efforts put forth to de-couple bevy crates, e.g. #2931 comes to mind also.

@alice-i-cecile
Copy link
Member Author

In some ways, I still think merging them probably makes sense; I can't recall a single usage of bevy_ecs without bevy_app which hasn't been misguided.

Yeah, this is about my perspective too. bevy_app is a very thin layer, and even if we roll it into bevy_ecs it can still be ignored.

@DJMcNab DJMcNab added S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged and removed D-Trivial Nice and easy! A great choice to get started with Bevy labels Jan 27, 2022
@alice-i-cecile alice-i-cecile changed the title bevy_app re-exports all of bevy_ecs::event Merge bevy_app into bevy_ecs Jan 27, 2022
@CAD97
Copy link
Contributor

CAD97 commented Jan 28, 2022

@CGMossa as the author of #2931: there's a difference here.

bevy_app is fundamentally a fairly thin usability layer on top of bevy_ecs, and doesn't add dependencies fundamentally different from bevy_ecs.

bevy_core, however, doesn't have a real "identity" in what it's for. It conflates multiple purposes, with the bevy_time functionality and glue code sort of arbitrarily in one location.

Moving the bevy_core components around as described in #2931 is an improvement to the organization of bevy code because it clarifies the internal dependency graph, and as a side effect, it makes it more practical to depend on a subset of said graph.

bevy_app and bevy_ecs could go either way; either bevy_app doesn't reëxport significant chunks of bevy_ecs, or bevy_ecs absorbs the orchestration of bevy_app. Either way could clean up the clarity of which crate does what, and merging the orchestration into bevy_ecs is simpler.

@mcobzarenco
Copy link
Contributor

Only because bevy_app is a thin layer around bevy_ecs shouldn't by itself mean there is no value in keeping things specific to the game engine separate (e. g. CoreStage ).

Thinking of use cases where bevy ecs is used as a general purpose way of structuring an application as ECS outside game development; with a custom event loop, "stages" etc.

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Jan 28, 2022

Only because bevy_app is a thin layer around bevy_ecs shouldn't by itself mean there is no value in keeping things specific to the game engine separate (e. g. CoreStage ).

I agree, this and things like DefaultPlugins should live in either the root bevy module or something like bevy_core.

Then, move App, Plugin and schedule running logic into bevy_ecs.

@bjorn3
Copy link
Contributor

bjorn3 commented Jan 28, 2022

What about instead moving Schedule out of bevy_ecs into bevy_app? This would make bevy_ecs store data, while bevy_app would store app logic.

@alice-i-cecile
Copy link
Member Author

What about instead moving Schedule out of bevy_ecs into bevy_app? This would make bevy_ecs store data, while bevy_app would store app logic.

That's an interesting split. I like it better than our current setup. I wonder if that dividing line would continue to be clean as we add more functionality 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-App Bevy apps and plugins A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Docs An addition or correction to our documentation X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

6 participants