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

Allow application code to be transport agnostic #119

Merged
merged 5 commits into from
Nov 7, 2023

Conversation

OleStrohm
Copy link
Contributor

This PR aims to provide ways of interacting with renet that are transport layer agnostic.
The basic additions in this PR are two bevy System Sets: RenetReceive and RenetSend.
These are the system sets where all transport layers should place their receive and send systems, allowing users to easily ensure that their systems run before/after Renet's transport layer systems run, rather than having to individually ensure that for each transport layer they care about.

The other addition in this PR is to move the Bevy run conditions from being implemented in each transport layer to making one single definition that uses RenetClient. This also means that each transport layer now has to update the RenetClient with the current state.

I also made quite a few methods private, specifically the methods that expose the internal connection status of the transport layers and the systems that previously marked when a transport layer would receive/send data. This was primarily done to help users migrating from previous versions notice that these shouldn't be used anymore, as well as not confuse new users.
If this change isn't desired (especially the one about the transport layer connection status), then I'm happy to revert this change.

Fixes #115

@OleStrohm
Copy link
Contributor Author

Also as an aside, I'm not entirely sure how you do the changelog, i.e. whether I should write the changelog here, or if you go through all the PRs with breaking changes when you draft a new release.

Copy link
Contributor

@UkoeHB UkoeHB left a comment

Choose a reason for hiding this comment

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

Looks good

bevy_renet/src/lib.rs Outdated Show resolved Hide resolved
bevy_renet/src/lib.rs Outdated Show resolved Hide resolved
renet/src/remote_connection.rs Outdated Show resolved Hide resolved
renet/src/remote_connection.rs Outdated Show resolved Hide resolved
renet/src/remote_connection.rs Outdated Show resolved Hide resolved
renet/src/remote_connection.rs Show resolved Hide resolved
renet/src/remote_connection.rs Outdated Show resolved Hide resolved
renet/src/remote_connection.rs Show resolved Hide resolved
renet/src/server.rs Outdated Show resolved Hide resolved
@lucaspoffo lucaspoffo self-requested a review November 7, 2023 01:40
Copy link
Owner

@lucaspoffo lucaspoffo left a comment

Choose a reason for hiding this comment

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

Thanks! This is exactly what I was thinking that needed to be done.

Great comments and the code looks fine for me.
With this, changing the transport layer should be even easier now.

Also, thanks @UkoeHB for the suggestions

@lucaspoffo lucaspoffo merged commit 1b4c1c9 into lucaspoffo:master Nov 7, 2023
2 of 3 checks passed
@Shatur Shatur mentioned this pull request Nov 12, 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.

Transport abstraction
3 participants