Skip to content

fix(DB): Pay Freya's Elders their 3.1 emblems - #496

Merged
Nyeriah merged 1 commit into
mainfrom
fix/freya-elder-emblems
Sep 13, 2026
Merged

Nyeriah merged 1 commit into
mainfrom
fix/freya-elder-emblems

Conversation

@Nyeriah

@Nyeriah Nyeriah commented Sep 13, 2026 •

Copy link
Copy Markdown
Member

Freya's Elders drop no emblem when killed before the encounter starts.

Freya's Gift pays one emblem plus one per Elder left alive to empower her, so an Elder killed early owes exactly the emblem the chest then no longer hands out. At 3.1 that is Emblem of Valor in 10-man and Emblem of Conquest in 25-man, matching the rest of this bracket.

Core only gives the six entries one shared Emblem of Triumph (azerothcore/azerothcore-wotlk#27634), so unlike the rest of this file the rows cannot simply be converted:

  • 33392 and 33393 have no loot id of their own, so there is no row to update.
  • 33391 points at the 10-man table 32915, and one table cannot hold Valor and Conquest at once.

So the loot ids are split and the rows rebuilt. Splitting 33391 off would have dropped the Book of Glyph Mastery it was inheriting from 32915, so that row is re-added on the 25-man side.

Applying the file twice produces identical rows, so it stays replay-safe when the DB updater re-runs it.

ulduar_hard_mode_emblems.cpp needs no change: it only swaps Valor for Conquest while a hard-mode loot mode is set, and an Elder killed before the pull loots on the default mode.

Tests Performed

Applied the full bracket file against a live world database. All three Elders yield one Emblem of Valor at 10-man and one Emblem of Conquest at 25-man, and 33391 keeps its Book of Glyph Mastery.

Known Issues

The 25-man Freya's Gift emblem counts were already inconsistent before this change and are left alone here. Counting the reference 34349 rolls alongside the direct rows, the 25-man chest pays 1 / 6 / 5 / 3 Conquest as the Elder count rises, where the 10-man ladder this bracket normalises is a clean 1 / 2 / 3 / 4 Valor plus the hard-mode Conquest.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Corrected loot rewards for Freya’s Elders in Ulduar.
    • Ten-player encounters now award Emblems of Valor.
    • Twenty-five-player encounters now award Emblems of Conquest.
    • Brightleaf now has a chance to drop the Book of Glyph Mastery.

Killed before Freya is engaged, each Elder owes the emblem her chest then
no longer hands out: Valor in 10-man, Conquest in 25-man.

Core gives all six entries one shared Emblem of Triumph, which this bracket
cannot simply convert. The 25-man Ironbranch and Stonebark have no loot id
of their own, and the 25-man Brightleaf points at the 10-man table, so one
table would have to hold both emblems. Split the loot ids and rebuild the
rows instead, keeping Brightleaf's Book of Glyph Mastery on both sizes.
@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 74aa0f32-d9ae-4878-8b74-54337d283098

📥 Commits

Reviewing files that changed from the base of the PR and between fb2faa9 and a03cff6.

📒 Files selected for processing (1)
  • src/Bracket_80_2/sql/world/progression_80_2_ulduar_emblems.sql

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The SQL update rebuilds loot for Freya’s Elders. It assigns 25-man loot IDs, removes existing emblem and book rows, and inserts separate 10-man and 25-man emblem drops plus Brightleaf Book of Glyph Mastery drops.

Changes

Freya Elder Loot

Layer / File(s) Summary
Rebuild Freya elder loot tables
src/Bracket_80_2/sql/world/progression_80_2_ulduar_emblems.sql
The script assigns loot IDs to 25-man elders, deletes existing emblem and book rows, and inserts Emblem of Valor, Emblem of Conquest, and 0.1% Book of Glyph Mastery drops.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a03cf

The migration assigns the correct emblems and Brightleaf book drops for both raid modes without introducing a known merge-blocking risk.

🚥 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 clearly summarizes the main database change: correcting emblem rewards for Freya's Elders.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/freya-elder-emblems

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.

@Nyeriah
Nyeriah merged commit dc7a2de into main Sep 13, 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