Skip to content

Quest - Imprisoned in the Citadel - Can now be completed. #22231

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

LuckyLuc96
Copy link
Contributor

@LuckyLuc96 LuckyLuc96 commented May 28, 2025

Quest is now completing appropriately for both factions.

Changes Proposed:

This PR proposes changes to:

  • Core (units, players, creatures, game systems).
  • Scripts (bosses, spell scripts, creature scripts).
  • Database (SAI, creatures, etc).

Issues Addressed:

SOURCE:

The changes have been validated through:

  • Live research (checked on live servers, e.g Classic WotLK, Retail, etc.)
  • Sniffs (remember to share them with the open source community!)
  • Video evidence, knowledge databases or other public sources (e.g forums, Wowhead, etc.)
  • The changes promoted by this pull request come partially or entirely from another project (cherry-pick). Cherry-picks must be committed using the proper --author tag in order to be accepted, thus crediting the original authors, unless otherwise unable to be found

Tests Performed:

This PR has been:

  • Tested in-game by the author.
  • Tested in-game by other community members/someone else other than the author/has been live on production servers.
  • This pull request requires further testing and may have edge cases to be tested.
    Screenshot from 2025-05-28 14-23-14

How to Test the Changes:

  • This pull request can be tested by following the reproduction steps provided in the linked issue
  • This pull request requires further testing. Provide steps to test your changes. If it requires any specific setup e.g multiple players please specify it as well.
  1. .mod rep 946 99999
  2. .additem 30622 and .additem 28395
  3. Enter heroic The Shattered Halls (.tele shatteredhalls)
  4. Accept the quest at the midway point and complete it by killing the Executioner mob in-front of the prisoners.

Known Issues and TODO List:

  • I discovered an adjacent bug that needs fixing, but I believe it's not related to my SQL. The horde captain's ingame ID is the exact same as the alliance captains. Strangely he acts like he should, and will allow you to turn in the quest. But if you kill him manually with .die and then .respawn him, he will respawn as the alliance captain. My smartAI script will ensure that the horde captain's death will result in quest failure. But I argue that this is not the real horde captain, but an imposter.

Note the NPC ID in the mouse-over information. The captain's ID is supposed to be 17296

2025-05-28.18-57-09.trimmed.mp4

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

…st to Alliance faction only (no longer sharable to horde players)
@github-actions github-actions bot added the DB related to the SQL database label May 28, 2025
@LuckyLuc96 LuckyLuc96 changed the title Quest - Imprisoned in the Citadel - Can now be completed for the Alliance faction. Quest - Imprisoned in the Citadel - Can now be completable. May 28, 2025
@heyitsbench
Copy link
Contributor

INSERT INTO `quest_template_wotlk` (`ID`, `AllowableRaces`, `RequiredNpcOrGo1`) VALUES
(9524, 1791, 0),
(9525, 690, 0);

Sourced from 3.4.0 44832.

@TheSCREWEDSoftware
Copy link
Contributor

1791

You sure it's 1791 for the alliance quest? that means eveveryone but a goblin can take it

@heyitsbench
Copy link
Contributor

You sure it's 1791 for the alliance quest?

Sniffs say it is. Either way, the RequiredNpcOrGo change is incorrect according to sniffs, so this is a hackfix.

@TheSCREWEDSoftware TheSCREWEDSoftware dismissed their stale review May 28, 2025 19:05

Was approved before I saw bench's sniff information.

@LuckyLuc96 LuckyLuc96 marked this pull request as draft May 28, 2025 19:27
@LuckyLuc96
Copy link
Contributor Author

I'm convinced I went about fixing the quest the wrong way. I will look into it further and see about changing it without editing the quest_template data.

…On Executioner mob's death, both horde and alliance quests will have their requirement met.
@LuckyLuc96 LuckyLuc96 marked this pull request as ready for review May 28, 2025 23:20
@LuckyLuc96 LuckyLuc96 changed the title Quest - Imprisoned in the Citadel - Can now be completable. Quest - Imprisoned in the Citadel - Can now be completed. May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quest - Imprisoned in the Citadel - does not complete. [Quest] Imprisoned in the Citadel
3 participants