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

Add signalling utils to bevy_matchbox #215

Merged
merged 6 commits into from
Jun 23, 2023

Conversation

garryod
Copy link
Collaborator

@garryod garryod commented Apr 16, 2023

Adds some utils for creating a MatchboxServer to a bevy app as a Resource.

stop_server is currently a no-op, need to figure out how best to kill a bevy task - probably scopes. Realised this is also an issue with our regular socket behaviour

@garryod garryod marked this pull request as draft April 16, 2023 21:19
@johanhelsing
Copy link
Owner

stop_server is currently a no-op, need to figure out how best to kill a bevy task - probably scopes. Realised this is also an issue with our regular socket behaviour

For the socket i think it would probably make most sense to either support dropping the socket, or have a channel we send to that will make the future clean up and stop.

@garryod
Copy link
Collaborator Author

garryod commented Apr 22, 2023

stop_server is currently a no-op, need to figure out how best to kill a bevy task - probably scopes. Realised this is also an issue with our regular socket behaviour

For the socket i think it would probably make most sense to either support dropping the socket, or have a channel we send to that will make the future clean up and stop.

Have opted to move the Task handle into the Component / Resource such that when they're dropped the task is terminated

@garryod
Copy link
Collaborator Author

garryod commented Apr 22, 2023

Also made the choice to remove the feature gate as target specific features are not yet supported (see: rust-lang/cargo#1197). An alternative method is to add a build script which provides an appropriate error message when someone tries to use a feature incompatible with the target, but this doesn't play particularly well with other tooling

@garryod garryod force-pushed the bevy-server branch 3 times, most recently from 1f0ebeb to ff4eb53 Compare April 22, 2023 18:53
@garryod garryod marked this pull request as ready for review April 22, 2023 18:58
Copy link
Collaborator

@simbleau simbleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Any bevy_matchbox improvements are good to me.

@simbleau
Copy link
Collaborator

@johanhelsing ?

@simbleau
Copy link
Collaborator

@johanhelsing approval?

Copy link
Owner

@johanhelsing johanhelsing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great :)

@simbleau simbleau merged commit 8e864f1 into johanhelsing:main Jun 23, 2023
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

Successfully merging this pull request may close these issues.

3 participants