Skip to content

Commit

Permalink
[10847] Unsummon guardians at second item use for items without coold…
Browse files Browse the repository at this point in the history 
…own.
  • Loading branch information
VladimirMangos committed Dec 9, 2010
1 parent dd313f3 commit bd99ddb
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 12 deletions.
25 changes: 14 additions & 11 deletions src/game/SpellEffects.cpp
View file
Expand Up @@ -4617,23 +4617,26 @@ void Spell::DoSummonGuardian(SpellEffectIndex eff_idx, uint32 forceFaction)

PetType petType = propEntry->Title == UNITNAME_SUMMON_TITLE_COMPANION ? PROTECTOR_PET : GUARDIAN_PET;

// protectors allowed only in single amount
if (petType == PROTECTOR_PET)
// second cast unsummon guardian(s) (guardians without like functionality have cooldown > spawn time)
if (m_caster->GetTypeId() == TYPEID_PLAYER)
{
Pet* old_protector = m_caster->GetProtectorPet();

// for same pet just despawn
if (old_protector && old_protector->GetEntry() == pet_entry)
bool found = false;
// including protector
while (Pet* old_summon = m_caster->FindGuardianWithEntry(pet_entry))
{
old_protector->Unsummon(PET_SAVE_AS_DELETED, m_caster);
return;
old_summon->Unsummon(PET_SAVE_AS_DELETED, m_caster);
found = true;
}

// despawn old pet before summon new
if (old_protector)
old_protector->Unsummon(PET_SAVE_AS_DELETED, m_caster);
if (found)
return;
}

// protectors allowed only in single amount
if (petType = PROTECTOR_PET)

This comment has been minimized.

Sign in to view

Copy link
@Feanordev

Feanordev Dec 10, 2010

Contributor

Is this meant to be ?
Assigning petType instead of checking it ?

This comment has been minimized.

Sign in to view

Copy link
@VladimirMangos

VladimirMangos Dec 10, 2010

Fixed in [10852]. Thank you :)
if (Pet* old_protector = m_caster->GetProtectorPet())
old_protector->Unsummon(PET_SAVE_AS_DELETED, m_caster);

// in another case summon new
uint32 level = m_caster->getLevel();

Expand Down
2 changes: 1 addition & 1 deletion src/shared/revision_nr.h
View file
@@ -1,4 +1,4 @@
#ifndef __REVISION_NR_H__
#define __REVISION_NR_H__
#define REVISION_NR "10846"
#define REVISION_NR "10847"
#endif // __REVISION_NR_H__

7 comments on commit bd99ddb

@Feanordev
Copy link
Contributor

Choose a reason for hiding this comment

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

Also another thing.
I think You should add m_CastItem to:
if (petType == PROTECTOR_PET)
and
if (m_caster->GetTypeId() == TYPEID_PLAYER)
checks.
Example why its needed is for example Army of the Dead and Mirror Image - without item-casting check they wont summon more than 1 and should.
Only items can have less cooldown than duration therefore making m_CastItem is enough imo

@VladimirMangos
Copy link

Choose a reason for hiding this comment

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

protector pet explicitly selected by used summon type. This summon type+summon_slot only used for protector pets.

About item cast: do you have any real data about problematic cases. Because now often summon in generic done by learned spells
and i not think that nice limit this way without reason. Ofc, i not have any examples using in like way (learned spell) summon of guardians.

@Feanordev
Copy link
Contributor

Choose a reason for hiding this comment

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

Arent Mirror Image (learn spell) and Army of the Dead summons guardian-type ? both are stopped (spawn only 1/3(Mirror Image) or 1/8(Army of the Dead) by the code added in this commit and adding m_CastItem enables mutlisummon for these. For me its that only item-summoned creatures should be spawned only once (and despawned on second use) - at least from what I saw on most item-summoned guardians etc.

@VladimirMangos
Copy link

Choose a reason for hiding this comment

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

" 1/3(Mirror Image) or 1/8(Army of the Dead)" look like wrong implemention of spells
In normal cases single DoSummon call summon all guardians in pack.

@Feanordev
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really, take a look on Armoy of the Dead:
http://www.wowhead.com/spell=42650/army-of-the-dead
it summons 1 ghoul every 500 ms not in pack (as its channeled summon)
therefore DoSummon is called 8 times and this limits count of them summoned

@VladimirMangos
Copy link

Choose a reason for hiding this comment

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

Ok, I see. But summon spell have duration... and it triggered spell.
I still not like idea use cast item check: we known that for different type summons exist not-item cast spells that have same functionality.

Instead check must work if parent spell casting code propertly set triggeredBySpell arg in Unit::Cast* call

if (!m_triggeredBySpellInfo && !m_triggeredByAuraSpell && m_caster->GetTypeId() == TYPEID_PLAYER)

@VladimirMangos
Copy link

Choose a reason for hiding this comment

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

Final version fix in [10866]

Please sign in to comment.