document Extended Teamplay extension with team ESP and pings#56
document Extended Teamplay extension with team ESP and pings#56Fran6nd wants to merge 2 commits into
Conversation
befa560 to
622c530
Compare
622c530 to
67ac806
Compare
Each feature is toggled independently by the server through a feature bitmask, so a server can enable any combination (or none) of them. Really, I was thinking of doing away with the whole BetterSpades-style extension thing since it's poorly-implemented, conceptually “imperfect”, and can easily be supplanted by a VersionResponse check. Context: my server stores a bitmask of
“quirks”
(originally named BS_BUGs, I'll rename them all to match soon(tm)) for each player that's connected,
it's 0 by default and corresponds to Voxlap behavior, gets set in
Anyway, I was thinking about just networking a thin abstraction over this. A single packet, begins with a packet ID, which exactly is unspecified until I dig around for an ideal one. The packet ID is followed by 0 or more bytes, each byte with 4 pairs of 2 bits, corresponding to a quirk. It is not possible for the packet to be invalid, though it is possible for clients to incorrectly state what they support (in which case a special new quirk will be made just for them). If a quirk is not understood by either peer (i.e., out of range of all known quirks), it is ignored by that peer. A client may send this packet once as soon as it connects, preferably sooner. A bitpair sent by a client may have any of the following values:
A server may send this packet at any time. A bitpair sent by a server may have any of the following values:
Here's some pseudocode for how to read/write this thing: #include <stdint.h>
#include <string.h>
/* These are indexes to the quirks array
* passed to the read/write functions.
*/
enum Quirk {
QuirkInFloor = 0,
QuirkNoShortPlayer,
QuirkScrewedDisconnectData,
QuirkOsCp437,
QuirkUtf8,
QuirkAscii,
QuirkOsBactionCull,
QuirkUtf8ColorImg,
QuirkInsky,
QuirkLength
};
/* These values get read and written in the quirks
* array passed to the read/write functions
*/
#define QUIRK_UNSPECIFIED 0
#define QUIRK_EITHER 1
#define QUIRK_OFF 2
#define QUIRK_ON 3
/* 48 is just used as a placeholder, don't depend on it */
#define QUIRKS_PACKET_ID 48
/* Packet ID is included in the size -- this is not the absolute max
* that could be received, just the most that'll be read and written.
*/
#define QUIRKS_PACKET_MAX_SIZE (1 + (QuirkLength+3)/4)
void read_quirks_packet(uint8_t quirks[QuirkLength], const uint8_t *data, size_t len) {
size_t i;
/* All quirks that don't have a byte in the packet for them
* are unspecified; unspecified has a bit-pattern of 0.
*/
memset(quirks, 0, QuirkLength);
/* Don't care about unknown quirks enough to
* overflow the quirks buffer
*/
if (len > QUIRKS_PACKET_MAX_SIZE)
len = QUIRKS_PACKET_MAX_SIZE;
/* Skip the packet ID; the caller of this function
* should've pre-validated it
*/
data++;
len--;
for (i=0; i < len*4; i++) {
quirks[i] = data[i/4] & 3;
data[i/4] >>= 2;
}
}
size_t write_quirks_packet(uint8_t *data, const uint8_t quirks[QuirkLength]) {
size_t len;
size_t i;
*data = QUIRKS_PACKET_ID;
for (i=0; i < QuirkLength; i++) {
if (i % 4 == 0) {
/* If the end of the packet is all unspecified (0) values it
* gets chopped off, though it doesn't strictly have to be.
*/
if (*data != 0)
len = data;
*(++data) = 0;
}
*data |= quirks[i] << 2 * (i % 4);
}
return len;
}That's just a draft, anyway. I might add a 2nd packet that lets the server specify a series of offsets to modify, too, if packet size from having hundreds of quirks ever becomes a tangible issue. |
This says “may”; that implies it isn't guaranteed, why? |
Does this start with a '\xff'? Does it handle colors for byte values ≤0x08-ish?
This is a really bad idea if you plan on having valid UTF-8, without NULs in the middle of it or overlong encodings and the like.
How short? Does it truncate bytewise, codepoint-wise, glyph-wise, . . ?
Does the server have to go out of its way to look at all the clients' strings to localize this? |
Make sure to account for lag; a packet from the client may still be in transit over the network after the server has sent its config packet out, and the client will most likely think it's sent the packet successfully for the next 5 seconds (as long as the server is well-behaved and doesn't pull a classic “rapid lag detected”). |
These should be the same packet.
If you decide to go my way, you'd end up with only 1 packet; piqueserver tells you to keep a subpacket ID anyway, but piqueserver's wrong. It's simpler to leave it out, and if a new quirk (“version”) gives you more packets to send, only then would it be worth considering a subpacket ID. I think it would probably be better to just extend regular packet IDs out to 2^15-ish bits (maybe add 128-ish to that packet ID to strictly enforce a canonical representation of all packets <128) by setting the high (128) bit on the base packet ID, and combining that with an additional byte directly after it.
How exactly do you verify that?
Huh? In any case, there's no reason to impose this restriction, just let the server do what it thinks is right. Team chat from enemies is mostly-valid, for example. |
That weird ghosty-terrain effect is a bug, disable volumetric fog or SSAO. |
|
Hi @DavidCo113, thank you for the detailed reply. On the extension framework vs. VersionResponse/quirks:Agreed the quirks approach is conceptually cleaner, and since you're framing it as a future direction, I think that's the right way to treat it — as its own step. Replacing the extension layer touches a lot of things, so it deserves to be specced and agreed on its own before anything migrates to it. For this PR I'd suggest we stay on the existing extension framework to keep the scope contained. The packet definitions barely change regardless of how negotiation works, so swapping just the negotiation layer later is not really a big deal. And numerous clients implement proto ext already so it would be easier for them to update. Moreover, if my pique pr makes it into master, server will be able to warn/kick/enforce however they want any protocol ext. -> I would suggest that we keep the quirk thing for a 0.77 protocol version. I am not happy with the idea to have mixed mechanisms into the same protocol version. I am open to it anyway though. On the ping timer "may" reset:You are 100% right. I thought that if client want to do differently, they can but it would degrade player experience. So not a big deal. But you are right, the spec must be strict. On the UTF-8 reason string:You're right that relaying verbatim without validation is a bad idea. I'll change it so the server validates it as well-formed UTF-8 before relaying and rejects/drops anything malformed. On the \xff question: I'll align it with the existing UTF-8 chat convention so it's consistent. On config timing / lag:Good catch. I'll note explicitly that after the server changes config, in-flight ping requests may still arrive, and the server simply drops them rather than treating them as a violation — no special handling needed beyond that. On unifying Ping Request and Ping Broadcast:Makes sense given the payloads are nearly identical — I'll look at collapsing them into a single ping packet, with the server filling in the player id on relay. Will update the spec if it stays clean. On terrain verification and who receives pings:Agreed on both. I'll drop the hardcoded "enemies must never receive pings" rule and instead leave distribution to the server's discretion — the protocol shouldn't bake in game-mode policy. For terrain/desync, I'll soften it to a server-side sanity check (bounds, and optionally a line-of-sight/solid check) rather than implying guaranteed verification, since the server is authoritative on placement anyway. On server-originated pings:One thing I'd like to add: the server itself should be able to emit pings, not only relay ones that originate from a player. A ping doesn't have to come from a Ping Request — the server can broadcast one on its own (e.g. objective markers, scripted events, admin callouts), which opens up extra gameplay possibilities. I'll account for this in the spec by reserving a sentinel player id for server-originated pings (e.g. 255) so clients can distinguish "server said" from "teammate said". I'll push an updated spec incorporating these. And waiting for your thought about this ! |
… pings, UTF-8 validation
thank you for this :) I was wondering where it was coming from! |
|
Agreed the quirks approach is conceptually cleaner, and since you're framing it as a future direction, I think that's the right way to treat it — as its own step.
Why not make this step first? I think it'd make things cleaner given it duplicates
that config packet you currently have (you'd want to add a new quirk and advertise
it as "either", then define "either" for that specific quirk
to always default to off), just on a larger scale.
Replacing the extension layer touches a lot of things, so it deserves to be specced and agreed on its own before anything migrates to it.
I don't think the piqueserver-betterspades extension advertising thing
touches much of anything at all as-is. How'd you like my draft, anyhow?
| Player ID | UByte | `0` | On relay, the player that pinged. `255` means the ping originated from the server itself. Ignored on the client -> server direction; the server fills it in authoritatively.
How does 255 display? Does it use colors from a specific team?
The server should rate-limit requests (a sane default is at most one per player per second).
Does the client enforce a limit itself, or just send off packets at will?
|
I really think something like a 0.77 is extremely unlikely to appear out of thin air on its own. Quirks serve as a smooth-ish transition to a better protocol that also happens to do a fair deal of bug/“feature” workaround while it's at it. Anyhow, it's not much different than the piqueserver extension thing you'd otherwise be using in that it's just a bit that signals some difference in protocol/gameplay/whatever handling from the “““norm””” – mine's just a bit less stupid.
I guess mixed mechanisms means alternate packets to do the same thing, or something like that? Even if you did have a hard version-barrier instead of individual bits and pieces you get to turn on and off, you'd still have to support two sets of packets to handle new and old clients, or break compatibility in the server implementation where no such need to break compatibility for e.g. XxMrNoobyFace420xX's CTF server exists. It's not that tough to speak two different languages at once though with a simple if/else, particularly when you already have to work around betterspades' fun anyway. |
Hi everyone.
My main in-game frustration is that I play most of the time with random strangers. Having real teamplay is a pain. I mean coordination, finding your teammate, spotting threats...
So on ZeroSapades, a few months ago, I have been doing something like this:
2026-04-23.00-00-11.mov
2026-04-28.14-10-08.mov
It relies only on chat messages sending to everyone from same team "PING X Y Z Reason". It is a poc, it works.
It provides teamesp as well.
However, a discussion started on Aloha: Kyrops (Kyrospades client) want it too as well as a lot of players BUT, after discussion with aloha.pk maintainers and a few others, it has been said that a protocol extension would be the best way to handle it. Especially knowing piqueserver/piqueserver#847.
So clients not supporting it can be kicked/warned they miss something or it can be disabled for gameplay purposes. Otherwise there would be a silent advantage for people having clients supporting it.
@DavidCo113 , if I remember correctly, would prefer to have the maximum use of commands instead of dedicated packets. Which I can understand. I would like to ear your thoughts from a server perspective.
This is intended to be mainly a discussion ;) I have been playing with this game for a long time but not with the protocol itself ;)
Waiting for your inputs.