Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retail: Fix shaman class hall's version of Firelands showing Things from Mount Hyjal. #1890

Merged
merged 7 commits into from
Jan 22, 2025

Conversation

donkers-mike
Copy link
Member

As described in this issue, the Shaman class hall's version of Firelands (a scenario) was showing Things to be gained from Mount Hyjal while this instance of Firelands has 0 connections to that zone.

New header for this version of Firelands is created to remain in line with ATT's look and feel and to prevent a "default" header from showing.

I've personally tested this implementation on two (separate) shamans both in the class hall version of Firelands as well as the actual raid and did not find inconsistencies.

Copy link
Collaborator

@ImUnicke ImUnicke left a comment

Choose a reason for hiding this comment

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

The new 'Firelands' file should be either:
1) Removed (and just allow 'mapped' content to generate in the appropriate minilist)
2) Moved in the hierarchy under Class Halls > Shaman (remains empty itself, Quests remain under their current Source)

@donkers-mike donkers-mike requested a review from ImUnicke January 22, 2025 11:16
@donkers-mike
Copy link
Member Author

@ImUnicke I've updated the Firelands file to move the zone under EF > Class Halls > Shaman and let mapping add the quests to it.

Result now is:
image

@DFortun81 DFortun81 dismissed ImUnicke’s stale review January 22, 2025 17:48

I think we've come to a good conclusion

@DFortun81 DFortun81 merged commit 2a3c50f into ATTWoWAddon:master Jan 22, 2025
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.

3 participants