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/Creature): Centipaar Tunneler attack on summon #9989

Merged
merged 2 commits into from
Jan 5, 2022

Conversation

febaut
Copy link
Contributor

@febaut febaut commented Jan 3, 2022

Centipaar Tunnelers from Water Survey quest now attack the player after collecting samples.

The issue suggests that maybe is a missing InhabitType but is not, tried adding water and still the same.

Changes Proposed:

  • Adding a new SMART SCRIPT to Centipaar Tunneler. Entry: 5459

Issues Addressed:

SOURCE:

Tests Performed:

How to Test the Changes:

  1. Add the smart script row
  2. .go xyz -7218 -2913 6.5 1 1
  3. .additem 8584
  4. Use the item. The mobs should insta attack you.

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.

Centipaar Tunnelers now attack the player after collecting samples. 
I preferred "Move one Yard"  rather "Attack Start" cause it causes a double auto attack each mob meaning a lot of damage.
@Yehonal Yehonal added the DB related to the SQL database label Jan 3, 2022
@@ -0,0 +1,3 @@
INSERT INTO `version_db_world` (`sql_rev`) VALUES ('1641245804524923284');

INSERT INTO `smart_scripts` (`entryorguid`, `source_type`, `id`, `link`, `event_type`, `event_phase_mask`, `event_chance`, `event_flags`, `event_param1`, `event_param2`, `event_param3`, `event_param4`, `event_param5`, `action_type`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `action_param5`, `action_param6`, `target_type`, `target_param1`, `target_param2`, `target_param3`, `target_param4`, `target_x`, `target_y`, `target_z`, `target_o`, `comment`) VALUES (5459, 0, 2, 0, 54, 0, 100, 0, 0, 0, 0, 0, 0, 46, 1, 0, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0, 0, 'Centipaar Tunneler - On Just Summoned - Move Forward 1 Yards');
Copy link
Contributor

@acidmanifesto acidmanifesto Jan 3, 2022

Choose a reason for hiding this comment

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

INSERT INTO `smart_scripts` (`entryorguid`, `source_type`, `id`, `link`, `event_type`, `event_phase_mask`, `event_chance`, `event_flags`, `event_param1`, `event_param2`, `event_param3`, `event_param4`, `event_param5`, `action_type`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `action_param5`, `action_param6`, `target_type`, `target_param1`, `target_param2`, `target_param3`, `target_param4`, `target_x`, `target_y`, `target_z`, `target_o`, `comment`) VALUES 
(5459, 0, 2, 0, 54, 0, 100, 0, 0, 0, 0, 0, 0, 46, 1, 0, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0, 0, 'Centipaar Tunneler - On Just Summoned - Move Forward 1 Yards');

Copy link
Contributor

Choose a reason for hiding this comment

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

Add line Break for readability. One extremely long line does make things a bit problematic on the human eye and other reviewers. We prefer to keep it all on one page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot. Will take it into account next time.

Copy link
Member

@Kitzunu Kitzunu Jan 4, 2022

Choose a reason for hiding this comment

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

Not next time. This time.

Also you always need to a DELETE statement before an INSERT (Only exception being for the sql_rev thing)
You can learn more about SQL stuff here https://www.azerothcore.org/wiki/sql-standards :)

@Nyeriah
Copy link
Member

Nyeriah commented Jan 4, 2022

Centipaar Tunnelers from Water Survey quest now attack the player after collecting samples.
I preferred "Move one Yard" rather "Attack Start" cause it causes a double auto attack each mob meaning a lot of damage.

This assumption is not correct, attack start won't cause them to hit you twice, it just tells the creature to start attacking you. In this specific case, you could use the action to start attacking the summoner.

@febaut
Copy link
Contributor Author

febaut commented Jan 4, 2022

Is not an "assumption". Please go and try with attack start. You'll get 4 hits at once instead of 2.

@Kitzunu
Copy link
Member

Kitzunu commented Jan 4, 2022

This is not an acceptable fix. This is a hack.

If there now is an issue with Attack Start then we should fix that. And not hack fix through things in the database.

@febaut
Copy link
Contributor Author

febaut commented Jan 4, 2022

Updated with Start Attacking. Thanks for the recommendations. A bit rude to someone that is just trying to help, but if it's usless, okay ^^.

@Kitzunu
Copy link
Member

Kitzunu commented Jan 4, 2022

I apologize if I seemed rude. I appreciate the effort. But this is not the right way of fixing the issue

@acidmanifesto acidmanifesto added the Needs sniff The issue/PR needs sniff data to be corrected. label Jan 4, 2022
@acidmanifesto
Copy link
Contributor

Added Need Sniff label for further clarification in how it should be handled.

@Nyeriah
Copy link
Member

Nyeriah commented Jan 4, 2022

If the creature attacks the summoner this is fine. I don’t understand what needs to be sniffed about this. The only part incorrect about this PR was forcing the creature move on top of the player to trigger the attack reaction instead of doing it through the summon event.

@Nyeriah Nyeriah added Ready to be Reviewed Waiting to be Tested and removed Needs sniff The issue/PR needs sniff data to be corrected. labels Jan 4, 2022
@Annamaria-CC
Copy link
Member

@febaut

Thanks you so much for your contribution! I have tested it and it is working as described!

@acidmanifesto
Copy link
Contributor

@febaut

Thanks you so much for your contribution! I have tested it and it is working as described!

The Sql still needs to be adjust and we need a delete query as well before the insert.

@febaut
Copy link
Contributor Author

febaut commented Jan 5, 2022

@febaut
Thanks you so much for your contribution! I have tested it and it is working as described!

The Sql still needs to be adjust and we need a delete query as well before the insert.

I did the changes already.

@acidmanifesto
Copy link
Contributor

@febaut
Thanks you so much for your contribution! I have tested it and it is working as described!

The Sql still needs to be adjust and we need a delete query as well before the insert.

I did the changes already.

I do apologize. I really did not see the delete line at all when i made the comment. hitting refresh a couple of times after another staffer informed me it was there, it then showed.
Thank you for the PR.

@Nyeriah Nyeriah merged commit 1020cf1 into azerothcore:master Jan 5, 2022
@Nyeriah
Copy link
Member

Nyeriah commented Jan 5, 2022

Thanks for your PR! 🚀

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 Ready to be Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Centipaar Tunneler NPCs of Gadgetzan Water Survey quest doesn't attack player
6 participants