-
Notifications
You must be signed in to change notification settings - Fork 37
OpenStarbound Support #156
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
Conversation
use a secret to control image name
remove testing discord kill command
release 2.0 with python 3.11
fix version
|
Update on caveat 1: it does appear to work with regular stations, but throws an exception once during protocol negotiation (that doesn't affect gameplay). I'll fix that when I get a chance. |
rubellyte
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Sorry for the extremely late review; this project isn't really maintained any more. For the record, I don't expect this to change; I'll try to pay more attention for future PRs if they come in, but I'm not expecting to write any code myself any time soon.
On the subject of the PR itself, I'll address your own comments:
- The vanilla client exception is an easy fix, I've commented on it as well.
- Based on what I'm seeing, this shouldn't have any issues with older versions of OpenStarbound, and we've never been concerned about supporting non-latest versions anyways.
- Works perfectly on Windows, as far as I can tell. Don't know about Mac, but I doubt there's any issue there.
- Restarting the server like this is definitely not ideal, but it does work, and I can't immediately think of a better way to handle this. Maybe it would hypothetically be better to install the decompression filters by default and have them default to just passing through data, and flip a toggle in that when compression support is detected? I don't know, just a cursory thought.
I don't know what the performance implications of this compression cycle are, but ultimately I doubt it poses a major issue next to the bottleneck that is our packet parser. I'm inclined to approve this once you address the comments I'm leaving.
Side note: Is #154 still valid? I'll merge that as well if so.
|
Howdy. I've updated my PR with changes that address your comments, I think. I've tested it briefly on a (heavily modded) server I control and it appears to be working well. I assume #154 is still valid as well, though I only run on Linux so I don't need it in day-to-day operation. |
|
Separately, I will consider your thought about installing filters and disabling them with a flag. My thinking was that I wanted to keep OSB support entirely out of the loop by default so I wouldn't touch the stock workflow unless necessary. I'm out of touch with the Starbound community; if people are generally switching to OSB then that is probably unimportant. I don't know when I'll have a chance to address this, so I plan to do it on a sub-branch and submit a separate PR if/when I do. |
Hi folks,
I don't know if you are still watching this repo at all, but I've done a bit of work to update the system to support OpenStarbound. OpenSB negotiates compressing the data stream during the first packet exchange, so I've added a plugin to detect that, and when it does so it deploys some wrappers around the asyncio readers that try to transparently perform that compression.
Caveats: