Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[335] Vmaps latests changes and Wintergrasp #20028

Closed
Jildor opened this issue Jul 14, 2017 · 27 comments
Closed

[335] Vmaps latests changes and Wintergrasp #20028

Jildor opened this issue Jul 14, 2017 · 27 comments

Comments

@Jildor
Copy link
Contributor

Jildor commented Jul 14, 2017

Description:

With latests changes of Vmaps, Wintergrasp have stranges bugs
d2a30fe
a409287
cfb0f9f
a2c123b
f6c8497
c357939

I think the causant is:
f6c8497
( I can't comment in the commit because is closed f6c8497#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: 445c5a0

TDB version: TDB335.63

Operating system: Debian

@Jildor
Copy link
Contributor Author

Jildor commented Jul 15, 2017

@Treeston can you take a look here?

@Treeston
Copy link
Member

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
Copy link
Contributor Author

Jildor commented Jul 19, 2017

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

@Aokromes
Copy link
Member

wintergrasp needs all spawns moved into db to work properly.

@Kittnz
Copy link
Member

Kittnz commented Jul 19, 2017

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

@Jildor
Copy link
Contributor Author

Jildor commented Jul 19, 2017

@Aokromes I think this not related with this

I'm saying the worldobjects with this: f6c8497#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
Copy link
Contributor Author

Jildor commented Jul 19, 2017

Hellfire outdoor capture points seems doesn't work
:(

@ghost
Copy link

ghost commented Jul 19, 2017

TrinityCore rev. a59ebb3 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
Copy link
Contributor Author

Jildor commented Jul 19, 2017

I think is this commit: f6c8497

I will try to revert and test ASAP

@ghost
Copy link

ghost commented Jul 19, 2017

@Jildor

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

😃

@Jildor
Copy link
Contributor Author

Jildor commented Jul 19, 2017

Reverting this: f6c8497
(here my revert: Jildor@0bf9e05 )

Wintergrasp Capture Points and vehicle count works

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

@Jildor
Copy link
Contributor Author

Jildor commented Jul 19, 2017

And confirm, Reverting this: a2c123b
(here my revert: Jildor@a7bc4f6 )
Hellfire outdoor capture points works fine

Then the problem is:
f6c8497
and
a2c123b

@Jildor
Copy link
Contributor Author

Jildor commented Jul 22, 2017

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

@Golrag
Copy link
Contributor

Golrag commented Jul 22, 2017

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
Copy link
Contributor

Golrag commented Jul 22, 2017

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
Copy link

ghost commented Jul 22, 2017

@Golrag

With these changes the problem is solved?

@Golrag
Copy link
Contributor

Golrag commented Jul 22, 2017

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

@Jildor
Copy link
Contributor Author

Jildor commented Jul 22, 2017

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
Copy link
Member

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

@Golrag
Copy link
Contributor

Golrag commented Jul 22, 2017

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
Copy link
Member

Treeston commented Jul 22, 2017

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

@Golrag
Copy link
Contributor

Golrag commented Jul 22, 2017

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

@Treeston
Copy link
Member

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
Copy link
Contributor

Golrag commented Jul 22, 2017

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

@Treeston
Copy link
Member

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).

Treeston added a commit that referenced this issue Jul 22, 2017
…n initially creating them. Fixes some issues with PvP objectives not having the correct area ID set on initialization. Closes #20028.
@ghost ghost mentioned this issue Jul 27, 2017
@Omniralis
Copy link

Omniralis commented Nov 26, 2018

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
Copy link
Member

your report doesn't belong to vmaps.
#22700

Shauren pushed a commit that referenced this issue Aug 19, 2020
…n initially creating them. Fixes some issues with PvP objectives not having the correct area ID set on initialization. Closes #20028.

(cherry picked from commit 28b3469)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants