Skip to content

fix: balancing tie fold - #176

Merged
FrancescoBorzi merged 2 commits into
masterfrom
fix-balancing-tie-fold
Jul 2, 2026
Merged

fix: balancing tie fold#176
FrancescoBorzi merged 2 commits into
masterfrom
fix-balancing-tie-fold

Conversation

@FrancescoBorzi

@FrancescoBorzi FrancescoBorzi commented Jul 2, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • Bug Fixes
    • Improved team balancing decisions so tied teams now use average item level more consistently as a fallback.
    • Fixed balance calculations to better reflect the current candidate selection when comparing teams.
  • Documentation
    • Clarified how the average item level setting behaves in different balancing modes, including when it is always used as a tie-breaker.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Modifies the balanced-team tiebreak logic in CFBG.cpp: level-sum tiebreaking now compares average item level directly, and candidate group level sums are no longer pre-folded into context totals before team resolution. Updates accompanying documentation comments in CFBG.h and CFBG.conf.dist to reflect the revised semantics.

Changes

Balanced team tiebreak refinement

Layer / File(s) Summary
Level-sum and avg ilvl tiebreak logic
src/CFBG.cpp
ResolveBalancedTeam now resolves balanced-level ties by comparing avgIlvlA vs avgIlvlH (keeping the current pick on exact tie), and SelectBalancedGroups no longer folds the candidate group's SumPlayerLevel into ctx.levelSumA/ctx.levelSumH before the decision.
Documentation and comment updates
src/CFBG.h, conf/CFBG.conf.dist
Updates the TeamBalanceContext::levelSumA comment to reflect candidate exclusion, and clarifies the CFBG.Include.Avg.Ilvl.Enable config comment regarding its dependency on CFBG.BalancedTeams.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SelectBalancedGroups
  participant ctx as TeamBalanceContext
  participant ResolveBalancedTeam

  SelectBalancedGroups->>ctx: compute avgIlvlA/avgIlvlH without folding candidate SumPlayerLevel
  SelectBalancedGroups->>ResolveBalancedTeam: request team decision
  ResolveBalancedTeam->>ctx: compare levelSumA vs levelSumH
  alt levels balanced
    ResolveBalancedTeam->>ctx: compare avgIlvlA vs avgIlvlH
    ResolveBalancedTeam-->>SelectBalancedGroups: return chosen team (keep pick on exact tie)
  else levels unbalanced
    ResolveBalancedTeam-->>SelectBalancedGroups: return chosen team by level sum
  end
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and matches the main change: fixing balancing tie-breaking logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-balancing-tie-fold

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/CFBG.cpp (1)

784-792: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

At formation, ctx.avgIlvlA/H holds unnormalized sums rather than averages, creating team-size bias in the unconditional tiebreaker logic.

At formation time (bg == nullptr), ctx.avgIlvlA/H are assigned stagedIlvlSum values, which accumulate raw sums of individual player ilvls (line 44: SumAverageItemLevel += player->GetAverageItemLevel()). During reinforcement, these fields hold true averages from GetBGTeamAverageItemLevel() (line 151: return sum / count). Lines 227–233 compare these values directly without normalization, causing the formation-time comparison to weight team size: a larger team appears to have higher "average" ilvl even if per-player gear is identical. This bias now applies unconditionally to all CFBG.BalancedTeams = 1 queues, not just when IsEnableAvgIlvl() was explicitly enabled.

Normalize stagedIlvlSum by player count before assignment to ctx.avgIlvlA/H, or compute a true average at formation time.

🤖 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/CFBG.cpp` around lines 784 - 792, The formation-time tiebreaker in
CFBG::BalancedTeams is using raw staged item-level sums instead of true
averages, which biases comparisons toward larger teams. Update the formation
path where ctx.avgIlvlA and ctx.avgIlvlH are assigned from stagedIlvlSum so they
are normalized by each team’s player count, matching the behavior of
GetBGTeamAverageItemLevel used when bg is present. Keep the fix localized to the
average item-level calculation in CFBG::BalancedTeams / ctx initialization so
the unconditional tiebreaker logic compares like-for-like averages.
🤖 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.

Outside diff comments:
In `@src/CFBG.cpp`:
- Around line 784-792: The formation-time tiebreaker in CFBG::BalancedTeams is
using raw staged item-level sums instead of true averages, which biases
comparisons toward larger teams. Update the formation path where ctx.avgIlvlA
and ctx.avgIlvlH are assigned from stagedIlvlSum so they are normalized by each
team’s player count, matching the behavior of GetBGTeamAverageItemLevel used
when bg is present. Keep the fix localized to the average item-level calculation
in CFBG::BalancedTeams / ctx initialization so the unconditional tiebreaker
logic compares like-for-like averages.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dc697b8a-0978-4a18-a316-c7d9024e8008

📥 Commits

Reviewing files that changed from the base of the PR and between 902f3af and 88544d7.

📒 Files selected for processing (3)
  • conf/CFBG.conf.dist
  • src/CFBG.cpp
  • src/CFBG.h

@FrancescoBorzi
FrancescoBorzi merged commit c57a603 into master Jul 2, 2026
2 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