-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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/FISHING): Fix "Disarmed!" fishing quest #21310
base: master
Are you sure you want to change the base?
Conversation
d4c4381
to
0822628
Compare
0822628
to
6b4cc5e
Compare
Please address the CI errors. Looks like there's existing conditions involved on the items you're changing. Might also want to check those conditions and see how they relate to the issue you're trying to fix. |
@p-tkachuk correct me if wrong as you phrased in a way which is hard to understand. When having the quest Disarmed! Currenty in AC if I fish in Dalaran with the quest I get the quest item and not the fish which could contain(?) the quest item? For those who actually want to be able to test.
Edit: -- Replaces `Severed Arm` (45323) with `Bloated Slippery Eel` (45328) in the Fishing Loot Entry `4567`
UPDATE `fishing_loot_template` SET `Item` = 45328 WHERE `Entry` = 4567 AND `Item` = 45323; Also what |
@TheSCREWEDSoftware sorry for my bad English :) There is a quest "Disarmed!" How it works on AC: When you fish outside the Dalaran prison you will catch Severed Arm, which is not correct. Due to the changes in patch 3.3.3 the quest was modified. The quest item before the changes should be fished outside Dalaran. That is why my PR do next changes: -- Delete Severed Arm from fish loot -- Delete Bloated Slippery Eel from fish loot (water outside of Dalaran, where Quest Item was before patch 3.3.3) -- Delete Bloated Slippery Eel from gameobject loot (fishing holes outside of Dalaran, where Quest Item was before patch 3.3.3) -- Update Bloated Slippery Eel Entry in reference loot (the quest fish already exist in loot table, but it is in "wrong" water. Right now it in the sewers. So I'm moving it to outside the prison) -- Delete Corroded Jewelry from fish loot (That is another fishing quest, and we have duplicated drop in DB. One in fishing_loot_template and another in "reference_loot_template") @sudlud yep, seems like I haven't clear everything. I'll take a look. |
af3d4f1
to
7e5e682
Compare
Fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditions for the event, quest and reference loot are correct. The only thing I had to do was change the fishing loot.
-- Replaces `Severed Arm` (45323) with `Bloated Slippery Eel` (45328) in the Fishing Loot Entry `4567` UPDATE `fishing_loot_template` SET `Item` = 45328 WHERE `Entry` = 4567 AND `Item` = 45323;Also what
Corroded Jewelry
duplicate it? there's only 1 entry in the fishing loot template.
As I said before, making all the changes you've submitted not intended changes.
I do not understand what you are saying, but I'll try to address each line of changes and comments.
If only this change will be made, then next things will happen:
Changes, which you proposed, technically will fix the quest, BUT it will keep all the garbage in DB, causing incorrect quest behavior: it would be possible to catch quest item in invalid zones.
Please, do next to see dupluicates:
I do hope it will clear things out. |
I'm not home to validate or check related to the zones. |
I only had a quick time to glance over the code and check the database the 2 places were they get removed make sensed, haven't tested. And i need to look into more for the corrupted one |
Changes Proposed:
This PR proposes changes to:
Issues Addressed:
Quest: https://www.wowhead.com/wotlk/quest=13836/disarmed#comments
The fishing loot should be https://www.wowhead.com/wotlk/item=45328/bloated-slippery-eel (which is already present in table)
This PR removes https://www.wowhead.com/wotlk/item=45323/severed-arm from fishing loot, which should be inside "bloated-slippery-eel"
ALSO: it removes duplicated loot for "Corroded Jewelry" (https://www.wowhead.com/wotlk/item=45903)
SOURCE:
The changes have been validated through:
Tests Performed:
This PR has been:
How to Test the Changes:
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.