[9121] NPC Pick-up Spells

[frcoun]

What bug does the patch fix? What features does the patch add?

- implement dummy auras for spells which are used for picking-up npc, replaced triggered spell because they didn't cause creature to despawn

For which repository revision was the patch created?

- 9025

Who has been writing this patch? Please include either forum user names or email addresses.

- me

Here is sql part for replacing current trigger spells by whole dummy

http://pastebin.com/f2ce5ef3a

c++ part, add spell cast from dummy and creature despawn

diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -1241,21 +1241,57 @@ void Spell::EffectDummy(uint32 i)
                        bg->EventPlayerDroppedFlag((Player*)m_caster);

                    m_caster->CastSpell(m_caster, 30452, true, NULL);
                    return;
                }
-                case 51592:                                 // Pickup Primordial Hatchling
+				case 51592:                                 // Pickup Primordial Hatchling
                {
                    if (!unitTarget || unitTarget->GetTypeId() != TYPEID_UNIT)
                        return;

                    Creature* creatureTarget = (Creature*)unitTarget;

-                    creatureTarget->ForcedDespawn();
-                    return;
+                    m_caster->CastSpell(m_caster, 51593, true);
+					creatureTarget->ForcedDespawn();
+					return;
+
+                }
+				case 55364:                                 // Create Ghoul Drool Cover
+                {
+                    if (!unitTarget || unitTarget->GetTypeId() != TYPEID_UNIT)
+                        return;
+
+                    Creature* creatureTarget = (Creature*)unitTarget;
+
+                    m_caster->CastSpell(m_caster, 55363, true);
+					creatureTarget->ForcedDespawn();
+					return;

                }
+				case 50926:                                 // Gluttonous Lurkers: Create Zul'Drak Rat Cover
+                {
+                    if (!unitTarget || unitTarget->GetTypeId() != TYPEID_UNIT)
+                        return;
+
+                    Creature* creatureTarget = (Creature*)unitTarget;
+
+                    m_caster->CastSpell(m_caster, 50927, true);
+					creatureTarget->ForcedDespawn();
+					return;
+
+                }
+				case 46167:                                 // Planning for the Future: Create Snowfall Glade Pup Cover
+                {
+                    if (!unitTarget || unitTarget->GetTypeId() != TYPEID_UNIT)
+                        return;
+
+                    Creature* creatureTarget = (Creature*)unitTarget;
+
+                    m_caster->CastSpell(m_caster, 46773, true);
+					creatureTarget->ForcedDespawn();
+					return;
+                }
                case 52308:                                 // Take Sputum Sample
                {
                    switch(i)
                    {
                        case 0:

[NoFantasy]

because they didn't cause creature to despawn

So maybe your database data is wrong? I see your point, but rewrite because of this isn't sufficient reason... I tested quest 12605, related to spell 51592/51593, with this data:

spell_id=51592, cast_flags=1

spell_id=51593, cast_flags=3

..and it works just fine.

[NoFantasy]

To outline this subject a bit, after looking closer to what happens, and how we previously defined this:

Yes, i agree it is correct to use the dummies in database, and handle them in mangos (or script if require things like text, etc).

For some of the mentioned in the above, it will be like

                case 50926:                                 // Gluttonous Lurkers: Create Zul'Drak Rat Cover
               case 51026:                                 // Create Drakkari Medallion Cover
               case 51592:                                 // Pickup Primordial Hatchling
               case 51961:                                 // Captured Chicken Cover
               {
                   if (!unitTarget || unitTarget->GetTypeId() != TYPEID_UNIT || m_caster->GetTypeId() != TYPEID_PLAYER)
                       return;

                   uint32 spellId = 0;

                   switch(m_spellInfo->Id)
                   {
                       case 50926: spellId = 50927; break;
                       case 51026: spellId = 50737; break;
                       case 51592: spellId = 51593; break;
                       case 51961: spellId = 51037; break;
                   }

                   // this is wrong, it should be unitTarget->CastSpell(m_caster, ...), but this fail since we despawn unitTarget.
                   // naturally, because unitTarget is no longer valid, the spells with delay will not work
                   m_caster->CastSpell(m_caster, spellId, true);

                   Creature* creatureTarget = (Creature*)unitTarget;

                   creatureTarget->ForcedDespawn();
                   return;
               }

One of the listed is clearly wrong, according to comments at www. It is expected to create random item, not just always the same. Also, we can not despawn creature when it is not expected.

The above will also re-define what we have said previously about the npc_spellclick system in a way. Currently it was informed that for db table, it should have two spells, one for creating the item, one for dealing with the dummy. This is not the case if we go for the code above, and for those we need to delete possible existing data and also adjust cast_flags for dummies we now handle.

[frcoun]

yeah i was just about try to rewrite the patch a kind of a way you did, but you were faster

well when i tested it i tried to use

Creature* creatureTarget = (Creature*)unitTarget;

creatureTarget->CastSpell(m_caster, ...)
creatureTarget->ForcedDespawn();
return;

and it worked in all cases except 46167 and all spells were for some reason instant

[NoFantasy]

46167 must in either case not be done here, because it's supposed to trigger some effect from nearby creature (the so-called mother of the pup goes crazy)

Edit:

to have a somewhat better and more reliable despawn that despawn creature when it's supposed to, and not before, i came up with

diff --git a/src/game/Creature.cpp b/src/game/Creature.cpp
index 10f491b..b0a8d73 100644
--- a/src/game/Creature.cpp
+++ b/src/game/Creature.cpp
@@ -105,6 +105,14 @@ bool AssistDelayEvent::Execute(uint64 /*e_time*/, uint32 /*p_time*/)
    return true;
}

+bool ForcedDespawnDelayEvent::Execute(uint64 /*e_time*/, uint32 /*p_time*/)
+{
+    if (m_owner->isAlive())
+        m_owner->ForcedDespawn();
+
+    return true;
+}
+
Creature::Creature() :
Unit(), i_AI(NULL),
lootForPickPocketed(false), lootForBody(false), m_groupLootTimer(0), lootingGroupLeaderGUID(0),
@@ -1294,8 +1302,16 @@ void Creature::Respawn()
    }
}

-void Creature::ForcedDespawn()
+void Creature::ForcedDespawn(uint32 msTimeToDespawn)
{
+    if (msTimeToDespawn)
+    {
+        ForcedDespawnDelayEvent *pEvent = new ForcedDespawnDelayEvent(this);
+
+        m_Events.AddEvent(pEvent, m_Events.CalculateTime(msTimeToDespawn));
+        return;
+    }
+
    setDeathState(JUST_DIED);
    RemoveCorpse();
    SetHealth(0);                                           // just for nice GM-mode view
diff --git a/src/game/Creature.h b/src/game/Creature.h
index e9031c2..ff88864 100644
--- a/src/game/Creature.h
+++ b/src/game/Creature.h
@@ -538,7 +538,7 @@ class MANGOS_DLL_SPEC Creature : public Unit
        void RemoveCorpse();
        bool isDeadByDefault() const { return m_isDeadByDefault; };

-        void ForcedDespawn();
+        void ForcedDespawn(uint32 msTimeToDespawn = 0);

        time_t const& GetRespawnTime() const { return m_respawnTime; }
        time_t GetRespawnTimeEx() const;
@@ -651,4 +651,14 @@ class AssistDelayEvent : public BasicEvent
        Unit&             m_owner;
};

+class ForcedDespawnDelayEvent : public BasicEvent
+{
+    public:
+        ForcedDespawnDelayEvent(Creature* owner) : BasicEvent(), m_owner(owner) { }
+        bool Execute(uint64 e_time, uint32 p_time);
+
+    private:
+        Creature* m_owner;
+};
+
#endif
diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
index 4f1427f..3261327 100644
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -1154,6 +1154,35 @@ void Spell::EffectDummy(uint32 i)
                            m_caster->CastSpell(m_caster, 49378, true);
                    }
                    return;
+                case 50926:                                 // Gluttonous Lurkers: Create Zul'Drak Rat Cover
+                case 51026:                                 // Create Drakkari Medallion Cover
+                case 51592:                                 // Pickup Primordial Hatchling
+                case 51961:                                 // Captured Chicken Cover
+                {
+                    if (!unitTarget || unitTarget->GetTypeId() != TYPEID_UNIT || m_caster->GetTypeId() != TYPEID_PLAYER)
+                        return;
+
+                    uint32 spellId = 0;
+
+                    switch(m_spellInfo->Id)
+                    {
+                        case 50926: spellId = 50927; break;
+                        case 51026: spellId = 50737; break;
+                        case 51592: spellId = 51593; break;
+                        case 51961: spellId = 51037; break;
+                    }
+
+                    if (const SpellEntry *pSpell = sSpellStore.LookupEntry(spellId))
+                    {
+                        unitTarget->CastSpell(m_caster, spellId, true);
+
+                        Creature* creatureTarget = (Creature*)unitTarget;
+
+                        if (const SpellCastTimesEntry *pCastTime = sSpellCastTimesStore.LookupEntry(pSpell->CastingTimeIndex))
+                            creatureTarget->ForcedDespawn(pCastTime->CastTime);
+                    }
+                    return;
+                }
                case 50243:                                 // Teach Language
                {
                    if (m_caster->GetTypeId() != TYPEID_PLAYER)
@@ -1186,17 +1215,6 @@ void Spell::EffectDummy(uint32 i)
                    m_caster->CastSpell(m_caster, 30452, true, NULL);
                    return;
                }
-                case 51592:                                 // Pickup Primordial Hatchling
-                {
-                    if (!unitTarget || unitTarget->GetTypeId() != TYPEID_UNIT)
-                        return;
-
-                    Creature* creatureTarget = (Creature*)unitTarget;
-
-                    creatureTarget->ForcedDespawn();
-                    return;
-
-                }
                case 52308:                                 // Take Sputum Sample
                {
                    switch(i)

Using this, the creature will not despawn before after a time, here determined by spell cast time. Such a function is not only usable here, but other misc places too, maybe especially from script lib side and of course similar dummy spell effects.

It may very well be it need some extra checks and code this delay despawn event, but at least it's a start

[NoFantasy]

46167 must in either case not be done here, because it's supposed to trigger some effect from nearby creature (the so-called mother of the pup goes crazy)

I was wrong, the initial was correct like you suggested. Will commit for this spell also.