Skip to content

Comments

removed _____ from veto debug logs#1074

Merged
K-ETFreeman merged 1 commit intodevelopfrom
refactor/veto-logs
Nov 9, 2025
Merged

removed _____ from veto debug logs#1074
K-ETFreeman merged 1 commit intodevelopfrom
refactor/veto-logs

Conversation

@K-ETFreeman
Copy link
Contributor

@K-ETFreeman K-ETFreeman commented Nov 9, 2025

removed _____ from veto debug logs

Summary by CodeRabbit

  • Chores
    • Improved internal debug logging formatting for consistency and readability across matchmaking services.

@coderabbitai
Copy link

coderabbitai bot commented Nov 9, 2025

Walkthrough

Debug logging statements across three service modules were cleaned by removing decorative underscore prefixes from log message labels. The functional behavior and logic remain unchanged; only log output formatting was simplified for consistency.

Changes

Cohort / File(s) Summary
Log message formatting cleanup
server/ladder_service/ladder_service.py, server/ladder_service/veto_system.py, server/matchmaker/map_pool.py
Removed decorative underscore prefixes from debug log message labels (e.g., "______queue.map_pools[map_pool.id]________________: %s""queue.map_pools[map_pool.id]: %s"). No logic or control flow changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

These are purely cosmetic changes to debug log formatting with no functional impact on behavior or logic.

Poem

🐰 Away with the underscores, long and dreary,
Clean logs now shine, no longer eerie,
Three files tidied, formatting bright,
Debug messages now read just right! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: removing decorative underscores from debug log messages across the veto system and related services.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/veto-logs

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e203cd and a94abc1.

📒 Files selected for processing (3)
  • server/ladder_service/ladder_service.py (1 hunks)
  • server/ladder_service/veto_system.py (1 hunks)
  • server/matchmaker/map_pool.py (2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: K-ETFreeman
Repo: FAForever/server PR: 1033
File: server/ladder_service/veto_system.py:113-124
Timestamp: 2025-10-26T15:11:39.923Z
Learning: In server/ladder_service/veto_system.py, the `+ [-1]` appended to map_pool_map_version_ids in the extract_pools_veto_config method was dead code from an old design idea to allow players to ban "last played map" as an anti-repetition feature. This feature was never implemented and the code was removed.
📚 Learning: 2025-10-26T15:11:39.923Z
Learnt from: K-ETFreeman
Repo: FAForever/server PR: 1033
File: server/ladder_service/veto_system.py:113-124
Timestamp: 2025-10-26T15:11:39.923Z
Learning: In server/ladder_service/veto_system.py, the `+ [-1]` appended to map_pool_map_version_ids in the extract_pools_veto_config method was dead code from an old design idea to allow players to ban "last played map" as an anti-repetition feature. This feature was never implemented and the code was removed.

Applied to files:

  • server/ladder_service/ladder_service.py
  • server/matchmaker/map_pool.py
  • server/ladder_service/veto_system.py
📚 Learning: 2025-10-26T14:43:30.226Z
Learnt from: K-ETFreeman
Repo: FAForever/server PR: 1033
File: server/matchmaker/map_pool.py:80-109
Timestamp: 2025-10-26T14:43:30.226Z
Learning: In server/matchmaker/map_pool.py, the choose_map method does not need a guard against all-zero final_weights because the veto system validation logic (including minimum_maps_after_veto constraints) prevents pools from being configured where all maps can be fully banned, ensuring at least some maps always have non-zero weights.

Applied to files:

  • server/ladder_service/ladder_service.py
  • server/matchmaker/map_pool.py
  • server/ladder_service/veto_system.py
📚 Learning: 2025-10-26T14:51:28.358Z
Learnt from: K-ETFreeman
Repo: FAForever/server PR: 1033
File: server/types.py:46-47
Timestamp: 2025-10-26T14:51:28.358Z
Learning: In server/types.py, the MapPoolMap Protocol intentionally requires map_pool_map_version_id to be non-Optional (int) because maps in a map pool context must have this ID. The concrete implementations (Map and NeroxisGeneratedMap) use Optional[int] to support usage outside of map pools, but when used as MapPoolMap they must be instantiated with a non-None value.

Applied to files:

  • server/matchmaker/map_pool.py
📚 Learning: 2025-10-26T14:43:05.147Z
Learnt from: K-ETFreeman
Repo: FAForever/server PR: 1033
File: server/ladder_service/veto_system.py:257-280
Timestamp: 2025-10-26T14:43:05.147Z
Learning: In server/ladder_service/veto_system.py, the field max_tokens_per_map is guaranteed to be non-negative through external constraints (not enforced at the database schema level in the visible code).

Applied to files:

  • server/ladder_service/veto_system.py
📚 Learning: 2025-10-29T18:56:18.770Z
Learnt from: K-ETFreeman
Repo: FAForever/server PR: 1071
File: server/types.py:35-40
Timestamp: 2025-10-29T18:56:18.770Z
Learning: In server/types.py and server/matchmaker/map_pool.py, max_tokens_per_map is intentionally typed as float (not int) because it can be dynamically calculated by VetoService.calculate_dynamic_tokens_per_map(), which returns fractional values (e.g., 2.4). While the database schema stores it as Integer for static configurations, the runtime type is float to support dynamic calculation when max_tokens_per_map == 0 in the configuration.

Applied to files:

  • server/ladder_service/veto_system.py
🧬 Code graph analysis (2)
server/ladder_service/ladder_service.py (3)
server/ladder_service/veto_system.py (1)
  • generate_initial_weights_for_match (170-209)
server/player_service.py (1)
  • all_players (72-73)
server/matchmaker/map_pool.py (1)
  • choose_map (80-108)
server/matchmaker/map_pool.py (1)
server/types.py (3)
  • id (45-45)
  • map_pool_map_version_id (47-47)
  • weight (50-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: unit-test
🔇 Additional comments (3)
server/ladder_service/veto_system.py (1)

190-190: LGTM! Improved log readability.

Removing the decorative underscores makes the debug log cleaner and easier to search/parse.

server/matchmaker/map_pool.py (1)

88-107: LGTM! Consistent log cleanup.

The removal of decorative underscores across all debug statements in choose_map improves readability and makes the log output more consistent and easier to work with.

server/ladder_service/ladder_service.py (1)

577-590: LGTM! Cleaner debug output.

The simplified log labels in _start_game are much more readable and align with the cleanup done in the other files. This makes debugging ladder match initialization easier.


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.

@K-ETFreeman K-ETFreeman merged commit 392d223 into develop Nov 9, 2025
10 checks passed
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