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.

Sign up for GitHub

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

Jump to bottom

fix(DB/Core/Quests): Fix the quest "Load'er Up" #20241

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

fix(DB/Core/Quests): Fix the quest "Load'er Up" #20241

wants to merge 8 commits into from

Conversation

cgrahamseven
Copy link
Contributor

@cgrahamseven cgrahamseven commented Oct 18, 2024 •
edited
Loading

Move quest 11881 to the core for better scripted control and more blizzlike behavior.

Closes AzerothCore issue #20239

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. .quest add 11881 (horde only)
  2. Go to crashed flying machine in scalding pools area of borean tundra
  3. Use item Jenny's whistle
  4. Jenny spawns and should maintain speed value of speed_run from db in creature_template while following player, not run at the same speed as player
  5. Allow Jenny to get aggro from mob and observe aura Crates Carried losing stacks along with visual effect of crates dropping from Jenny
  6. Any damage after stacks from Crates Carried has dropped to zero should despawn Jenny
  7. Run Jenny back to Fezzix in Taunka'le village.
  8. When Jenny is within 15 meters of Fezzix, you should receive completion of quest
  9. Jenny walks back to fezzix and then despawns

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.

fix(DB/Core/Quests): Fix quest 11881
3eaaccf 
Move quest 11881 to the core for better scripted control and more
blizzlike behavior.

Closes AzerothCore issue azerothcore#20239
@github-actions github-actions bot added the DB related to the SQL database label Oct 18, 2024
@github-actions github-actions bot added CORE Related to the core Script file-cpp Used to trigger the matrix build labels Oct 18, 2024
@Grimdhex Grimdhex changed the title fix(DB/Core/Quests): Fix quest 11881 fix(DB/Core/Quests): Fix the quest "Load'er Up" Nov 15, 2024
@Grimdhex
Copy link
Contributor

This PR can't be done with SAI ?

@cgrahamseven
Copy link
Contributor Author

This PR can't be done with SAI ?

It was previously SAI but it was broken. The summoned NPC would immediately despawn on damage taken. I also added a feature to temp summons to prevent them from inheriting their owner's speed values since this NPC in particular runs alot slower than the player. Previously, the summon would act like a warlock pet or hunter pet and match speed with the player. Additionally, there are some events that are part of this quest that are easier to handle in a script as opposed to SAI. It's working pretty damn close to the original blizzard quest now.

@Exitare
Copy link
Member

Exitare commented Dec 14, 2024 •
edited
Loading

Tested PR.
Works like a charm.
Thanks for the clear test instructions.
I tested both scenarios, loosing crates up until a despawn happens, as well as a successful quest completion.
The npc is also using its own speed, and is not affected by player speeds.

Kitzunu
Kitzunu requested changes Dec 14, 2024
View reviewed changes
data/sql/updates/pending_db_world/rev_1729269741948776900.sql Outdated
WHERE `entryorguid` = 25969;
-- Update creature template for NPC Jenny (25969)
UPDATE `creature_template`
SET `speed_run` = 1, `speed_walk` = 0.8, `speed_swim` = 0.6, `AIName` = "", `ScriptName` = "npc_jenny"
Copy link
Member

Choose a reason for hiding this comment

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

speed values are sniffed. They don't change unless you have sniffed values

src/server/scripts/Northrend/zone_borean_tundra.cpp Outdated
@@ -2041,6 +2129,7 @@ void AddSC_borean_tundra()
new npc_thassarian2();
new npc_leryssa();
new npc_counselor_talbot();
new npc_jenny();
Copy link
Member

Choose a reason for hiding this comment

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

Use the new registry model.

RegisterCreatureAI()

@TheSCREWEDSoftware
Copy link
Contributor

After the changes does this still need testing? Also @cgrahamseven the changes asked.

@cgrahamseven
Copy link
Contributor Author

After the changes does this still need testing? Also @cgrahamseven the changes asked.

@TheSCREWEDSoftware I don't have the time right now to look into the change requested by @Kitzunu to use RegisterCreatureAI. The NPC is not a ScriptedAI. If you'd like the PR either revert it back to the previous registration or have someone else do whatever conversion is necessary. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kitzunu Kitzunu Kitzunu requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
CORE Related to the core DB related to the SQL database file-cpp Used to trigger the matrix build Ready to be Reviewed Script
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Quest "Load'er Up" broken (Horde)
6 participants
@cgrahamseven @Grimdhex @Exitare @TheSCREWEDSoftware @Kitzunu @Helias