Reload packet from OS reporting UINT8_MAX for clip and reserve

[Haxk20]

While implementing reload logic in SpadesX i noticed a rather concerning issue that is OpenSpades when client does a reload it sends in reload packet 255 for both clip and reserve.

The code has some comment that it should be 255 but this wrong. The values sent to the server should be REAL value of clip and reserve.

They should be real so that server can do a check for the actual ammo and adjust according to client if the calculation done on server side is in range where it could be just ping. (Server has 5 bullets shot yet client would really shoot only 4 for example).

I also noticed that the client doesnt set the ammo to whatever server sent to the client unless the values are really off.

Client should ALWAYS depend on what ammo the server sent. As this can cause desync. Yes the desync is handled but it is not ideal to have any in the first place. @yvt

[Haxk20]

@yvt :|

[NotAFile]

This is intended behavior. The client's information should be disregarded.

[Haxk20]

This is intended behavior. The client's information should be disregarded.

The why the F even send this crap ? BetterSpades sends this as its supposed to. One would expect the clients to at least somewhat do the same.

And dont get me wrong. Ofc the server has to calculate this on its own. But good to have a check. Like we do with position.

[xtreme8000]

The worse thing is that afaik pyspades servers just relay "this crap" back to clients without even adding what the server thinks is the correct ammo count. So like @Haxk20 said, what even is its purpose then if no one can use it for anything.

[yvt]

Looking at the code, I think I'm starting to remember why it's done in this way.

IIRC the WeaponReload packet type is used for the following purposes:

1. (C-to-S) To start reloading.
2. (S-to-C) To inform everyone (including the initiating player) that someone started reloading.
3. (S-to-C) To notify the initiating player that the reloading cycle is complete and to inform the new ammo count.

(2) is an exact copy of (1), so if (1) contained real values, the client wouldn't be able to distinguish between (2) and (3). (OS waits for (3) before fully completing the reload action to be very sure, but there's a fallback mechanism and work-arounds for subtle server-side quirks I don't quite remember.)

(2) is also broadcast to all players, so if (1) contained real values, the server would happily reveal the player's current ammo count to everyone in the server.

[Haxk20]

Looking at the code, I think I'm starting to remember why it's done in this way.

IIRC the WeaponReload packet type is used for the following purposes:

1. (C-to-S) To start reloading.

2. (S-to-C) To inform everyone (including the initiating player) that someone started reloading.

3. (S-to-C) To notify the initiating player that the reloading cycle is complete and to inform the new ammo count.

(2) is an exact copy of (1), so if (1) contained real values, the client wouldn't be able to distinguish between (2) and (3). (OS waits for (3) before fully completing the reload action to be very sure, but there's a fallback mechanism and work-arounds for subtle server-side quirks I don't quite remember.)

(2) is also broadcast to all players, so if (1) contained real values, the server would happily reveal the player's current ammo count to everyone in the server.

The way i handled it for now in SpadesX is that i send the real values as soon as i get the reload packet from server. Yes there is supposed to be delay ofc but i didnt implement that yet.

What i still dont understand why you are "fixing" server side issues with "hacks". If the server is not supposed to inform the other players of the ammo count of the reloading player that DOESNT mean that you should send 255 255 or 0 0. What server SHOULD do is to grab the packet, read it, Check if the values are correct and adjust if necessary. Then server should create 2 new packets. Both reload. 1 with 0 0 or 255 255 and the second one with real values.

Real ammo ofc gets sent to the reloading player and the fake to the rest of the players. This is a tiny change in nearly all servers and can be implemented rather easily.

I just dont like the fact that clients send to the server fake info and client and server at any time communicate the ammo count. Except when server is telling the client the count. Which client follows blindly. Server has realistically no idea what ammo count the client has. It could be out by quite a few bullets due to lag and etc.

So we have to fix this server side (I will do this in SpadesX in the upcoming days) and client should fix this by sending the real value.

[yvt]

What i still dont understand why you are "fixing" server side issues with "hacks".

Because that's how the existing AoS 0.75β-compatible server programs (especially pyspades) work, and they will probably continue to do so.

So you are suggesting changing the protocol to fix the problem. Do you have a concrete solution that preferably doesn't involve breaking compatibility with other clients or introducing a separate client-side code path dedicated for the revised protocol? Otherwise, I'm not sure if this is worth investing in.

[Haxk20]

What i still dont understand why you are "fixing" server side issues with "hacks".

Because that's how the existing AoS 0.75β-compatible server programs (especially pyspades) work, and they will probably continue to do so.

So you are suggesting changing the protocol to fix the problem. Do you have a concrete solution that preferably doesn't involve breaking compatibility with other clients or introducing a separate client-side code path dedicated for the revised protocol? Otherwise, I'm not sure if this is worth investing in.

How is this changing the protocol ? Protocol doesnt say anything about what you should send there. And afaik old servers wont have the slithtest issue handling whatever you send to them. Cause they usually just ignore it either way.

So i dont see how changing 2 numbers would really break anything here. And if it does we can test it ofc.

I myself send a lot of modified packets for each player to prevent nofog hacks for example. Players outside of the 134 block range are just straight up invisible to the player. Or better say the are placed on 0 0 0. And it doesnt break anything.

[Haxk20]

Well its been a while. I reread it. I see what you mean about not being able to differentiate. I mean thats not that big of an issue. The bigger issue is players knowing how much ammo you have.

The main concern i have here is that Server is basically in the dark about client ammo and reserve count. We emulate this on server side. But it does rarely go out of whack. But the issue is it does. And yes dying and being in tent resets all this. But there may be 2 or 3 bullets. Depends on the difference and those bullets are technically illegal. And should kill since server counted less then what we actually have or if server counts more bullets it may allow kill in modified client that it shoudlnt. Basically the most basic anti cheat for ammo. And in babel or normal ctf. 2 bullets. Fine i guess. Arena tho ? Oooofffff there it truly matters. Its very competitive

Now what we can actually do without breaking compatibility. Not sending the client ammo is dumb. That is not the solution thats for sure. We can setup an extension in protocol like it was done previously for such things where we check if server knows how to handle actual real ammo count. That would be once we get on server side from client that the player reloaded. We would not just copy the clip and reserve count and send it to all players BUT send fake data. Like the clients expect. So 0 0 or 255 255. Preferably 255 255 so we cant confuse it. And send this to all players. And once reloading is done. Send packet to the initiating reload player that it finished. And send them their new ammo.

This can be done on servers (Since pique and SpadesX are the only ones alive basically then pique and spadesx) without any special handling. Just do it like this. It wont break anything. The issue comes with clients. Thats where the check comes in. If server supports the real ammo handling. Clients would send actual ammo data. If server doesnt. They would send 0 0 or 255 255. Both are either way impossible to get. Since the ammo sent would be the ammo client had before doing the reloading math. Obviously as server is supposed to do that

@yvt @xtreme8000 @NotAFile Do any of you have anything against this ? I dont see anything bad. And since we will have that check in place. Technically servers can also return to the old way of just copying data. But since voxlap sends 0 0 we might just as well change OpenSpades to send 0 0 in there in case server doesnt support it and that way servers can default to sending 0 0 to all players and then real data to the actual player since the thing was that it copies what it gets from first reload packet. But yes i did actually check. Voxlap sends only 0 0. So we might just as well match voxlap even if this doesnt get implemented (Dont see any reason why not)

[Haxk20]

Ping