NoFantasy

Accepted for [9269]. Earlier accepted patch was never added to master

Should be fixed in 9268, it was just a typo in there i think.

Mh, no it's a bug. Seems I got things mixed up typeid_unit vs gameobject. Corrected shortly.

It's impossible to enforce to full extend, but there are examples (like the one mentioned) where you inform the user that "hey, i see you make many commits in your fork, you should inform where they are actually from" and get a "fuck you asshole" back in your face. It's about making an example, saying "we don't want you guys around here" and then teach (hopefully) the community what it is about. We can't be the police, but we can help some getting better at how they behave in a community like ours

Agree with both, get rid of the not-serious people that is not interested in improving what they do. Sure, honest mistakes can happen, but seeing a huge amount of commits, the owner is fully aware of what he is doing (for the example, he was already warned at scriptdev2 site), it's clear that they do not belong in a open-sourced community.

@Xeross: i agree, there is a small tendency to playing ping-pong, like "it's not mangos problem, it's script problem" and then you find a statement at "the others" site that it's not a script problem, it's mangos problem. The cause? In my view, it's very often ignorance about trying to figure out how it's really all connected. Sure, it can be a script problem, that the script is not made, but then what is the script is just like it should be, but need a spell to function properly? Blame <random> guy at Mangos, blame script for not making a hack or try to figure out how it should work and make fix for bot mangos and script? Clearly, too few choose the last option. If people would spend more time to put their head in to the code, trying to figure out how stuff is really connected, maybe the original poster would never have posted what he did.

Thanks. In [9236] with the corrections.

No objections, but hope for more active discussions/comments in channel

spell 72525? I think that must be a typo in the updated diff.

I totally agree! Added to [9230]

Quite horrible site to download from, but after 6-7 attempts, it decided to respond. I wonder, is there any reason to not handle this GO-type where we handle every other GO? Logically it should be handled in GameObject::Use like any other.

Sure, player pet will like before then. However, pet of npc will run, instead of walking when for example following his master using a waypoint. No matter how much we twist and turn it, it need a more extended correction so it will be right in all cases

Maybe crash log would help?

Blame this: http://github.com/mangos/mangos/commit/99e6b052b78b17f0c8d2439b9931a1def5867602 Both current and old way is in fact wrong, so a better solution is in the 'think box'

I think m_target can not be NULL. If it is, then it should crash...

Ok, thanks for testing dardennf. That means doing anything with movement generator is wrong. Instead, we must do proper check from isInAccessablePlaceFor() and make sure this is really used in SelectHostileTarget()

You must not mix up _destination point_ with the points in between. i_destinationHolder.SetDestination(traveller, x, y, z); is the destination, and it is in fact correct for this function. What you are talking about is pathfinding, the generated points between current location and destination location, which is a far more complex issue, not really related to this topic at all

Lol, just some test code i figured wouldn't hurt to try: src/game/TargetedMovementGenerator.cpp | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/game/TargetedMovementGenerator.cpp b/src/game/TargetedMovementGenerator.cpp index 10f2597..1a3b0c9 100644 --- a/src/game/TargetedMovementGenerator.cpp +++ b/src/game/TargetedMovementGenerator.cpp @@ -65,6 +65,12 @@ TargetedMovementGenerator<T>::_setTargetLocation(T &owner) i_target->GetClosePoint(x,y,z,owner.GetObjectSize(),i_offset,i_angle); } + if (owner.GetTypeId() == TYPEID_UNIT && !((Creature*)&owner)->canFly()) + { + const Map* pMap = owner.GetBaseMap(); + z = std::max(pMap->GetHeight(x, y, MAX_HEIGHT), pMap->GetWaterLevel(x, y)); + } + /* We MUST not check the distance difference and avoid setting the new location for smaller distances. By that we risk having far too many GetContactPoint() calls freezing the whole system. It pretty much glue them to ground and follow you from there, instead of going up in to the air. For correctness, it's probably not right, but at least can give some idea how to make them move on the ground instead of following up in the air. No idea if it have any side-effects, but i assume there are.

Can you be more specific on this? - is it instant evade or do they stand still for a little while (like 2-5 seconds) before they evade? - are there cases where creature attempt use ranged spell and can possibly knock player off the mount so he fall back on to the ground?

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

Accepted to 9114

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

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.

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.

Yes, i would agree, it is still correct to limit to 1 target. Exclusion of explicit target must be done another place.