Don't check offline players for speedhacking#836
Conversation
The speedhack checks validate what a game client claims about its own timing. An offline player has no client: the server itself paces its walks and attacks, so there is nothing to validate and every hit is a false positive - answered, with the default configuration, by a persisted account ban. They do get hit. On a test server with 500 offline bots, 15 of 100 accounts were banned within half an hour, at about one account a minute. Only one of the 101 warnings in that window came from the walk check; the rest came from the attack token bucket, which refills at most one token per max(60, 450 - AttackSpeed * 1.2) ms, so at most one per 450 ms. The MU Helper attacks once per 500 ms tick and normally stays ahead of that. It stops staying ahead when the ticks lose their cadence: PeriodicTimer starts the next tick immediately after one that ran long, so two attacks can land a few dozen milliseconds apart while the bucket only refills for the time that actually elapsed. Those warnings fall in a window in which the host was also timing out database connections, and a later two-hour run of the same fleet on an idle host produced none - it is a load-dependent false positive. A configuration can draw two tokens from a single tick on its own: with UseDrainLife enabled, PerformDrainLifeRecoveryAsync attacks before PerformAttackAsync. This is not specific to bots. The same class and the same tick drive a regular player's /offlevel session, and neither the checks nor the tick distinguish the two. Retuning the thresholds would weaken the check against the clients it was written for and still leave the cause in place, so the players it cannot meaningfully judge are skipped instead.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where offline players were incorrectly being flagged and banned for speedhacking. Because offline players are controlled by the server rather than a client, their timing patterns can trigger false positives in the existing cheat detection logic. The changes exempt these players from speedhack checks, ensuring that server-paced actions do not result in accidental account bans. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request bypasses speed hack detection for server-controlled (offline) players to prevent false positives and unintended account bans. The WalkCheatCheckAsync and AttackCheatCheckAsync methods in SpeedHackDetectPlugIn are updated to skip validation for offline players, and corresponding unit tests are added to verify this behavior. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
The speedhack checks validate what a game client claims about its own timing.
An offline player has no client - the server itself paces its walks and attacks,
so there is nothing to validate there and every hit is a false positive. With the
default configuration a false positive is answered with
AccountState.Banned,saved to the database.
They do get hit
On a test server running 500 offline bots, 15 of 100 accounts were banned
within half an hour, at roughly one account a minute and accelerating:
Of the 101 warnings in that window, exactly one came from the walk check -
the only one which logs a line of its own. The other hundred came from
AttackCheatCheckAsync, whose bucket holds five tokens and refills at most oneper
max(60, 450 - AttackSpeed * 1.2)ms, i.e. one per 450 ms.A MU Helper tick attacks once per 500 ms, which normally stays ahead of that
refill. It stops staying ahead when the ticks lose their cadence:
PeriodicTimerstarts the next tick immediately after one that ran long, so two attacks can land
a few dozen milliseconds apart while the bucket only refills for the time that
actually elapsed. Once it is empty, every following tick is a violation until the
fourth warning bans the account.
That makes it load dependent, and the evidence says so too: the warnings fall in
a window in which the host was also timing out database connections, and a later
two-hour run of the same fleet on an idle host produced none.
One configuration draws two tokens from a single tick on its own: with
UseDrainLifeenabled,PerformDrainLifeRecoveryAsyncattacks beforePerformAttackAsyncin the same cycle.This is not specific to bots. The same class and the same tick drive a regular
player's
/offlevelsession, and nothing in the checks or in the tickdistinguishes the two - the only
IsBotgate on this path is a randomized250-900 ms delay before engaging a new target, which makes a bot slower than a
human's offline session, not faster.
The change
Both check methods return early for
OfflinePlayer. The four call sites(
Player.WalkToAsync,HitAction,TargetedSkillDefaultPlugin,AreaSkillAttackAction) are untouched, so a fifth one added later inherits theexemption automatically.
No new configuration option: a switch would only offer to re-enable a check that
cannot produce a true positive for these players, and the cost of a false one is
a persisted ban.
Retuning
MaxAttackTokensorWalkSpeedToleranceMswas the alternative. Itwould weaken the check against the clients it was written for and still leave the
cause in place, so it is not what this does.
Testing
MUnique.OpenMU.Tests: 590 passed, 0 failed.Two regression tests were added to the existing fixture, both built from the
patterns that already detect a regular player there:
TestOfflinePlayerIsNotCheckedOnWalkAsync- the twelve-walk burst which bans aregular player in
TestWalkSpeedHackDetectionBansAccountAsync;TestOfflinePlayerIsNotCheckedOnAttackAsync- the rapid attack series detectedin
TestAttackSpeedHackDetectionAsync.Both were verified to fail without the change (2 failed, 0 passed) and pass with
it, so they cannot go vacuously green.
Workaround for anyone hitting this before the fix ships
SpeedHackDetectConfigurationis editable in the admin panel: turningAutoBanoff keeps the warnings in the log while stopping the bans.