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

Scripts/Ebon Hold: Bloody Breakout #24126

Merged
merged 10 commits into from Feb 28, 2020
Merged

Scripts/Ebon Hold: Bloody Breakout #24126

merged 10 commits into from Feb 28, 2020

Conversation

Sorikoff
Copy link
Contributor

@Sorikoff Sorikoff commented Feb 1, 2020

Changes proposed:

Rework existing script.

  • Move some parts to SAI.
  • Improve intro/outro.
  • Add/correct missing gossips, flags, movements, timers, IDs...

Target branch(es): 3.3.5

Issues addressed:

Tests performed: Works good in overall.

@Sorikoff
Copy link
Contributor Author

Sorikoff commented Feb 2, 2020

Could someone help with a couple of issues?

  1. The following texts doesn't play their emotes.
(28912,7,0,'The death of the High Inquisitor of New Avalon will not go unnoticed. You need to get out of here at once! Go, before more of them show up. I''ll be fine on my own.',12,0,100,1,0,0,29239,0,'Koltira Deathweaver'),
(28912,8,0,'I''ll draw their fire, you make your escape behind me.',12,0,100,1,0,0,29240,0,'Koltira Deathweaver'),
(28912,9,0,'Your High Inquisitor is nothing more than a pile of meat, Crusaders! There are none beyond the grasp of the Scourge!',14,0,100,22,0,0,29241,0,'Koltira Deathweaver');
  1. Koltira does not despawn on reaching point 10 (I use me->DespawnOrUnsummon()).

@ghost

This comment has been minimized.

@Killyana
Copy link
Member

Killyana commented Feb 2, 2020

For the despawn you can put 1188 as action in the last point, but better to find why me->DespawnOrUnsummon() is not working before.

@Sorikoff Sorikoff marked this pull request as ready for review February 9, 2020 14:33
@Sorikoff
Copy link
Contributor Author

Sorikoff commented Feb 9, 2020

Works good in overall now. Some minor issues are still present but they were there with the original script too tho (like emotes).
Also using waypoints from Cmangos now.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Killyana
Copy link
Member

Killyana commented Feb 9, 2020

Emotes are not played because they are override by me->SetByteValue

@ghost
Copy link

ghost commented Feb 9, 2020

Does that mean they could be played if that line is removed in the script while the values are set in the DB or by SAI?

@Sorikoff
Copy link
Contributor Author

Sorikoff commented Feb 9, 2020

The first emote (first text) is playing I think but bytes are being set.

@Killyana
Copy link
Member

Killyana commented Feb 9, 2020

No Idea in this case

@ghost
Copy link

ghost commented Feb 9, 2020

Maybe a timing issue, as noted in #22287 ?

@Killyana
Copy link
Member

There's a missing text: "Stay in the anti-magic field! Make them come to you!" , 29225
And Koltira must equip his weapon.
Otherwise, it look good

@Sorikoff
Copy link
Contributor Author

Added lines.

@jackpoz
Copy link
Member

jackpoz commented Feb 22, 2020

what's the status with this PR ?

@Killyana
Copy link
Member

Need I final test, and maybe check the cpp.

@Killyana
Copy link
Member

Killyana commented Feb 22, 2020

Everything looks fine except in case the player dies, I think in this case it's better to despawn Koltira and every summoned npc, and set the quest to failed.
Also change the respawn time of Koltira to 1min instead of 6

Also if the player leave the area a reset must happen.

@Sorikoff
Copy link
Contributor Author

Everything looks fine except in case the player dies, I think in this case it's better to despawn Koltira and every summoned npc, and set the quest to failed.

Isn't Reset called in such case?

@Killyana
Copy link
Member

When I died the script kept going an summoning npcs.

@Sorikoff
Copy link
Contributor Author

Added 5 sec check (until outro events) that will despawn Koltira && summons if player is dead/30m from Koltira).

@Killyana
Copy link
Member

In which case: void Reset() overridevoid Reset() override will be triggered ?

@Sorikoff
Copy link
Contributor Author

At respawn.

@Killyana
Copy link
Member

It looks good, it can be merge

{
{ 1653.36f, -6038.34f, 127.584f },
{ 1653.765f, -6035.075f, 127.5844f },
{ 1651.89f, -6037.101f, 127.5844f }
Copy link

Choose a reason for hiding this comment

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

Cosmetic/codestyle: 8 space indent is slightly redundant. Reduce to 4 ?

@ghost
Copy link

ghost commented Feb 28, 2020

Maybe this is a proper time to consider adding the commit 9009c82 to make ci/circleci: nopch work again, or choose to rebase this PR branch on the current 3.3.5 TC branch, if you prefer that instead.

@jackpoz jackpoz merged commit 2b14b72 into TrinityCore:3.3.5 Feb 28, 2020
@jackpoz
Copy link
Member

jackpoz commented Feb 28, 2020

Thanks for the PR :)

@Sorikoff Sorikoff deleted the 3.3.5-breakout branch February 29, 2020 15:23
Shauren pushed a commit that referenced this pull request Dec 22, 2021
* Scripts/Ebon Hold: Bloody Breakout

* Scripts/Ebon Hold: Bloody Breakout (part 2)

* Improvements

* Fail quest

* Spacing

* Update 9999_99_99_99_world.sql

* Update 9999_99_99_99_world.sql

* Rename 9999_99_99_99_world.sql to 2020_02_28_04_world.sql

Co-authored-by: Giacomo Pozzoni <giacomopoz@gmail.com>
(cherry picked from commit 2b14b72)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants