search

iamgreaser / iceball

Open-source rewrite of the VOXLAP version of Ace of Spades.
http://iceball.build
GNU General Public License v3.0
113 stars 32 forks source link

Hostname support w/ heartbeat resolution #177

Closed NateShoffner closed 9 years ago

rakiru commented 9 years ago

Changing the heartbeat protocol requires a version bump, which breaks compatibility with previous versions. There is issue #173 where changes are being discussed. I'm going to close this for now, as the protocol is going to change drastically, so this won't be usable then anyway.

NateShoffner commented 9 years ago

I had asked about the version bump in IRC. Currently, the hostname is just appended to the packet, so in theory it wouldn't have to break the current protocol, just extend it. Could turn it into an optional field, of sorts, to retain backwards compatibility and avoid having to bump the version in this instance.

Without trying to split the two topics, the idea in that issue is for variable/optional fields. As far as functionality is concerned, something like a hostname should be validated and not just used "as is" like a normal server property.

rakiru commented 9 years ago

Yeah, I had a quick look at my IRC logs after commenting. It's up to grease if he wants to pull this, but it feels like a convenient hack to me. I'm also not a big fan of the fixed-length strings at the moment, doubly so when they're relatively massive.

What use does having the hostname serve? Is it something that is necessary straight away? There were discussions on IRC yesterday about what should be included in the protocol, and some of those comments have been added to the ticket in the issue tracker. Feel free to answer this question there, or I'll quote any points made here on it later.

NateShoffner commented 9 years ago

It's really just a vanity/convenience feature for users. It's a lot easier to remember/share a hostname than an IP for a server. The client already resolves hostnames, so if you feed it iceball://hostname.com:27640, it'll work as intended.

The fixed-length strings are a bit odd as well, should certainly be variable-length/null-terminated. Obviously that could be accommodated here with ease.

rakiru commented 9 years ago

There aren't really enough servers now that that's too much of a problem, and you can just copy link from the web server list at the moment. It could be handy, but if you have to remember it, then you're probably not using the server list anyway.

As stated in the ticket, the original plan for v3 was variable-length strings of some sort, but yeah, if we do hack it on the end here, I'd rather it wasn't also fixed-length.

iamgreaser commented 9 years ago

We're opting for the heartbeat packets to be in a JSON format in the next heartbeat version, to allow for easy expansion w/o having to update the heartbeat server version so often. Exactly how the format goes is undecided but it'll be sorted out pretty easily.

As for HB server to launcher, changes in that can be backwards-compatible and it doesn't require a protocol update.

But I will probably be using bits of your code as a reference, so thanks for making the pull request.