Skip to content

Fix/bulletsync shot multiplication - #5116

Open
QueryOfficial wants to merge 2 commits into
multitheftauto:masterfrom
QueryOfficial:fix/bulletsync-shot-multiplication
Open

Fix/bulletsync shot multiplication#5116
QueryOfficial wants to merge 2 commits into
multitheftauto:masterfrom
QueryOfficial:fix/bulletsync-shot-multiplication

Conversation

@QueryOfficial

Copy link
Copy Markdown
Contributor

Summary

A shot fired once could be reported to the server many times, so the damage of a single
bullet stacks. Two defences are added to CGame::Packet_Bulletsync:

  • Repeated trajectory. A shot carrying the same start and end as the previous one from
    that player within 100 ms is dropped. CClientGame::BulletFire captures the vectors
    after the game applies weapon spread, so two real shots differ well beyond CVector's
    epsilon, and no weapon fires fast enough for this window to catch a genuine pair.
  • Packet rate. Each accepted shot draws from a per-player token bucket (30/s
    sustained, 30 burst); a player who keeps exhausting it is disconnected. Same shape as
    the existing throttles for satchel detonation and resource start acks.

The bucket arithmetic lives in Shared/mods/deathmatch/logic/CTokenBucket.h so it can be
unit tested. Packet_PlayerResourceStart has equivalent logic inline; migrating it is a
separate change I did not want to fold into this one.

Motivation

Nothing bounded how often bullet sync could be sent, and nothing recognised the same shot
arriving twice. Every copy passes weapon, ammo, trajectory and range validation, because
it is a faithful copy of a shot that really happened.

Neither check works alone: a rate ceiling high enough for fast weapons leaves slow ones
unbounded, and the trajectory check can be defeated by jittering the copies. The limits
are deliberately conservative — no player firing normally should ever be affected, at the
cost of leaving a low rate, jittered multiplier undetected. Bounding that properly needs
server side ammo accounting, since ammo is currently reported by the client in puresync.

I could not find an open issue tracking this.

Test plan

Tests/client/CTokenBucket_Tests.cpp — 10 cases run by the existing Tests workflow,
covering burst capacity, refill pacing and cap, sub-token accumulation, the drop counter
surviving partial refill and clearing on full refill, and degenerate parameters. One case
runs 5000 shots at 13/s — above the fastest bullet sync weapon — without a throttle.

Local result: 10/10 new, 338/338 total.

In game:

  1. Every bullet sync weapon (22-34), on foot and in a vehicle, sustained automatic fire
    and sniper fire at range — no shot lost, no disconnect.
  2. The same under packet loss and high ping, where shots arrive bunched.
  3. onPlayerWeaponFire still fires once per shot and damage is unchanged.

Checklist

  • Your code should follow the coding guidelines.
  • Smaller pull requests are easier to review. If your pull request is beefy, your pull request should be reviewable commit-by-commit.

A single shot could be reported many times: every copy carried the trajectory of the
original, and nothing bounded how many bullet sync packets a player could send.

Drop a shot that repeats the previous trajectory within 100ms - the game applies weapon
spread and the muzzle moves, so a player firing normally cannot reproduce both endpoints
- and run every packet through a per-player token bucket, following the pattern already
used for satchel detonation and resource start acks.

The bucket arithmetic lives in CTokenBucket.h so it can be covered by unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant