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

fix(DB/Core/Quest): Add scripts & spawn for quest "Imprisoned in the citadel" #21114

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

Conversation

Exitare
Copy link
Member

@Exitare Exitare commented Jan 8, 2025

This is an as close as possible blizzlike solution. I skimmed through youtube videos of the quest, and comments.
I dont have any sniffs regarding the exact placement of the npcs, but from what i gathered they are supposed to be at the same place.
I used a script because I am not aware of a smartAI action that triggers the CanBeSeen() check for a specific team Id.

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.

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. .go xyz 122.41 253.17 -14.33 540
  2. Horde character should only see the horde quest npc (make sure to be either level 70 or activate low level quests next to your minimap) / Alliance characters should only see the alliance quest npc.

Known Issues and TODO List:

  • [ ]
  • [ ]

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.

@github-actions github-actions bot added DB related to the SQL database Script file-cpp Used to trigger the matrix build labels Jan 8, 2025
@heyitsbench
Copy link
Contributor

I have a feeling these creatures likely are spawned based on the first player to enter the instance, i.e. not both spawned at once and just hidden to the opposing faction, same vein as creatures in the various ICC instances.

@Exitare
Copy link
Member Author

Exitare commented Jan 8, 2025

I dont know how exactly it should work. The question is also how "blizzlike" AC wants to be, as in blizzlike for the player or also blizzlike for the underlying systems (without hacks obviously)

@heyitsbench
Copy link
Contributor

AC's goal is Blizzlike at all points. I would have proposed going into the instance on retail with an alliance and horde character in a party together to see what happens, but the modifications to the instance in Cata may muddy the water there, and I know ICC instances can't be entered with cross-faction groups on retail.

@Exitare
Copy link
Member Author

Exitare commented Jan 8, 2025

Ok, thanks for the clarification. i dont have the ability to access more information at this point, so this is my take on the problem, but if more information emerges I can adjust it.

@Exitare Exitare added the Needs sniff The issue/PR needs sniff data to be corrected. label Jan 8, 2025
@fangshun2004
Copy link
Contributor

fangshun2004 commented Jan 8, 2025

I sniffed The Shattered Halls on 3.4.3.57364

DELETE FROM `creature` WHERE (`id1` = 17288);
INSERT INTO `creature` (`guid`, `id1`, `id2`, `id3`, `map`, `zoneId`, `areaId`, `spawnMask`, `phaseMask`, `equipment_id`, `position_x`, `position_y`, `position_z`, `orientation`, `spawntimesecs`, `wander_distance`, `currentwaypoint`, `curhealth`, `curmana`, `MovementType`, `npcflag`, `unit_flags`, `dynamicflags`, `ScriptName`, `VerifiedBuild`, `CreateObject`, `Comment`) VALUES
(151301, 17288, 0, 0, 540, 3714, 3714, 3, 1, 0, 121.2784423828125, 252.496307373046875, -14.5990028381347656, 1.274090290069580078, 7200, 0, 0, 6104, 0, 0, 0, 0, 0, '', 57364, 0, NULL);

DELETE FROM `quest_poi_points` WHERE (`QuestID`=9524 AND `Idx1`=1 AND `Idx2`=0) OR (`QuestID`=9524 AND `Idx1`=0 AND `Idx2`=0);
INSERT INTO `quest_poi_points` (`QuestID`, `Idx1`, `Idx2`, `X`, `Y`, `VerifiedBuild`) VALUES
(9524, 1, 0, -365, 3129, 57364), -- 9524
(9524, 0, 0, -344, 3138, 57364); -- 9524

DELETE FROM `creature_queststarter` WHERE (`id`=17288 AND `quest`=9524);
INSERT INTO `creature_queststarter` (`id`, `quest`, `VerifiedBuild`) VALUES
(17288, 9524, 57364); -- 9524 offered by 17288


DELETE FROM `creature_questender` WHERE (`id`=17290 AND `quest`=9524);
INSERT INTO `creature_questender` (`id`, `quest`, `VerifiedBuild`) VALUES
(17290, 9524, 57364); -- 9524 ended by 17290

@Exitare
Copy link
Member Author

Exitare commented Jan 8, 2025

Thank you for sharing!

@heyitsbench what should I do now?
Seems like npc location parsing is not available, the sniffs however correspond exactly to the current db structure.
Should I change the scripts to adhere to your suggestion ?
It would fix the quest being inaccessible.

@Nyeriah
Copy link
Member

Nyeriah commented Jan 8, 2025

It should be handled like icc scripts and later, update entry on player enter. Doubled npcs is something we probably would have caught in sniffs before

@Exitare
Copy link
Member Author

Exitare commented Jan 8, 2025

Guess thats better

@Exitare Exitare requested review from heyitsbench and Nyeriah January 8, 2025 20:14
@Exitare Exitare added Ready to be Reviewed Waiting to be Tested and removed Needs sniff The issue/PR needs sniff data to be corrected. labels Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB related to the SQL database file-cpp Used to trigger the matrix build Ready to be Reviewed Script Waiting to be Tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Quest] Imprisoned in the citadel [Quest] Imprisoned in the citadel
4 participants