fix(anon): let pinned teammates see each other - #4945
Open
Zixer1 wants to merge 1 commit into
Open
Conversation
With anonymizeNames on, seesReal was true only for yourself or a viewer granted reveal access, so in a Team game you could not identify your own teammate. A team that cannot coordinate is not a team. A player now also sees the real name of anyone on their pinned team. Only pinned teams: those are assigned server-side (matchmakingTeams), so the server knows them here. A team game that groups by clanTag/friends is resolved on the clients and the server has no answer to give, so nothing is revealed there. Safe for the same reason 'target === viewer' already is: this widens only username and cosmetics, neither of which the simulation reads (Player.hash excludes names). clanTag and friends DO feed assignTeams and are still blanked identically for every viewer.
Contributor
WalkthroughThe server now allows players on the same assigned matchmaking team to see real names and cosmetics when names are anonymized. Tests cover opposing teams, unassigned players, non-matchmade games, symmetry, and blank team-assignment fields. ChangesMatchmaking team visibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/GameServer.ts`:
- Around line 364-369: The seesReal/gameInfo flow exposes team-assignment data
for pinned teammates while anonymizeNames is enabled. In
src/server/GameServer.ts lines 364-369, keep username and cosmetics visible but
ensure gameInfo returns clanTag as null and friends as undefined; update
tests/server/AnonymizeNamesTeammates.test.ts lines 15-29 so makeClient creates
non-null clanTag and non-empty friends, and lines 109-114 assert those fields
are hidden for every player.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7102cdc8-6865-4e4f-961f-b73027261959
📒 Files selected for processing (2)
src/server/GameServer.tstests/server/AnonymizeNamesTeammates.test.ts
Comment on lines
364
to
369
| private seesReal(viewer: ClientID | undefined, target: ClientID): boolean { | ||
| return ( | ||
| !this.gameConfig.anonymizeNames || | ||
| target === viewer || | ||
| this.sameMatchmadeTeam(viewer, target) || | ||
| this.viewerSeesAllNames(viewer) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep team-assignment inputs hidden when teammate identity is visible.
Lines 364-369 make pinned teammates enter gameInfo()'s real branch. That branch sends clanTag and friends at src/server/GameServer.ts lines 1595-1601. This exposes team-assignment inputs when anonymizeNames is enabled. It conflicts with the PR requirement that those fields remain blank for every viewer.
src/server/GameServer.ts#L364-L369: keepusernameandcosmeticsvisible for pinned teammates, but forceclanTagtonullandfriendstoundefinedingameInfo()whileanonymizeNamesis enabled.tests/server/AnonymizeNamesTeammates.test.ts#L15-L29: allowmakeClient()to create clients with non-nullclanTagvalues and non-emptyfriendsvalues.tests/server/AnonymizeNamesTeammates.test.ts#L109-L114: assert that every player hasclanTag === nullandfriends === undefined.
Proposed fix
- clanTag: hideClanTags ? null : (c.clanTag ?? null),
+ clanTag:
+ this.gameConfig.anonymizeNames || hideClanTags
+ ? null
+ : (c.clanTag ?? null),
clientID: c.clientID,
- friends: friendsFor(c),
+ friends: this.gameConfig.anonymizeNames
+ ? undefined
+ : friendsFor(c),📍 Affects 2 files
src/server/GameServer.ts#L364-L369(this comment)tests/server/AnonymizeNamesTeammates.test.ts#L15-L29tests/server/AnonymizeNamesTeammates.test.ts#L109-L114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/GameServer.ts` around lines 364 - 369, The seesReal/gameInfo flow
exposes team-assignment data for pinned teammates while anonymizeNames is
enabled. In src/server/GameServer.ts lines 364-369, keep username and cosmetics
visible but ensure gameInfo returns clanTag as null and friends as undefined;
update tests/server/AnonymizeNamesTeammates.test.ts lines 15-29 so makeClient
creates non-null clanTag and non-empty friends, and lines 109-114 assert those
fields are hidden for every player.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With
anonymizeNameson,seesRealwas true only for yourself or a viewer granted reveal access — so in a Team game you could not identify your own teammate. A team that cannot coordinate is not a team.A player now also sees the real name of anyone on their pinned team. Only pinned teams: those are assigned server-side via
matchmakingTeams, so the server knows them here. A team game that groups byclanTag/friendsis resolved on the clients, so the server has no answer to give and nothing is revealed there.Safe for the same reason
target === vieweralready is: this widens onlyusernameandcosmetics, neither of which the simulation reads (Player.hashexcludes names).clanTagandfriendsdo feedassignTeamsand are still blanked identically for every viewer.Pairs with #4944, which is what lets a bot pin teams in the first place.