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

Support multiple servercontexts #629

Merged

Conversation

Askaholic
Copy link
Collaborator

Refactors the server broadcasting code so that multiple ServerContext's can be run correctly at the same time. This will allow us to start working on a v2 version of the protocol which can concurrently with the legacy protocol. As a sample I've added a SimpleJsonProtocol class that sends each message as a json object followed by a newline.

I think we should try to transition to the new protocol incrementally. As a first step, we can swap out the "transport" or "wire" layer from QDataStream to Json, and keep everything else the same. We could implement this in the client and start using it immediately, which would cut the amount of network traffic in half (due to the legacy protocol using utf-16 encoding instead of utf-8). There isn't really a down side to doing this, as we can keep adding new incremental protocol versions on new ports. Hopefully with these refactors it will even be possible to implement a websocket protocol, but I haven't looked into the details of that yet. I just know that there is a websockets library in python that uses asyncio.

@Askaholic Askaholic force-pushed the support-multiple-servercontexts branch 2 times, most recently from f9248ac to 257e322 Compare July 19, 2020 18:04
Copy link
Member

@cleborys cleborys left a comment

Choose a reason for hiding this comment

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

Very cool how relatively few changes this needed now!

If there were multiple instances with e.g. different player services, would the player services be distinguishable in the logs?
Should there be tests for having multiple instances?

server/__init__.py Show resolved Hide resolved
@Askaholic Askaholic force-pushed the support-multiple-servercontexts branch from 257e322 to b521a4a Compare July 25, 2020 21:03
@Askaholic
Copy link
Collaborator Author

If there were multiple instances, you wouldn't be able to tell the difference in the log. Mostly i didn't bother with that since I really don't know why we would ever be running multiple server instances in the same process. I had considered actually forcing the ServerInstance to be a singleton through a metaclass, but decided it was probably overkill to code that in :P.

@Askaholic Askaholic force-pushed the support-multiple-servercontexts branch from b521a4a to 77ccc26 Compare July 25, 2020 22:56
@Askaholic Askaholic force-pushed the support-multiple-servercontexts branch 2 times, most recently from a58b4cc to 35382df Compare August 3, 2020 03:30
This class is capable of handling multiple attached ServerContext's
correctly, enabling the server to listen on multiple ports with
different Protocol implementations.
@Askaholic Askaholic force-pushed the support-multiple-servercontexts branch from 35382df to 468fcb5 Compare August 6, 2020 17:56
@Askaholic Askaholic merged commit ebacbf3 into FAForever:develop Aug 7, 2020
@Askaholic Askaholic deleted the support-multiple-servercontexts branch August 7, 2020 22:03
@Askaholic Askaholic mentioned this pull request Aug 4, 2021
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants