Trinitycore: [335] Vmaps latests changes and Wintergrasp

[Jildor]

Description:

With latests changes of Vmaps, Wintergrasp have stranges bugs
d2a30fef1371cf904e56f96042d686c9da439175
a409287feffae22b9690cfc58d6e3e877fe39e28
cfb0f9fb19c03677b127f09407a6bd00bfefbe55
a2c123b5296b2723ae2a0211e7aaafe37fb9246d
f6c849729b27b77228704b595de3adaf24da2c10
c35793941b3aae9b54dce92389aa528d40050754

I think the causant is:
f6c849729b27b77228704b595de3adaf24da2c10
( I can't comment in the commit because is closed https://github.com/TrinityCore/TrinityCore/commit/f6c849729b27b77228704b595de3adaf24da2c10#commitcomment-22449905 )

I revert the commits and extract Vmaps and works fine

Wintergrasp vehicles count don't work
Wintergrasp Capture Points in Workshop don't work
...

Branch(es): 3.3.5

TC rev. hash/commit: 445c5a00e8734f410ce1abc222ed847ead6741ed

TDB version: TDB335.63

Operating system: Debian

[Jildor]

@Treeston can you take a look here?

[Treeston]

No idea - I have no clue how WG code works, and other places that use the same core logic work fine (EotS capture points).

[Jildor]

I'm not sure, but in Wintergrasp the phasemask changes, then can be possible worldobjects are in different phase with this change?

[Aokromes]

wintergrasp needs all spawns moved into db to work properly.

[Kittnz]

Dynamic spawns need to be implemented first, then move everything to db..

[Jildor]

@Aokromes I think this not related with this

I'm saying the worldobjects with this: https://github.com/TrinityCore/TrinityCore/commit/f6c849729b27b77228704b595de3adaf24da2c10#diff-29afb15ead2a8204cac07d94f074f260R2561
now gets phasemask (I'm not sure if before too or not xD)

Then, in Wintergrasp, the workshops zones are differents phases and change with defender team I think

And in EotS or Hellfire outdoor capture point are in phasemask 1

But....I don't know if is true this or not xD

[Jildor]

Hellfire outdoor capture points seems doesn't work
:(

[ghost]

TrinityCore rev. a59ebb35b60b 2017-07-18 22:05:30 +0200 (3.3.5 branch) (Win64, Release, Static)
Using World DB: TDB 335.63 + updates up to and including 2017_07_18_04_world_335.sql

Confirmed, Hellfire Fortifications can't be captured, the PvP progress bar does not show up at all.
(The PvP quests can't be completed either, because the PvP objectives can't be done.)

When did this change? I seem to remember that these objectives worked half a year ago.

[Jildor]

I think is this commit: https://github.com/TrinityCore/TrinityCore/commit/f6c849729b27b77228704b595de3adaf24da2c10

I will try to revert and test ASAP

[ghost]

@Jildor

But I remember how many months ago I did not have a problem! It is strange

:smiley:

[Jildor]

Reverting this: https://github.com/TrinityCore/TrinityCore/commit/f6c849729b27b77228704b595de3adaf24da2c10
(here my revert: https://github.com/Jildor/TrinityCore/commit/0bf9e0554586a305e0de8fbbcecac1541fcd8590 )

Wintergrasp Capture Points and vehicle count works

Now I will try to revert: a2c123b5296b2723ae2a0211e7aaafe37fb9246d
(I think is the causant of Capture Points in outdoor helllfire)

[Jildor]

And confirm, Reverting this: a2c123b5296b2723ae2a0211e7aaafe37fb9246d
(here my revert: https://github.com/Jildor/TrinityCore/commit/a7bc4f6b670488296da14205e4237926fc33d130 )
Hellfire outdoor capture points works fine

Then the problem is:
https://github.com/TrinityCore/TrinityCore/commit/f6c849729b27b77228704b595de3adaf24da2c10
and
https://github.com/TrinityCore/TrinityCore/commit/a2c123b5296b2723ae2a0211e7aaafe37fb9246d

[Jildor]

@Treeston @Golrag
Any idea what the cause might be?

[Golrag]

For vehicle count, seems like ZoneScript is null so BattlefieldWG::OnCreatureCreate never gets called.
Edit: in WorldObject::SetZoneScript GetZoneId() returns 0 when creature is created.

[Golrag]

try those two changes

diff --git a/src/server/game/Entities/Creature/Creature.cpp b/src/server/game/Entities/Creature/Creature.cpp
index b11a5a8018..8dbe5be504 100644
--- a/src/server/game/Entities/Creature/Creature.cpp
+++ b/src/server/game/Entities/Creature/Creature.cpp
@@ -963,6 +963,7 @@ bool Creature::Create(ObjectGuid::LowType guidlow, Map* map, uint32 phaseMask, u
     //! Relocate before CreateFromProto, to initialize coords and allow
     //! returning correct zone id for selecting OutdoorPvP/Battlefield script
     Relocate(x, y, z, ang);
+    UpdatePositionData();

     // Check if the position is valid before calling CreateFromProto(), otherwise we might add Auras to Creatures at
     // invalid position, triggering a crash about Auras not removed in the destructor

diff --git a/src/server/game/Entities/GameObject/GameObject.cpp b/src/server/game/Entities/GameObject/GameObject.cpp
index be0406be5b..6ae82ea3ce 100644
--- a/src/server/game/Entities/GameObject/GameObject.cpp
+++ b/src/server/game/Entities/GameObject/GameObject.cpp
@@ -254,6 +254,7 @@ bool GameObject::Create(ObjectGuid::LowType guidlow, uint32 name_id, Map* map, u
     }

     SetPhaseMask(phaseMask, false);
+    UpdatePositionData();

     SetZoneScript();
     if (m_zoneScript)

[ghost]

@Golrag

With these changes the problem is solved?

[Golrag]

Yea, still some problems but not related to this issue like wrong faction shown for workshops.

[Jildor]

Nice @Golrag 👍
Wintergrasp Capture Points and Vehicle Count works
Hellfire outdoor Capture Points works

Please, make a pull request and make a .die to @Treeston 😋

[Treeston]

Ah, so area data wasn't accurate on the first tick? Good catch, @Golrag.

[Golrag]

Won't have time to create a PR today I think, maybe tomorrow :p
@Treeston shouldn't this function always be called when relocate has been called ? Maybe there are more issues don't know for sure :p

[Treeston]

It is, in map relocation handler. (Map::XRelocation)

[Golrag]

Yeah but it isn't when we call Position::Relocate I wonder if we should call it then

[Treeston]

You should never be calling Position::Relocate on a unit directly. It is a horrible horrible thing that will break many things, not just this.

PS: Non-C++ devs blindly spamming thumbs up on shit they don't understand is truly helpful, keep it up.

[Golrag]

So a GetMap()->CreatureRelocation would be a better fix than what I wrote ? I'm on mobile & won't PR today :s

[Treeston]

No, your fix is perfectly fine - it notifies the creature that its zone data changed on create (that's what UpdatePositionData does). Right now, zone/area data is only valid after the first Map::XRelocation call, which doesn't happen until later - that's an oversight, and your fix is correct.

However, all later changes to create position will (should) eventually go through a Map::XRelocation method. The only one that doesn't is calling the Position methods on Unit (e.g. unit->Relocate(pos)) directly, which is why that's a Very Bad Thing™ and should never happen (it also skips stuff like line of sight notifiers, visibility updates, completely desyncs server and client states and many many more, btw).

[Omniralis]

I have a reply to this with some hopefully helpful information, but I am here looking for a fix, not presenting one. The zone captures in Wintergrasp are working, but the factions are backwards. Meaning, alliance captures the horde way, and horde captures the alliance way. I'm not sure how to fix this. Also, there are a couple walls missing in WG and if you spawn ones in their place, they do not show up on the map. I've been testing for hours, if there are specific questions I'll be sure to answer them. This is on a fresh compile with the latest TC updates.

[Aokromes]

your report doesn't belong to vmaps.
https://github.com/TrinityCore/TrinityCore/pull/22700