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

[Merged by Bors] - Make Command's public? #2034

Closed
wants to merge 2 commits into from

Conversation

deprilula28
Copy link
Contributor

I'm using Bevy ECS in a project of mine and I'd like to do world changes asynchronously.

The current public API for creating entities, Commands , has a lifetime that restricts it from being sent across threads. CommandQueue on the other hand is a Vec of commands that can be later ran on a World.

So far this is all public, but the commands themselves are private API. I know the intented use is with Commands, but that's not possible for my use case as I mentioned, and so I simply copied over the code for the commands I need and it works. Obviously, this isn't a nice solution, so I'd like to ask if it's not out of scope to make the commands public?

I'm using Bevy ECS in a project of mine and I'd like to do world changes asynchronously. 

The current public API for creating entities, `Commands` , has a lifetime that restricts it from being sent across threads. `CommandQueue` on the other hand is a Vec of commands that can be later ran on a World. 

So far this is all public, but the commands themselves are private API. I know the intented use is with `Commands`, but that's not possible for my use case as I mentioned, and so I simply copied over the code for the commands I need and it works. Obviously, this isn't a nice solution, so I'd like to ask if it's not out of scope to make the commands public?
@Moxinilian Moxinilian added C-Code-Quality A section of code that is hard to understand or change A-ECS Entities, components, systems, and events labels Apr 27, 2021
@bjorn3
Copy link
Contributor

bjorn3 commented Apr 27, 2021

Could you make a Commands from a CommandQueue, use this Commands to add commands and then pass the CommandQueue around?

@deprilula28
Copy link
Contributor Author

Could you make a Commands from a CommandQueue, use this Commands to add commands and then pass the CommandQueue around?

You mean with a different World object? That seems quite a bit more hacky than my current hack

@bjorn3
Copy link
Contributor

bjorn3 commented Apr 28, 2021

Forgot that Commands::new takes &'a World as argument.

@cart
Copy link
Member

cart commented Apr 28, 2021

I don't see much harm in exporting these. The Command trait is a public api that we encourage people to implement for custom types. I see no real harm in people building their own custom "Command applier" logic and using the built-in Commands.

To make the types actually usable, we'll also want to make their fields public. Additionally, you missed a few Command types in this file. Can you make them public too?

@cart
Copy link
Member

cart commented Apr 28, 2021

bors r+

bors bot pushed a commit that referenced this pull request Apr 28, 2021
I'm using Bevy ECS in a project of mine and I'd like to do world changes asynchronously. 

The current public API for creating entities, `Commands` , has a lifetime that restricts it from being sent across threads. `CommandQueue` on the other hand is a Vec of commands that can be later ran on a World. 

So far this is all public, but the commands themselves are private API. I know the intented use is with `Commands`, but that's not possible for my use case as I mentioned, and so I simply copied over the code for the commands I need and it works. Obviously, this isn't a nice solution, so I'd like to ask if it's not out of scope to make the commands public?
@bors bors bot changed the title Make Command's public? [Merged by Bors] - Make Command's public? Apr 28, 2021
@bors bors bot closed this Apr 28, 2021
NiklasEi pushed a commit to NiklasEi/bevy that referenced this pull request May 1, 2021
I'm using Bevy ECS in a project of mine and I'd like to do world changes asynchronously. 

The current public API for creating entities, `Commands` , has a lifetime that restricts it from being sent across threads. `CommandQueue` on the other hand is a Vec of commands that can be later ran on a World. 

So far this is all public, but the commands themselves are private API. I know the intented use is with `Commands`, but that's not possible for my use case as I mentioned, and so I simply copied over the code for the commands I need and it works. Obviously, this isn't a nice solution, so I'd like to ask if it's not out of scope to make the commands public?
Havvy pushed a commit to Havvy/bevy that referenced this pull request Jul 11, 2021
bors bot pushed a commit that referenced this pull request Jul 12, 2021
In #2034, the `Remove` Command did not get the same treatment as the rest of the commands. There's no discussion saying it shouldn't have public fields, so I am assuming it was an oversight. This fixes that oversight.
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
I'm using Bevy ECS in a project of mine and I'd like to do world changes asynchronously. 

The current public API for creating entities, `Commands` , has a lifetime that restricts it from being sent across threads. `CommandQueue` on the other hand is a Vec of commands that can be later ran on a World. 

So far this is all public, but the commands themselves are private API. I know the intented use is with `Commands`, but that's not possible for my use case as I mentioned, and so I simply copied over the code for the commands I need and it works. Obviously, this isn't a nice solution, so I'd like to ask if it's not out of scope to make the commands public?
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
In bevyengine#2034, the `Remove` Command did not get the same treatment as the rest of the commands. There's no discussion saying it shouldn't have public fields, so I am assuming it was an oversight. This fixes that oversight.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants