Trinitycore: [3.3.5] Skadi the Ruthless appears under map after dismounting Grauf

[TheWinchesters]

Description:
Creature: Skadi the Ruthless (https://www.wowhead.com/npc=26693)
Spell: Skadi Teleport (https://www.wowhead.com/spell=61790)

Current behaviour:
As soon as he dismounts the drake, he appears somewhere under map.
It is no like 'you will see him falling', it is more like:
Exit vehicle
plop!
Under map

Expected behaviour:
Skadi should jump to the platform

Steps to reproduce the problem:

1. .go c id 26693
2. mod hp xxxxxxxxxx
3. Start the encounter
4. Kill harpooners and pick up harpoons
5. Use them three times when Skadi is in range

After dismounting Grauf, you can see where he is, doing:

1. /tar Skadi
2. .bind

Branch(es): 3.3.5
TC rev. hash/commit: b1604ba
Operating system: Win 8.1

[Killyana]

When he dismount, he must execute a jump, I remember this was missing or not working correctly since years.

[TheWinchesters]

There is a DoCast with that spell in the code but for some reason looks like it is ignored.
Maybe that function needs to be somewhere else in the code.

[Keader]

@sirikfoll

[Killyana]

The spell has a teleport effect https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp#L266
But here we can see the boss is jumping: https://youtu.be/Y0wDGhrLRi0?t=175

[TheWinchesters]

First of all, I know that this is wrong and it s a stupid / bad / horrible change but he jumps if:
Put DoCast(me, SPELL_SKADI_TELEPORT, true); after _phase = PHASE_GROUND;
Also add DoCast(me, SPELL_SKADI_TELEPORT, true); before DoMeleeAttackIfReady(); inside void UpdateAI(uint32 diff) override (class boss_skadi).

He will jump as seen on the video, but of course that is some bad, really bad, code that someone like me does when trying stupid things.
I know that this is completely useless, no sense, bad, etc, but the only reason to comment it is that perhaps someone with knowledge can understand why he jumps with that change, why he does not do it with the current code and where the problem is.

[illfated]

Even if you say that it is bad coding, I will try to visualize what you described, just in case someone can come up with a better idea based on your attempts.

diff --git a/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp b/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp
index f3b7fb688e..292073f3dd 100644
--- a/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp
+++ b/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp
@@ -269,6 +269,7 @@ public:
                     me->SetImmuneToPC(false);
                     me->SetReactState(REACT_AGGRESSIVE);
                     _phase = PHASE_GROUND;
+                    DoCast(me, SPELL_SKADI_TELEPORT, true);

                     scheduler
                         .Schedule(Seconds(8), [this](TaskContext crush)
@@ -313,6 +314,7 @@ public:
                 if (!UpdateVictim())
                     return;

+                DoCast(me, SPELL_SKADI_TELEPORT, true);
                 DoMeleeAttackIfReady();
             }
         }

[zabustak]

set before unmount canfly true and then sheldue once a event with
DoCast(me, SPELL_SKADI_TELEPORT, true);

[illfated]

Not sure what you are saying there.
Could you visualize with a code snippet?
(use ```diff to highlight your suggestions)

[zabustak]

```
case ACTION_GAUNTLET_END:

• me->SetCanFly(true);
  me->ExitVehicle();
  Talk(SAY_DRAKE_DEATH);
  DoCast(me, SPELL_SKADI_TELEPORT, true);

                 scheduler

• .Schedule(Seconds(1), this
• {
• DoCast(me, SPELL_SKADI_TELEPORT, true);
• })
  .Schedule(Seconds(8), this
  {
  DoCastVictim(SPELL_CRUSH);

[sirikfoll]

Possible Fix:

diff --git a/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp b/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp
index f3b7fb688e..b3a41e5170 100644
--- a/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp
+++ b/src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_skadi.cpp
@@ -26,6 +26,7 @@
 #include "SpellScript.h"
 #include "TemporarySummon.h"
 #include "utgarde_pinnacle.h"
+#include "Vehicle.h"

 enum Spells
 {
@@ -261,9 +262,8 @@ public:
                     Talk(SAY_DRAKE_BREATH);
                     break;
                 case ACTION_GAUNTLET_END:
-                    me->ExitVehicle();
                     Talk(SAY_DRAKE_DEATH);
-                    DoCast(me, SPELL_SKADI_TELEPORT, true);
+                    DoCast(me, SPELL_SKADI_TELEPORT);
                     summons.DespawnEntry(NPC_WORLD_TRIGGER);
                     me->RemoveFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_NOT_SELECTABLE);
                     me->SetImmuneToPC(false);
@@ -349,7 +349,7 @@ public:
         void JustDied(Unit* /*killer*/) override
         {
             if (Creature* skadi = _instance->GetCreature(DATA_SKADI_THE_RUTHLESS))
-                skadi->AI()->DoAction(ACTION_GAUNTLET_END);
+                skadi->ExitVehicle();

             me->DespawnOrUnsummon(Seconds(6));
         }
@@ -357,7 +357,11 @@ public:
         void PassengerBoarded(Unit* who, int8 /*seatId*/, bool apply) override
         {
             if (!apply)
+            {
+                if (Creature* skadi = _instance->GetCreature(DATA_SKADI_THE_RUTHLESS))
+                    skadi->AI()->DoAction(ACTION_GAUNTLET_END);
                 return;
+            }

             Movement::MoveSplineInit init(who);
             init.DisableTransportPathTransformations();
@@ -842,6 +846,28 @@ class spell_skadi_poisoned_spear : public SpellScriptLoader
         }
 };

+// 61791 - Ride Vehicle
+class spell_skadi_ride_vehicle : public AuraScript
+{
+    PrepareAuraScript(spell_skadi_ride_vehicle);
+
+    void OnRemoveVehicle(AuraEffect const* /*aurEff*/, AuraEffectHandleModes /*mode*/)
+    {
+        PreventDefaultAction();
+        Unit* caster = GetCaster();
+        if (!caster)
+            return;
+
+        GetTarget()->GetVehicleKit()->RemovePassenger(caster);
+        caster->SetControlled(false, UNIT_STATE_ROOT);
+    }
+
+    void Register() override
+    {
+        OnEffectRemove += AuraEffectRemoveFn(spell_skadi_ride_vehicle::OnRemoveVehicle, EFFECT_0, SPELL_AURA_CONTROL_VEHICLE, AURA_EFFECT_HANDLE_REAL);
+    }
+};
+
 class spell_summon_gauntlet_mobs_periodic : public SpellScriptLoader
 {
     public:
@@ -961,6 +987,7 @@ void AddSC_boss_skadi()
     new spell_skadi_reset_check();
     new spell_skadi_launch_harpoon();
     new spell_skadi_poisoned_spear();
+    RegisterAuraScript(spell_skadi_ride_vehicle);
     new spell_summon_gauntlet_mobs_periodic();
     new achievement_girl_love_to_skadi();
     new at_skadi_gaunlet();

DELETE FROM world.`spell_script_names` WHERE `ScriptName`='spell_skadi_ride_vehicle';
INSERT INTO world.`spell_script_names` (`spell_id`,`ScriptName`) VALUES
(61791,'spell_skadi_ride_vehicle');

[illfated]

Looks promising, needs testing from those who have observed the issue.
@TheWinchesters : would you like to test sirikfoll's suggested code to see if it solves your issue?

[Killyana]

@sirikfoll I tested the patch and it works fine, you can push it

[Cobus420]

Tested after Latest commit https://github.com/TrinityCore/TrinityCore/commit/7aa91653c9e11e5c26642fe40f8f035b5d93af25
After Skadi is dead you are in combat until leaving the Instanz.

[illfated]

edit: Sorry, I was a bit quick there. You did say "After Skadi is dead". Have you tested it more than one time and found that it happens always? Did you test with the patch too?

[D0d0]

@Cobus420 this issue is also appearing without @sirikfoll patch