Skip to content

Conversation

@ssiccha
Copy link

@ssiccha ssiccha commented Jan 4, 2026

There are problems with the ban system when a game doesn't start due to
connection issues. The current hypothesis is that all players except for
the match host receive a violation, regardless of whether they could
establish connections successfully or not.

The in-development Pioneer ICE Adapter is better in terms of initial
connections. So the impact of the erroneous violations might become
small enough to simpy accept it. Once Pioneer goes into prod we can
think about reverting this.

Summary by CodeRabbit

  • Chores
    • Updated configuration settings to explicitly define a feature flag with improved clarity and maintainability.

✏️ Tip: You can customize this high-level summary in your review settings.

Brutus5000 and others added 6 commits December 28, 2025 11:21
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
There are problems with the ban system when a game doesn't start due to
connection issues. The current hypothesis is that all players except for
the match host receive a violation, regardless of whether they could
establish connections successfully or not.

The in-development Pioneer ICE Adapter is better in terms of initial
connections. So the impact of the erroneous violations might become
small enough to simpy accept it. Once Pioneer goes into prod we can
think about reverting this.
@coderabbitai
Copy link

coderabbitai bot commented Jan 4, 2026

📝 Walkthrough

Walkthrough

A configuration file was updated to explicitly define the LADDER_VIOLATIONS_ENABLED flag with a value of false, replacing a previously commented-out or undefined setting. This enables the feature toggle via explicit boolean configuration.

Changes

Cohort / File(s) Summary
Configuration Settings
apps/faf-lobby-server/config/config.yaml
Added explicit LADDER_VIOLATIONS_ENABLED: false configuration entry with accompanying comments to define the ladder violations feature toggle.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A hop, skip, and toggle so fine,
The config flag now shines bright and defined,
False by default, the rabbit agrees,
No ladder breaches shall bend the trees! 🐰⚙️

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'config: faf-lobby-server - Disable bans' directly describes the main change: disabling the ban system in the faf-lobby-server configuration via the LADDER_VIOLATIONS_ENABLED flag.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1a4686 and b6018ef.

📒 Files selected for processing (1)
  • apps/faf-lobby-server/config/config.yaml
🔇 Additional comments (1)
apps/faf-lobby-server/config/config.yaml (1)

58-58: This is a deployment configuration repository; implementation verification requires access to the faf-lobby-server source code.

The LADDER_VIOLATIONS_ENABLED: false flag is correctly defined in the Helm configuration for the faf-lobby-server deployment. However, whether this flag actually disables the ban system depends on the implementation in the faf-lobby-server application source code, which is maintained in a separate repository (FAForever/faf-lobby-server). The gitops-stack repository contains only deployment and configuration artifacts, not the application logic. Verification of the flag's functional impact should be performed against the faf-lobby-server source code repository.

Likely an incorrect or invalid review comment.

Comment on lines +48 to +57
# 3rd Jan 2026: Disable matchmaking bans
# There are problems with the ban system when a game doesn't start due to
# connection issues. The current hypothesis is that all players except for the
# match host receive a violation, regardless of whether they could establish
# connections successfully or not.
#
# The in-development Pioneer ICE Adapter is better in terms of initial
# connections. So the impact of the erroneous violations might become small
# enough to simpy accept it. Once Pioneer goes into prod we can think about
# reverting this.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typo in comment.

Line 56 contains a typo: "simpy" should be "simply".

🔎 Proposed fix
 # The in-development Pioneer ICE Adapter is better in terms of initial
 # connections. So the impact of the erroneous violations might become small
-# enough to simpy accept it. Once Pioneer goes into prod we can think about
+# enough to simply accept it. Once Pioneer goes into prod we can think about
 # reverting this.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# 3rd Jan 2026: Disable matchmaking bans
# There are problems with the ban system when a game doesn't start due to
# connection issues. The current hypothesis is that all players except for the
# match host receive a violation, regardless of whether they could establish
# connections successfully or not.
#
# The in-development Pioneer ICE Adapter is better in terms of initial
# connections. So the impact of the erroneous violations might become small
# enough to simpy accept it. Once Pioneer goes into prod we can think about
# reverting this.
# 3rd Jan 2026: Disable matchmaking bans
# There are problems with the ban system when a game doesn't start due to
# connection issues. The current hypothesis is that all players except for the
# match host receive a violation, regardless of whether they could establish
# connections successfully or not.
#
# The in-development Pioneer ICE Adapter is better in terms of initial
# connections. So the impact of the erroneous violations might become small
# enough to simply accept it. Once Pioneer goes into prod we can think about
# reverting this.
🤖 Prompt for AI Agents
In apps/faf-lobby-server/config/config.yaml around lines 48 to 57, there's a
typo in the comment: change "simpy" to "simply" on line 56; update the comment
text accordingly so it reads "...might become small enough to simply accept it."
and save the file.

@Sheikah45 Sheikah45 force-pushed the develop branch 7 times, most recently from 73e25ff to 86676ce Compare January 5, 2026 02:10
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.

6 participants