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/Scripts): Reset Scourgelord Tyrannus encounter state #21057

Merged
merged 7 commits into from
Dec 30, 2024

Conversation

blinkysc
Copy link
Contributor

@blinkysc blinkysc commented Dec 28, 2024

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:

  • [ X] 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 Pit of Saron
  2. go to Scourgelord Tyrannus start combat and die

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 Dec 28, 2024
@Talamorts1
Copy link

Talamorts1 commented Dec 28, 2024

Fixes #15824

Copy link
Contributor

@TheSCREWEDSoftware TheSCREWEDSoftware left a comment

Choose a reason for hiding this comment

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

Match the SQL Standards

flags_extra=flags_extra | 1073741824

Copy link
Contributor

@TheSCREWEDSoftware TheSCREWEDSoftware left a comment

Choose a reason for hiding this comment

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

Now we just need to wait for someone that has cpp knowledge to review the cpp line.

@Talamorts1
Copy link

Now we just need to wait for someone that has cpp knowledge to review the cpp line.

its one line of code and its alot better than the original hack fix that was applied.

I worked on this project way before more people did. Ive made modules that are still being used - Talamortis

@@ -0,0 +1,2 @@
-- Remove HARD_RESET flag while retaining IMMUNITY_KNOCKBACK
UPDATE `creature_template` SET `flags_extra` = `flags_extra` | 1073741824 WHERE `entry` = 36658;
Copy link
Member

Choose a reason for hiding this comment

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

Now with your comment added I think that the sql query currently is not doing what you intended to do.
Right now it's only setting the knockback flag (which was probably already set before?)

But you wanted to remove another flag instead - so please update this to actually remove that other flag.

https://www.azerothcore.org/wiki/bit-and-bytes-tutorial

https://www.azerothcore.org/flag-checker/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that, changed to newer syntax forgetting bitwise operation

@@ -0,0 +1,2 @@
-- Remove HARD_RESET flag while retaining IMMUNITY_KNOCKBACK
UPDATE `creature_template` SET `flags_extra` = `flags_extra` &~ (2147483648) WHERE `entry` = 36658;
Copy link
Member

Choose a reason for hiding this comment

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

He should despawn on evade, so this is wrong

Copy link
Contributor Author

@blinkysc blinkysc Dec 30, 2024

Choose a reason for hiding this comment

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

It's definitely a hack on the current hack that exists, but seems to at least fix (at the moment) the boss not "reseting" if players die at him. Think the logic is that because line 100 pInstance->SetData(DATA_TYRANNUS, NOT_STARTED); in reset() sets to not even started (soft reset ish?); the HARD_RESET must not happen. Perhaps you have more thoughts @Talamorts1

Choose a reason for hiding this comment

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

He should despawn on evade, so this is wrong

where is your evidence of this? this Flag was applied in a previous "Fix",

if this is the case then you need rework your instance script to spawn npc if event is not done and creature is not in instance.

@Nyeriah Nyeriah changed the title fix(DB/Scripts) Scourgelord Tyrannus fix(DB/Scripts) Reset Scourgelord Tyrannus encounter state Dec 30, 2024
@Nyeriah Nyeriah changed the title fix(DB/Scripts) Reset Scourgelord Tyrannus encounter state fix(DB/Scripts): Reset Scourgelord Tyrannus encounter state Dec 30, 2024
@github-actions github-actions bot removed the DB related to the SQL database label Dec 30, 2024
@Nyeriah Nyeriah merged commit 4e49f52 into azerothcore:master Dec 30, 2024
12 checks passed
@Nyeriah
Copy link
Member

Nyeriah commented Dec 30, 2024

I will be addressing the respawn issue in another PR.

@Talamorts1
Copy link

Testing another fix now. Ill keep you updated.

@Nyeriah
Copy link
Member

Nyeriah commented Dec 30, 2024

@Talamorts1 As I said, I am already working on this

@TheSCREWEDSoftware
Copy link
Contributor

Thanks @blinkysc for submitting this PR.

@Talamorts1 as mentioned by Nyeriah they would be addressing the issue so there's no need in over-laping work. If you think there other issues that you could resolve be free to submit a PR. But the issue mentions someone fixing it, ask them first if they are still working on it, otherwise is wasting everyone's time :)

@Talamorts1

This comment was marked as off-topic.

@Nyeriah
Copy link
Member

Nyeriah commented Dec 30, 2024

Tyrannus isn't a permanent spawn. So that would never work

@TheSCREWEDSoftware
Copy link
Contributor

I've marked your comment as off-topic @Talamorts1 if you want someone to test your fixes or you have fixes open your own PR.

@Talamorts1
Copy link

na im good, this is why i stopped helping in the first place peace out ^.^

@azerothcore azerothcore locked as resolved and limited conversation to collaborators Dec 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
file-cpp Used to trigger the matrix build Ready to be Reviewed Script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Pit of Saron]: Scourgelord Tyrannus doesn't restart when you die
5 participants