feat: BiS builds for Classic, TBC and WotLK#60
Merged
Helias merged 7 commits intoJul 18, 2026
Merged
Conversation
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds category-based NPC talent template gossip navigation, the supporting database column and migrations, idempotent Classic/TBC/WotLK template datasets, and new Classic/WotLK PvE talent mappings. ChangesTemplate navigation and data
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant npc_talent_template
participant CharacterDB
Player->>npc_talent_template: Open template gossip
npc_talent_template->>CharacterDB: Load eligible indexed templates and categories
CharacterDB-->>npc_talent_template: Return category and template data
npc_talent_template-->>Player: Show root categories and uncategorized templates
Player->>npc_talent_template: Select category or template
npc_talent_template-->>Player: Show submenu or apply selected template
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@data/sql/db-characters/base/classic_pve_p4_bis.sql`:
- Around line 91-93: Update the position-17 Bear60PvEP4BiS row to use relic item
23198 instead of duplicating main-hand item 9449, preserving the existing
template and other item fields.
In `@data/sql/db-characters/base/npc_talent_template.sql`:
- Line 40: Add an upgrade migration for the new category column used by
npc_talent_template.cpp, since the base schema only affects fresh installations.
Place the migration in the repository’s established database update mechanism,
adding category to npc_talent_template with the same VARCHAR(64) NOT NULL
DEFAULT '' definition while preserving existing installations and indexes.
In `@data/sql/db-characters/base/tbc_pve_p0_bis.sql`:
- Around line 408-423: Add the omitted equipment rows to the Phase 0 Priest
templates in data/sql/db-characters/base/tbc_pve_p0_bis.sql: add Discipline pos
2 at lines 408-423, Holy pos 2 and pos 17 at lines 437-451, and Shadow positions
5, 8, 9, and 14 at lines 465-477, preserving each template’s existing row
structure and values.
In `@data/sql/db-characters/base/tbc_pve_p2_bis.sql`:
- Around line 581-595: Regenerate the Phase 2 melee weapon rows in
data/sql/db-characters/base/tbc_pve_p2_bis.sql: add Enhancement positions 15 and
16 at lines 581-595, add the required Arms weapon position(s) at lines 728-742,
and add Fury positions 15 and 16 at lines 756-770. Populate each row with the
appropriate main-hand and off-hand equipment for its BiS template.
In `@data/sql/db-characters/base/wotlk_pve_p1_bis.sql`:
- Around line 15-16: Update the PvE Phase 1 action rows, including
Blood80PvEP1BiSTank and the remaining specs, so their talent and glyph override
fields reference matching PvE 80 keys rather than *80PvP. Add the corresponding
PvE talent/glyph keys where absent, while preserving the full and “Talents and
Glyphs only” action behavior.
In `@data/sql/db-characters/base/wotlk_pve_p4_bis.sql`:
- Around line 1914-1964: Update every Protection80PvEP4BiSTank entry shown for
RACEMASK_HUMAN, RACEMASK_A, and RACEMASK_H so its gear item IDs use the correct
Phase 4 ICC/RS Best-in-Slot items, replacing the current Phase 3 IDs in the
generated source data. Preserve the existing class, build, race-mask, slot, and
non-item fields.
- Around line 14-16: Verify the template mappings used by the generator for all
31 PvE BiS sets, starting with the Blood80PvEP4BiSTank entries, and determine
whether corresponding *80PvE talent and glyph templates exist or are intended.
If they exist, update the generator to use the matching PvE overrides instead of
*80PvP; otherwise preserve the current mappings and add a TODO documenting the
missing PvE data.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bdeff47e-cc6b-4969-a45c-1ca5a5c4a7cb
📒 Files selected for processing (19)
.gitignoredata/sql/db-characters/base/classic_pve_p2_bis.sqldata/sql/db-characters/base/classic_pve_p4_bis.sqldata/sql/db-characters/base/classic_pve_p5_bis.sqldata/sql/db-characters/base/classic_pve_p6_bis.sqldata/sql/db-characters/base/classic_pve_talents.sqldata/sql/db-characters/base/npc_talent_template.sqldata/sql/db-characters/base/npc_talent_template_data_1_80_pvp_s6.sqldata/sql/db-characters/base/npc_talent_template_data_2_70_pve_t6.sqldata/sql/db-characters/base/tbc_pve_p0_bis.sqldata/sql/db-characters/base/tbc_pve_p1_bis.sqldata/sql/db-characters/base/tbc_pve_p2_bis.sqldata/sql/db-characters/base/tbc_pve_p3_bis.sqldata/sql/db-characters/base/wotlk_pve_p1_bis.sqldata/sql/db-characters/base/wotlk_pve_p2_bis.sqldata/sql/db-characters/base/wotlk_pve_p3_bis.sqldata/sql/db-characters/base/wotlk_pve_p4_bis.sqlsrc/npc_talent_template.cppsrc/npc_talent_template.h
Comment on lines
+91
to
+93
| ('Druid', 'Bear60PvEP4BiS', @RACEMASK_ALL, 14, 17107, 1889, 0, 0, 0, 0, 0), | ||
| ('Druid', 'Bear60PvEP4BiS', @RACEMASK_ALL, 15, 9449, 1900, 0, 0, 0, 0, 0), | ||
| ('Druid', 'Bear60PvEP4BiS', @RACEMASK_ALL, 17, 9449, 0, 0, 0, 0, 0, 0); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use the Bear relic in position 17.
Line 93 duplicates main-hand item 9449 into the ranged/relic slot. The Phase 5 and 6 Bear templates use 23198 there, so this row cannot produce the intended Phase 4 set.
Proposed fix
-('Druid', 'Bear60PvEP4BiS', `@RACEMASK_ALL`, 17, 9449, 0, 0, 0, 0, 0, 0);
+('Druid', 'Bear60PvEP4BiS', `@RACEMASK_ALL`, 17, 23198, 0, 0, 0, 0, 0, 0);📝 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
| ('Druid', 'Bear60PvEP4BiS', @RACEMASK_ALL, 14, 17107, 1889, 0, 0, 0, 0, 0), | |
| ('Druid', 'Bear60PvEP4BiS', @RACEMASK_ALL, 15, 9449, 1900, 0, 0, 0, 0, 0), | |
| ('Druid', 'Bear60PvEP4BiS', @RACEMASK_ALL, 17, 9449, 0, 0, 0, 0, 0, 0); | |
| ('Druid', 'Bear60PvEP4BiS', `@RACEMASK_ALL`, 14, 17107, 1889, 0, 0, 0, 0, 0), | |
| ('Druid', 'Bear60PvEP4BiS', `@RACEMASK_ALL`, 15, 9449, 1900, 0, 0, 0, 0, 0), | |
| ('Druid', 'Bear60PvEP4BiS', `@RACEMASK_ALL`, 17, 23198, 0, 0, 0, 0, 0, 0); |
🤖 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 `@data/sql/db-characters/base/classic_pve_p4_bis.sql` around lines 91 - 93,
Update the position-17 Bear60PvEP4BiS row to use relic item 23198 instead of
duplicating main-hand item 9449, preserving the existing template and other item
fields.
| @@ -37,6 +37,7 @@ CREATE TABLE IF NOT EXISTS `mod_npc_talent_template_index` ( | |||
| `mask` int unsigned NOT NULL DEFAULT '0', | |||
| `minLevel` int unsigned NOT NULL DEFAULT '0', | |||
| `maxLevel` int unsigned NOT NULL DEFAULT '0', | |||
| `category` VARCHAR(64) NOT NULL DEFAULT '', | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for how base SQL is registered/applied and whether an ALTER exists anywhere.
fd -e sql . data/sql | sed -n '1,50p'
rg -nP 'ALTER\s+TABLE\s+`?mod_npc_talent_template_index`?' -g '*.sql'
rg -nP 'db-characters|AddSC|Database.*(base|updates)|sql/db' -n src include CMakeLists.txt 2>/dev/null | sed -n '1,40p'Repository: azerothcore/mod-npc-talent-template
Length of output: 1110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== relevant SQL files ==\n'
git ls-files 'data/sql/**' | sed -n '1,120p'
printf '\n== module metadata / docs references ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'db-characters|db-world|base/|updates/|sql.*base|sql.*updates|sql/server' .
printf '\n== inspect candidate docs and manifest files ==\n'
for f in README.md mod.cpp CMakeLists.txt .github/workflows/* 2>/dev/null; do
[ -e "$f" ] && echo "--- $f" && sed -n '1,220p' "$f"
doneRepository: azerothcore/mod-npc-talent-template
Length of output: 1177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file sizes ==\n'
wc -l data/sql/db-characters/base/npc_talent_template.sql data/sql/db-world/base/npc_talent_template_data.sql 2>/dev/null || true
printf '\n== outline relevant SQL files ==\n'
ast-grep outline data/sql/db-characters/base/npc_talent_template.sql --view expanded || true
ast-grep outline data/sql/db-world/base/npc_talent_template_data.sql --view expanded || true
printf '\n== show surrounding lines in the characters schema file ==\n'
sed -n '1,220p' data/sql/db-characters/base/npc_talent_template.sql
printf '\n== show surrounding lines in the world schema file ==\n'
sed -n '1,220p' data/sql/db-world/base/npc_talent_template_data.sqlRepository: azerothcore/mod-npc-talent-template
Length of output: 5110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for update SQL patterns in module ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'db-characters/updates|db-world/updates|characters/updates|world/updates|Base.*sql|updates/.*sql' .
printf '\n== inspect any loader/registration references ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'module.*sql|sql.*loader|Load.*sql|Apply.*sql|database.*sql|characters db|world db' src include . 2>/dev/null || trueRepository: azerothcore/mod-npc-talent-template
Length of output: 221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== top-level files ==\n'
git ls-files | sed -n '1,200p'
printf '\n== README / docs ==\n'
for f in README.md readme.md docs/*; do
[ -e "$f" ] || continue
echo "--- $f ---"
sed -n '1,240p' "$f"
done
printf '\n== search for installation/update guidance ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'install|update|upgrad|base/sql|data/sql|db-characters/base|db-world/base|module sql|migrations' README.md . 2>/dev/null || trueRepository: azerothcore/mod-npc-talent-template
Length of output: 6036
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== find any update folders or versioned SQL files ==\n'
fd -t d 'updates' .
fd -t f -e sql .Repository: azerothcore/mod-npc-talent-template
Length of output: 1164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== relevant loader code ==\n'
sed -n '1,220p' src/npc_talent_template.cpp
printf '\n== search for table create / select columns ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'mod_npc_talent_template_index|category|SELECT .*category|INSERT .*mod_npc_talent_template_index|CREATE TABLE IF NOT EXISTS `mod_npc_talent_template_index`' src data/sql README.md
printf '\n== include.sh ==\n'
sed -n '1,240p' include.shRepository: azerothcore/mod-npc-talent-template
Length of output: 50392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== module SQL filenames only ==\n'
git ls-files 'data/sql/db-characters/base/*' 'data/sql/db-world/base/*' | sed -n '1,120p'
printf '\n== any mention of acore_characters.updates ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'acore_characters\.updates|DELETE FROM .*updates|INSERT INTO .*updates|updates WHERE name=' .Repository: azerothcore/mod-npc-talent-template
Length of output: 1494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== include.sh ==\n'
sed -n '1,220p' include.sh
printf '\n== relevant README lines ==\n'
nl -ba README.md | sed -n '28,45p'
printf '\n== module install/update markers ==\n'
rg -n --hidden --glob '!**/.git/**' -e 'base/.*sql|updates WHERE name|DROP TABLE IF EXISTS acore_characters\.mod_npc_talent_template|templatenpc reload' README.md data/sql src include.shRepository: azerothcore/mod-npc-talent-template
Length of output: 260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== top-level build files ==\n'
git ls-files | rg '(^|/)(CMakeLists\.txt|include\.sh|README\.md|\.github/workflows/.*)$'
printf '\n== CMakeLists snippets ==\n'
[ -f CMakeLists.txt ] && sed -n '1,240p' CMakeLists.txt || trueRepository: azerothcore/mod-npc-talent-template
Length of output: 320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== include.sh =='
cat -n include.sh
echo
echo '== README excerpt =='
cat -n README.md | sed -n '28,45p'Repository: azerothcore/mod-npc-talent-template
Length of output: 1818
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== CMakeLists.txt =='
[ -f CMakeLists.txt ] && cat -n CMakeLists.txt | sed -n '1,220p' || trueRepository: azerothcore/mod-npc-talent-template
Length of output: 194
Add an upgrade migration for category. data/sql/db-characters/base/npc_talent_template.sql only covers fresh installs, and this repo has no module updates/ path for existing databases. Upgraded servers will keep the old mod_npc_talent_template_index schema, so the new SELECT ... category in src/npc_talent_template.cpp will fail unless the column is added in a migration.
🤖 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 `@data/sql/db-characters/base/npc_talent_template.sql` at line 40, Add an
upgrade migration for the new category column used by npc_talent_template.cpp,
since the base schema only affects fresh installations. Place the migration in
the repository’s established database update mechanism, adding category to
npc_talent_template with the same VARCHAR(64) NOT NULL DEFAULT '' definition
while preserving existing installations and indexes.
Comment on lines
+408
to
+423
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 0, 32090, 3001, 2740, 2835, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 1, 30377, 0, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 4, 21875, 2661, 2734, 2740, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 5, 21873, 0, 2734, 2740, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 6, 24261, 2748, 2740, 2734, 2728, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 7, 29251, 2656, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 8, 29183, 2650, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 9, 27536, 2322, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 10, 29373, 0, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 11, 29168, 0, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 12, 29376, 0, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 13, 21625, 0, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 14, 29354, 2621, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 15, 23556, 2505, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 16, 29353, 907, 0, 0, 0, 0, 0), | ||
| ('Priest', 'Discipline70PvEP0BiS', @RACEMASK_ALL, 17, 27885, 0, 0, 0, 0, 0, 0); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Regenerate the incomplete Phase 0 Priest templates. Several equipment rows were omitted, so these actions cannot produce complete BiS sets.
data/sql/db-characters/base/tbc_pve_p0_bis.sql#L408-L423: add the missing Disciplinepos = 2row.data/sql/db-characters/base/tbc_pve_p0_bis.sql#L437-L451: add the missing Holypos = 2andpos = 17rows.data/sql/db-characters/base/tbc_pve_p0_bis.sql#L465-L477: add the missing Shadow positions 5, 8, 9, and 14.
📍 Affects 1 file
data/sql/db-characters/base/tbc_pve_p0_bis.sql#L408-L423(this comment)data/sql/db-characters/base/tbc_pve_p0_bis.sql#L437-L451data/sql/db-characters/base/tbc_pve_p0_bis.sql#L465-L477
🤖 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 `@data/sql/db-characters/base/tbc_pve_p0_bis.sql` around lines 408 - 423, Add
the omitted equipment rows to the Phase 0 Priest templates in
data/sql/db-characters/base/tbc_pve_p0_bis.sql: add Discipline pos 2 at lines
408-423, Holy pos 2 and pos 17 at lines 437-451, and Shadow positions 5, 8, 9,
and 14 at lines 465-477, preserving each template’s existing row structure and
values.
Comment on lines
+581
to
+595
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 0, 30190, 3003, 2829, 2735, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 1, 30017, 0, 0, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 2, 30055, 2986, 2726, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 4, 30185, 2661, 2726, 2731, 2735, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 5, 30106, 0, 2726, 2731, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 6, 30192, 3012, 2726, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 7, 30039, 2939, 0, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 8, 30091, 2647, 2735, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 9, 30189, 2564, 0, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 10, 29997, 0, 0, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 11, 30052, 0, 0, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 12, 28830, 0, 0, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 13, 29383, 0, 0, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 14, 24259, 368, 2726, 0, 0, 0, 0), | ||
| ('Shaman', 'Enhancement70PvEP2BiS', @RACEMASK_ALL, 17, 27815, 0, 0, 0, 0, 0, 0); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Regenerate the Phase 2 melee weapon rows. These full BiS templates contain no main-hand or off-hand equipment.
data/sql/db-characters/base/tbc_pve_p2_bis.sql#L581-L595: add Enhancement positions 15 and 16.data/sql/db-characters/base/tbc_pve_p2_bis.sql#L728-L742: add the Arms weapon position(s).data/sql/db-characters/base/tbc_pve_p2_bis.sql#L756-L770: add Fury positions 15 and 16.
📍 Affects 1 file
data/sql/db-characters/base/tbc_pve_p2_bis.sql#L581-L595(this comment)data/sql/db-characters/base/tbc_pve_p2_bis.sql#L728-L742data/sql/db-characters/base/tbc_pve_p2_bis.sql#L756-L770
🤖 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 `@data/sql/db-characters/base/tbc_pve_p2_bis.sql` around lines 581 - 595,
Regenerate the Phase 2 melee weapon rows in
data/sql/db-characters/base/tbc_pve_p2_bis.sql: add Enhancement positions 15 and
16 at lines 581-595, add the required Arms weapon position(s) at lines 728-742,
and add Fury positions 15 and 16 at lines 756-770. Populate each row with the
appropriate main-hand and off-hand equipment for its BiS template.
Comment on lines
+15
to
+16
| ('Death Knight', 'Blood80PvEP1BiSTank', @ACTION+000, '|cff00ff00|TInterface\\icons\\spell_deathknight_bloodpresence:30|t|r Use Blood PvE P1 BiS Tank', 7, @MINLEVEL, @MAXLEVEL, 'Blood80PvP', 'Blood80PvP', 'WotLK Phase 1'), | ||
| ('Death Knight', 'Blood80PvEP1BiSTank', @ACTION+001, '|cff00ff00|TInterface\\icons\\spell_deathknight_bloodpresence:30|t|r Use Blood PvE P1 BiS Tank (Talents and Glyphs only)', 6, @MINLEVEL, @MAXLEVEL, 'Blood80PvP', 'Blood80PvP', 'WotLK Phase 1'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not apply PvP talents and glyphs from PvE actions.
These rows—and the remaining Phase 1 specs—point both overrides to *80PvP. Consequently, the advertised PvE full and “Talents and Glyphs only” actions apply PvP builds. Add matching PvE talent/glyph keys and reference them here.
🤖 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 `@data/sql/db-characters/base/wotlk_pve_p1_bis.sql` around lines 15 - 16,
Update the PvE Phase 1 action rows, including Blood80PvEP1BiSTank and the
remaining specs, so their talent and glyph override fields reference matching
PvE 80 keys rather than *80PvP. Add the corresponding PvE talent/glyph keys
where absent, while preserving the full and “Talents and Glyphs only” action
behavior.
Comment on lines
+14
to
+16
| INSERT INTO `mod_npc_talent_template_index` (`playerClass`, `playerSpec`, `gossipAction`, `gossipText`, `mask`, `minLevel`, `maxLevel`, `glyphOverride`, `talentOverride`, `category`) VALUES | ||
| ('Death Knight', 'Blood80PvEP4BiSTank', @ACTION+000, '|cff00ff00|TInterface\\icons\\spell_deathknight_bloodpresence:30|t|r Use Blood PvE P4 BiS Tank', 7, @MINLEVEL, @MAXLEVEL, 'Blood80PvP', 'Blood80PvP', 'WotLK Phase 4'), | ||
| ('Death Knight', 'Blood80PvEP4BiSTank', @ACTION+001, '|cff00ff00|TInterface\\icons\\spell_deathknight_bloodpresence:30|t|r Use Blood PvE P4 BiS Tank (Talents and Glyphs only)', 6, @MINLEVEL, @MAXLEVEL, 'Blood80PvP', 'Blood80PvP', 'WotLK Phase 4'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Verify the use of PvP talent and glyph overrides for PvE templates.
The talentOverride and glyphOverride columns are mapped to Blood80PvP. This pattern of using *80PvP instead of *80PvE templates affects all 31 PvE BiS sets in this file.
If PvE templates (e.g., Blood80PvE) exist in the database or are intended to be added, please update the generator script to use them so players receive the correct PvE talents and glyphs. If they do not exist yet, consider leaving a TODO or tracking this missing data in a separate issue.
🤖 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 `@data/sql/db-characters/base/wotlk_pve_p4_bis.sql` around lines 14 - 16,
Verify the template mappings used by the generator for all 31 PvE BiS sets,
starting with the Blood80PvEP4BiSTank entries, and determine whether
corresponding *80PvE talent and glyph templates exist or are intended. If they
exist, update the generator to use the matching PvE overrides instead of *80PvP;
otherwise preserve the current mappings and add a TODO documenting the missing
PvE data.
Comment on lines
+1914
to
+1964
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 0, 48433, 3818, 3637, 3537, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 1, 47133, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 2, 48455, 3852, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 4, 48451, 3832, 3575, 3532, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 5, 47076, 3601, 3537, 3293, 0, 0, 3293), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 6, 47061, 3822, 3537, 3293, 3575, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 7, 47003, 3232, 3537, 3532, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 8, 47111, 3850, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 9, 48453, 3860, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 10, 47157, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 11, 47955, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 12, 47088, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 13, 47735, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 14, 47549, 3605, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 15, 47506, 3870, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 16, 45587, 3849, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_HUMAN, 17, 47660, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 0, 48433, 3818, 3637, 3537, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 1, 47133, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 2, 48455, 3852, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 4, 48451, 3832, 3575, 3532, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 5, 47076, 3601, 3537, 3293, 0, 0, 3293), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 6, 47061, 3822, 3537, 3293, 3575, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 7, 47003, 3232, 3537, 3532, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 8, 47111, 3850, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 9, 48453, 3860, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 10, 47157, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 11, 47955, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 12, 47088, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 13, 47735, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 14, 47549, 3605, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 15, 47506, 3870, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 16, 45587, 3849, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_A, 17, 47660, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 0, 48468, 3818, 3637, 3537, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 1, 47468, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 2, 48470, 3852, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 4, 48466, 3832, 3575, 3532, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 5, 47444, 3601, 3537, 3293, 0, 0, 3293), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 6, 47434, 3822, 3537, 3293, 3575, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 7, 47430, 3232, 3537, 3532, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 8, 47459, 3850, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 9, 48467, 3860, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 10, 47476, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 11, 48027, 0, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 12, 47432, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 13, 47735, 0, 0, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 14, 47550, 3605, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 15, 47513, 3870, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 16, 45587, 3849, 3532, 0, 0, 0, 0), | ||
| ('Warrior', 'Protection80PvEP4BiSTank', @RACEMASK_H, 17, 47660, 0, 0, 0, 0, 0, 0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Update Warrior Protection gear to use Phase 4 items.
The generated gear for Protection80PvEP4BiSTank consists of Phase 3 items (e.g., 48433 - Wrynn's Greathelm of Triumph, 47133, 48455) rather than Phase 4 (ICC/RS) items. It appears this dataset was either duplicated from the Phase 3 script or the scraping tooling failed for this specific build.
Please update the generator's source data to use the correct Phase 4 Best-in-Slot items (item IDs 50xxx-51xxx) for Protection Warriors.
🤖 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 `@data/sql/db-characters/base/wotlk_pve_p4_bis.sql` around lines 1914 - 1964,
Update every Protection80PvEP4BiSTank entry shown for RACEMASK_HUMAN,
RACEMASK_A, and RACEMASK_H so its gear item IDs use the correct Phase 4 ICC/RS
Best-in-Slot items, replacing the current Phase 3 IDs in the generated source
data. Preserve the existing class, build, race-mask, slot, and non-item fields.
Member
|
build is failing, I think that the directory structure of the sql data should be updated to import automatically the base sql files |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
data/sql/db-characters/base/31_tbc_pve_p1_bis.sql (1)
144-158: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRegenerate the incomplete Phase 1–3 gear templates.
The full BiS actions use mask
7, but these generated templates omit required equipment rows and therefore cannot apply complete sets.
data/sql/db-characters/base/31_tbc_pve_p1_bis.sql#L144-L158: add Beastmastery positions 15 and 16.data/sql/db-characters/base/31_tbc_pve_p1_bis.sql#L442-L457: add Holy Priest position 17.data/sql/db-characters/base/31_tbc_pve_p1_bis.sql#L471-L485: add Shadow Priest positions 8 and 14.data/sql/db-characters/base/31_tbc_pve_p1_bis.sql#L739-L753: add the Arms main-hand position 15.data/sql/db-characters/base/31_tbc_pve_p1_bis.sql#L767-L781: add Fury positions 15 and 16.data/sql/db-characters/base/32_tbc_pve_p2_bis.sql#L203-L216: add Survival positions 11, 15, and 16.data/sql/db-characters/base/32_tbc_pve_p2_bis.sql#L410-L425: add Discipline Priest position 8.data/sql/db-characters/base/32_tbc_pve_p2_bis.sql#L439-L453: add Holy Priest positions 8 and 17.data/sql/db-characters/base/32_tbc_pve_p2_bis.sql#L557-L572: add Elemental position 16.data/sql/db-characters/base/32_tbc_pve_p2_bis.sql#L614-L629: add Restoration Shaman position 16.data/sql/db-characters/base/33_tbc_pve_p3_bis.sql#L262-L277: add Fire Mage position 11.data/sql/db-characters/base/33_tbc_pve_p3_bis.sql#L291-L306: add Frost Mage position 11.data/sql/db-characters/base/33_tbc_pve_p3_bis.sql#L409-L422: add Discipline Priest positions 5, 8, and 11.data/sql/db-characters/base/33_tbc_pve_p3_bis.sql#L436-L448: add Holy Priest positions 5, 8, 11, and 17.data/sql/db-characters/base/33_tbc_pve_p3_bis.sql#L462-L477: add Shadow Priest position 11.data/sql/db-characters/base/33_tbc_pve_p3_bis.sql#L611-L625: add Restoration Shaman positions 11 and 16.🤖 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 `@data/sql/db-characters/base/31_tbc_pve_p1_bis.sql` around lines 144 - 158, Regenerate the incomplete BiS template rows so every listed class/spec action uses mask 7 with complete equipment positions. Update data/sql/db-characters/base/31_tbc_pve_p1_bis.sql lines 144-158, 442-457, 471-485, 739-753, and 767-781 with the specified missing positions; data/sql/db-characters/base/32_tbc_pve_p2_bis.sql lines 203-216, 410-425, 439-453, 557-572, and 614-629; and data/sql/db-characters/base/33_tbc_pve_p3_bis.sql lines 262-277, 291-306, 409-422, 436-448, 462-477, and 611-625. Preserve each existing template’s class/spec, race mask, position ordering, and generated equipment values while adding only the requested rows.data/sql/db-characters/base/11_npc_talent_template_data_2_70_pve_t6.sql (3)
41-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the mask to prevent unintended gear overrides.
These secondary "Talents and Glyphs only" entries for Rogues incorrectly use a mask of
7(which commands the engine to apply Gear, Talents, and Glyphs) instead of6(Talents and Glyphs only). Consequently, selecting these secondary options will incorrectly override the player's gear.Additionally, the Assassination entry is completely missing the "(Talents and Glyphs only)" string, creating two identical user-facing menu options that both do the exact same thing.
🐛 Proposed fix
-('Rogue', 'Assassination70PvET6', `@ACTION`+024, '|cff00ff00|TInterface\\icons\\ability_rogue_eviscerate:30|t|r Use Assassination T6 PvE', 7, `@MINLEVEL`, `@MAXLEVEL`, 'Assassination70PvE', 'Assassination70PvE', 'TBC Tier 6'), -('Rogue', 'Combat70PvET6', `@ACTION`+025, '|cff00ff00|TInterface\\icons\\ability_backstab:30|t|r Use Combat T6 PvE (Talents and Glyphs only)', 7, `@MINLEVEL`, `@MAXLEVEL`, 'Combat70PvE', 'Combat70PvE', 'TBC Tier 6'), +('Rogue', 'Assassination70PvET6', `@ACTION`+024, '|cff00ff00|TInterface\\icons\\ability_rogue_eviscerate:30|t|r Use Assassination T6 PvE (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Assassination70PvE', 'Assassination70PvE', 'TBC Tier 6'), +('Rogue', 'Combat70PvET6', `@ACTION`+025, '|cff00ff00|TInterface\\icons\\ability_backstab:30|t|r Use Combat T6 PvE (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Combat70PvE', 'Combat70PvE', 'TBC Tier 6'),🤖 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 `@data/sql/db-characters/base/11_npc_talent_template_data_2_70_pve_t6.sql` around lines 41 - 42, Update the Rogue secondary entries Assassination70PvET6 and Combat70PvET6 to use mask 6 instead of 7, preserving player gear while applying only talents and glyphs. Add “(Talents and Glyphs only)” to the Assassination70PvET6 description so it is distinct from the primary option.
24-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStandardize the gossip text formatting.
The string formatting is duplicated and inconsistent with the rest of the template entries.
📝 Proposed fix
-('Mage', 'Arcane70PvET6', `@ACTION`+003, '|cff00ff00|TInterface\\icons\\spell_holy_magicalsentry:30|t|r Use Arcane PvE (Talents and Glyphs only) T6 PvE', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Arcane70PvE', 'Arcane70PvE', 'TBC Tier 6'), +('Mage', 'Arcane70PvET6', `@ACTION`+003, '|cff00ff00|TInterface\\icons\\spell_holy_magicalsentry:30|t|r Use Arcane T6 PvE (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Arcane70PvE', 'Arcane70PvE', 'TBC Tier 6'),🤖 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 `@data/sql/db-characters/base/11_npc_talent_template_data_2_70_pve_t6.sql` at line 24, Standardize the gossip text in the Arcane70PvET6 entry to match the formatting used by the surrounding talent template rows, removing the duplicated icon/color markup while preserving the existing action, labels, and tier metadata.
48-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStandardize the gossip text formatting.
The string has an inconsistent suffix order compared to the other class entries.
📝 Proposed fix
-('Druid', 'Bear70PvET6', `@ACTION`+033, '|cff00ff00|TInterface\\icons\\ability_racial_bearform:30|t|r Use Feral Bear (Talents and Glyphs only) T6 PvE', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Bear70PvE', 'Bear70PvE', 'TBC Tier 6'), +('Druid', 'Bear70PvET6', `@ACTION`+033, '|cff00ff00|TInterface\\icons\\ability_racial_bearform:30|t|r Use Feral Bear T6 PvE (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Bear70PvE', 'Bear70PvE', 'TBC Tier 6'),🤖 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 `@data/sql/db-characters/base/11_npc_talent_template_data_2_70_pve_t6.sql` at line 48, Update the gossip text for the Bear70PvET6 entry to use the same suffix ordering as the surrounding class entries, preserving the existing icon, action, and talent descriptions.data/sql/db-characters/base/10_npc_talent_template_data_1_80_pvp_s6.sql (1)
31-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStandardize the gossip text.
The gossip text for these secondary Priest entries is missing the "S6 PvP" string that is consistently used across all other class entries to clarify which specific tier's talents/glyphs are being applied.
📝 Proposed fix
-('Priest', 'Holy80PvPS6', `@ACTION`+010, '|cff00ff00|TInterface\\icons\\spell_holy_holybolt:30|t|r Use Holy (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Holy80PvP', 'Holy80PvP', 'WotLK PvP S6'), -('Priest', 'Shadow80PvPS6', `@ACTION`+011, '|cff00ff00|TInterface\\icons\\spell_shadow_shadowwordpain:30|t|r Use Shadow (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Shadow80PvP', 'Shadow80PvP', 'WotLK PvP S6'), +('Priest', 'Holy80PvPS6', `@ACTION`+010, '|cff00ff00|TInterface\\icons\\spell_holy_holybolt:30|t|r Use Holy S6 PvP (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Holy80PvP', 'Holy80PvP', 'WotLK PvP S6'), +('Priest', 'Shadow80PvPS6', `@ACTION`+011, '|cff00ff00|TInterface\\icons\\spell_shadow_shadowwordpain:30|t|r Use Shadow S6 PvP (Talents and Glyphs only)', 6, `@MINLEVEL`, `@MAXLEVEL`, 'Shadow80PvP', 'Shadow80PvP', 'WotLK PvP S6'),🤖 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 `@data/sql/db-characters/base/10_npc_talent_template_data_1_80_pvp_s6.sql` around lines 31 - 32, The gossip text for the secondary Priest entries in the talent template data is missing the standard “S6 PvP” qualifier. Update the descriptions for Holy80PvPS6 and Shadow80PvPS6 to include “S6 PvP” while preserving their existing class, specialization, and talents/glyphs wording.
🤖 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 `@data/sql/db-characters/base/20_classic_pve_talents.sql`:
- Line 9: Update the cleanup DELETE statements in
data/sql/db-characters/base/20_classic_pve_talents.sql:9-9,
21_classic_pve_p2_bis.sql:7-8, 22_classic_pve_p4_bis.sql:7-8,
23_classic_pve_p5_bis.sql:7-8, and 24_classic_pve_p6_bis.sql:7-8 to filter by
both playerClass and playerSpec, using the exact class/spec pairs inserted by
each file rather than playerSpec alone.
In `@data/sql/db-characters/base/40_wotlk_pve_p1_bis.sql`:
- Around line 20-21: Replace the PvP glyphOverride suffix with the matching PvE
suffix for every PvE template: update all affected entries in
data/sql/db-characters/base/40_wotlk_pve_p1_bis.sql lines 20-21,
data/sql/db-characters/base/41_wotlk_pve_p2_bis.sql lines 20-21,
data/sql/db-characters/base/42_wotlk_pve_p3_bis.sql lines 20-21, and
data/sql/db-characters/base/43_wotlk_pve_p4_bis.sql lines 20-21, applying the
equivalent change across every spec in each file.
- Around line 337-338: Feral Tank templates currently reuse the Feral DPS talent
configuration. In the Feral80PvEP1BiSTank entries, update the talentOverride to
FeralTank80PvE and optionally replace the cat-form icon with the bear-form icon;
apply the same change in
data/sql/db-characters/base/40_wotlk_pve_p1_bis.sql:337-338,
data/sql/db-characters/base/41_wotlk_pve_p2_bis.sql:334-335,
data/sql/db-characters/base/42_wotlk_pve_p3_bis.sql:334-335, and
data/sql/db-characters/base/43_wotlk_pve_p4_bis.sql:331-332. In
data/sql/db-characters/base/44_wotlk_pve_talents.sql:114-170, add the dedicated
talent ID mappings for playerSpec FeralTank80PvE.
---
Outside diff comments:
In `@data/sql/db-characters/base/10_npc_talent_template_data_1_80_pvp_s6.sql`:
- Around line 31-32: The gossip text for the secondary Priest entries in the
talent template data is missing the standard “S6 PvP” qualifier. Update the
descriptions for Holy80PvPS6 and Shadow80PvPS6 to include “S6 PvP” while
preserving their existing class, specialization, and talents/glyphs wording.
In `@data/sql/db-characters/base/11_npc_talent_template_data_2_70_pve_t6.sql`:
- Around line 41-42: Update the Rogue secondary entries Assassination70PvET6 and
Combat70PvET6 to use mask 6 instead of 7, preserving player gear while applying
only talents and glyphs. Add “(Talents and Glyphs only)” to the
Assassination70PvET6 description so it is distinct from the primary option.
- Line 24: Standardize the gossip text in the Arcane70PvET6 entry to match the
formatting used by the surrounding talent template rows, removing the duplicated
icon/color markup while preserving the existing action, labels, and tier
metadata.
- Line 48: Update the gossip text for the Bear70PvET6 entry to use the same
suffix ordering as the surrounding class entries, preserving the existing icon,
action, and talent descriptions.
In `@data/sql/db-characters/base/31_tbc_pve_p1_bis.sql`:
- Around line 144-158: Regenerate the incomplete BiS template rows so every
listed class/spec action uses mask 7 with complete equipment positions. Update
data/sql/db-characters/base/31_tbc_pve_p1_bis.sql lines 144-158, 442-457,
471-485, 739-753, and 767-781 with the specified missing positions;
data/sql/db-characters/base/32_tbc_pve_p2_bis.sql lines 203-216, 410-425,
439-453, 557-572, and 614-629; and
data/sql/db-characters/base/33_tbc_pve_p3_bis.sql lines 262-277, 291-306,
409-422, 436-448, 462-477, and 611-625. Preserve each existing template’s
class/spec, race mask, position ordering, and generated equipment values while
adding only the requested rows.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 29345ca2-1ed3-439c-ac08-fdfb9ae21120
📒 Files selected for processing (18)
data/sql/db-characters/base/00_npc_talent_template.sqldata/sql/db-characters/base/01_mod_npc_talent_template_index_category.sqldata/sql/db-characters/base/10_npc_talent_template_data_1_80_pvp_s6.sqldata/sql/db-characters/base/11_npc_talent_template_data_2_70_pve_t6.sqldata/sql/db-characters/base/20_classic_pve_talents.sqldata/sql/db-characters/base/21_classic_pve_p2_bis.sqldata/sql/db-characters/base/22_classic_pve_p4_bis.sqldata/sql/db-characters/base/23_classic_pve_p5_bis.sqldata/sql/db-characters/base/24_classic_pve_p6_bis.sqldata/sql/db-characters/base/30_tbc_pve_p0_bis.sqldata/sql/db-characters/base/31_tbc_pve_p1_bis.sqldata/sql/db-characters/base/32_tbc_pve_p2_bis.sqldata/sql/db-characters/base/33_tbc_pve_p3_bis.sqldata/sql/db-characters/base/40_wotlk_pve_p1_bis.sqldata/sql/db-characters/base/41_wotlk_pve_p2_bis.sqldata/sql/db-characters/base/42_wotlk_pve_p3_bis.sqldata/sql/db-characters/base/43_wotlk_pve_p4_bis.sqldata/sql/db-characters/base/44_wotlk_pve_talents.sql
|
|
||
| -- Idempotency: remove this file's templates first so re-application cannot duplicate rows | ||
| -- (the index table has no unique key; without this a re-applied file would append copies). | ||
| DELETE FROM `mod_npc_talent_template_talents` WHERE `playerSpec` IN ('Balance60PvE', 'Bear60PvE', 'Cat60PvE', 'Restoration60PvE', 'Marksmanship60PvE', 'Fire60PvE', 'Holy60PvE', 'Protection60PvE', 'Shadow60PvE', 'Combat60PvE', 'Elemental60PvE', 'Enhancement60PvE', 'Affliction60PvE', 'Fury60PvE'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File list =="
git ls-files 'data/sql/db-characters/base/*.sql' | sed -n '1,40p'
echo
echo "== Relevant table definitions and inserts =="
rg -n --no-heading -C 3 'CREATE TABLE `mod_npc_talent_template_(talents|index|gear)`|INSERT INTO `mod_npc_talent_template_(talents|index|gear)`|DELETE FROM `mod_npc_talent_template_(talents|index|gear)`' data/sql/db-characters -g '*.sql'Repository: azerothcore/mod-npc-talent-template
Length of output: 50392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect target files =="
for f in \
data/sql/db-characters/base/20_classic_pve_talents.sql \
data/sql/db-characters/base/21_classic_pve_p2_bis.sql \
data/sql/db-characters/base/22_classic_pve_p4_bis.sql \
data/sql/db-characters/base/23_classic_pve_p5_bis.sql \
data/sql/db-characters/base/24_classic_pve_p6_bis.sql
do
echo "--- $f"
sed -n '1,120p' "$f"
done
echo
echo "== Table schemas if present =="
rg -n --no-heading -C 4 'CREATE TABLE `mod_npc_talent_template_(talents|index|gear)`' data/sql/db-characters -g '*.sql'Repository: azerothcore/mod-npc-talent-template
Length of output: 46488
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in data/sql/db-characters/base/20_classic_pve_talents.sql \
data/sql/db-characters/base/21_classic_pve_p2_bis.sql \
data/sql/db-characters/base/22_classic_pve_p4_bis.sql \
data/sql/db-characters/base/23_classic_pve_p5_bis.sql \
data/sql/db-characters/base/24_classic_pve_p6_bis.sql
do
echo "### $f"
nl -ba "$f" | sed -n '1,40p'
done
echo "### schema matches"
rg -n --no-heading -C 2 'playerClass|playerSpec' data/sql/db-characters -g '*.sql' | sed -n '1,120p'Repository: azerothcore/mod-npc-talent-template
Length of output: 273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
from collections import defaultdict
root = Path("data/sql/db-characters/base")
files = [
root / "20_classic_pve_talents.sql",
root / "21_classic_pve_p2_bis.sql",
root / "22_classic_pve_p4_bis.sql",
root / "23_classic_pve_p5_bis.sql",
root / "24_classic_pve_p6_bis.sql",
]
pairs = defaultdict(set)
for path in files:
text = path.read_text(encoding="utf-8")
for m in re.finditer(r"\('([^']+)',\s*'([^']+)'", text):
player_class, player_spec = m.groups()
pairs[player_spec].add(player_class)
collisions = {spec: sorted(classes) for spec, classes in pairs.items() if len(classes) > 1}
print("cross-class playerSpec collisions in target files:", len(collisions))
for spec, classes in sorted(collisions.items()):
print(spec, "=>", classes)
PYRepository: azerothcore/mod-npc-talent-template
Length of output: 1056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
from collections import defaultdict
root = Path("data/sql/db-characters/base")
files = sorted(root.glob("*.sql"))
pairs = defaultdict(set)
for path in files:
text = path.read_text(encoding="utf-8", errors="ignore")
for m in re.finditer(r"\('([^']+)',\s*'([^']+)'", text):
player_class, player_spec = m.groups()
pairs[player_spec].add(player_class)
collisions = {spec: sorted(classes) for spec, classes in pairs.items() if len(classes) > 1}
print("files scanned:", len(files))
print("specs seen:", len(pairs))
print("cross-class playerSpec collisions:", len(collisions))
for spec, classes in sorted(list(collisions.items())[:50]):
print(f"{spec} -> {', '.join(classes)}")
PYRepository: azerothcore/mod-npc-talent-template
Length of output: 2084
Scope these cleanup DELETEs by both playerClass and playerSpec.
Several playerSpec values are shared across classes here (Holy60PvE, Restoration60PvE, Protection60PvE), so deleting by spec alone can remove unrelated rows. Use the exact (playerClass, playerSpec) pairs inserted in each file.
📍 Affects 5 files
data/sql/db-characters/base/20_classic_pve_talents.sql#L9-L9(this comment)data/sql/db-characters/base/21_classic_pve_p2_bis.sql#L7-L8data/sql/db-characters/base/22_classic_pve_p4_bis.sql#L7-L8data/sql/db-characters/base/23_classic_pve_p5_bis.sql#L7-L8data/sql/db-characters/base/24_classic_pve_p6_bis.sql#L7-L8
🤖 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 `@data/sql/db-characters/base/20_classic_pve_talents.sql` at line 9, Update the
cleanup DELETE statements in
data/sql/db-characters/base/20_classic_pve_talents.sql:9-9,
21_classic_pve_p2_bis.sql:7-8, 22_classic_pve_p4_bis.sql:7-8,
23_classic_pve_p5_bis.sql:7-8, and 24_classic_pve_p6_bis.sql:7-8 to filter by
both playerClass and playerSpec, using the exact class/spec pairs inserted by
each file rather than playerSpec alone.
Helias
changed the title
Member
|
thanks for the PR! |
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
3 participants
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.
Changes Proposed:
Main goal: provide Best-in-Slot (BiS) sets for every class and build, at each stage of the game across Classic, TBC, and WotLK PvE content.
To achieve that, this PR:
categorycolumn onmod_npc_talent_template_index) so the growing number of sets stays navigable: categorized specs are shown behind a>> Categorysub-menu with a<< Backbutton, while uncategorized specs remain directly in the root menu (existing setups are unaffected)...._1_80_pvp_s6,..._2_70_pve_t6) to use the new category grouping.Issues Addressed:
SOURCE:
Tests Performed:
static).<< Backto root, uncategorized specs still shown).How to Test the Changes:
acore_charactersdatabase and start the worldserver.Summary by CodeRabbit
New Features
Bug Fixes
Chores