-
-
Notifications
You must be signed in to change notification settings - Fork 58
Moving off async outside IO and into bevy_ecs #178
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
How the hell does this event system work???
Gunna take verq's suggestion and try decoupling the handshake and play stages of the networking
Dammit, verq was right
Sweatty said to merge it and fix later so it's on him if it doesn't work (jk) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Turns out a homebrew ECS and event system isn't very reliable. Who would have guessed? Not us apparently.
Description
This is a massive PR implementing a huge amount of changes over the entire codebase. There are 2 big parts of this PR:
Part 1 is completely separating out the sync and async code. Async code is now exclusively used for networking IO and the rest of the server runs entirely on native threads, managed by bevy_ecs. Doing fully async or fully sync has been the easiest option for a while but it is far from the most efficient, so we decided to bite the bullet and do it properly. Originally I was able to split the async and sync without using bevy, hence the name of the branch, but this quickly highlighted the issues with our own ECS and event system which leads into the second major change;
Part 2 is entirely removing our homemade ECS and event system, in favor of bevy's much more advanced and performant ECS and event system. This comes with many upsides, such as less maintenance cost on us, more effective parallelism and a much more flexible architecture. While we are very proud of our custom ECS, we ultimately decided that using a better alternative developed by some extremely talented programmers and driven by a passionate community was the way forward. Due to the major differences between bevy and our old ECS, massive changes had to be made to the codebase (hence the ~7.7k total lines changed at time of writing) so large amounts of the codebase will be wildly different.
Some less major changes were made such as:
Motivation and Context
After constant issues with the ECS and the original developer of our event system dropping off the face of the planet with no-one having any idea how it worked, we decided to cut our losses and move onto a more battle-tested ECS and event system. Just eyeballing it, performance is a bit better but not by a massive amount. However, the callstack not being muddied by tokio and bevy's reflection approach means that diagnosing performance issues are going to be much easier in the future.
How has this been tested?
Units tests aren't really for feasible for this, but it compiles fine and all functionality seems to work as intended. Still keeping an eye out for bugs.
Screenshots (if appropriate):
Types of changes
Checklist: