Pathfind - npc can go through tree objects

damnedsoul commented 11 years ago

WoWScrnShot_040713_165323 Hi, I found problem with latest sources of C-mangos-tbc: npc can go through tree objects. I think the problem is with new dynamic Vmaps and new data from them.

Ve1vet commented 11 years ago

more of that... You can go in BT on Reliquary of Souls & it won't hit you( on switching parses it will hover because of los ), because of LOS, try to find there LOS. With Kalekgos same thing, etc. This came from patch with liquid maps & re-extracting everything.

xfurry commented 11 years ago

@vovk Can you give more details about the Reliquary of Souls issue? I remember that last year, when I cleaned that encounter there were no issues left with this one.

Ve1vet commented 11 years ago

If Reliquary of Souls in los( TBC ), it won't hit you in mele, but still able to cast spell. More correctly, if her main target in los( at this time los everywhere ). If you kill one of them( Essence of Suffering, Essence of Desire, Essence of Anger ) in that state, it just dies & in dead state become kill everyone. By the way Fixate begun work in that state, when Essence of Suffering in dead state. Before Liquid VMaps was added Reliquary of the Lost worked correctly.

xfurry commented 11 years ago

@vovk I just tested this last night and it worked flawless. Also, I can't understand how can someone not be in LoS for this encounter. What's the relationship between the liquid vmaps and this boss, since there is no liquid around that area.

Schmoozerd commented 11 years ago

it appears there is a bug with dynvmaps (maybe with the change of liquiq-extraction) in TBC

xfurry commented 11 years ago

Ok, but I still don't see the relationship between liquids and Black Temple. I think we may be talking about 2 distinct issues here. I will build a 243 server tonight and test as described above.

Ve1vet commented 11 years ago

Thats the point. Visually there is no los. But now in tbc in Reliquary of Souls room los everywhere.

xfurry commented 11 years ago

Ok

So, the first issue, from Elwyn Forest, I can confirm. It seems that the trees don't have any LoS. I also noticed this in some other areas, like Hellfire Peninsula For the second issue, I tested the Reliquary, and indeed it seems like it has some glitches, probably related to the first issue.

There is something in commits [s1779] - [s1795]. Poke @stfx about this.

PS. The issue can be reproduced on Classic as well.

DomGries commented 11 years ago

No idea but I would guess due to slightly different format/type of pre-wotlk models

xfurry commented 11 years ago

Ok, then poke @cyberium or @SilverIce about this. They have the knowledge required to sort this out. I will also try to dig through the DynVmaps commits, so I can see what went wrong. This may have happened during the implementation of the GO models.

Can anyone confirm that before [s1779] this worked fine?

xfurry commented 11 years ago

Guys, we really need your support in order to solve this issue, since I'm not very experienced on map and models stuff.

I checked all the dynvmaps commits on TBC and I couldn't find any visible mistake. Also I didn't have any conflict when I backported this. So I suppose that the dynvmaps commits actually cleaned up some code which was valid for TBC and classic, but was no longer useful on LK. And this is the point where I need help, because I have no idea which are the vmap model differences between 3.x version and 2.x version of the client. Any ideas would be appreciated.

Thanks,

SilverIce commented 11 years ago

fyi: static vmaps - all 3D models that exist on client side (not terrain that is just 2D array of heights). handles collision checks, area ID, liquid, indoor/outdoor queries. dynamic vmaps - 3d gameobject models ONLY. handles collision checks (just LoS and height queries and nothing more).

Do you still think that this is vmaps/dyn vmaps issue and not something mmap related? True, mmap generator use vmaps so it's possible that model conversion was broken. Maybe during TBC backport someone backported WoTLK specific model extraction code?

vermie commented 11 years ago

From the screen shot, it looks like the model is missing entirely from the mmap. Someone should review TerrainBuilder::loadVMap andthe vmap extensions to make sure they work correctly after the dynvmap changes.

xfurry commented 11 years ago

From my tests I strongly think it's a vmap issue, since I could cast any spell through trees. I tried this on the exact same trees that are shown in the image (Northshire, human start area). On the 335 server this isn't possible; as you can see the trees are pretty large.

damnedsoul commented 11 years ago

There is bug with vmaps objects on maps (trees etc), object aren't used in mmap generator properly, (I recommend to use recastdemo for better view of this bug, you will see empty mmap without trees)

PatSmuk commented 11 years ago

It's funny you bring up these trees, because there's an image on Google that shows the exact same area but with the trees working properly:

RedcastDemo_Northshire_Abbey1

Belarrius commented 11 years ago

some problem here

xfurry commented 11 years ago

@Patman64 how is that image generated? Maybe I can try to understand why isn't this working right after the dynvmaps commits.

Schmoozerd commented 11 years ago

this image is from recastnavigation tools (dep/recastnavigation/RecastDemo i think) -- but probably from wotlk times

Belarrius commented 11 years ago

Hey Patman64, Here is exactly the same area with MangosZero.

See, it lacks a LOT of models. (mdx or m2?), it does not have to be trees, for example ......

On MangosZero 1842 this is correct. Full VMAP. On Modern MangosZero it lacks.

Problably the f*\ VMAPz04 or Extractor?

nortshiremangoszero

xfurry commented 11 years ago

@Belarrius the issue is related to the commits I linked above. Those commits implemented dynamic vmaps, but it seems that also broke the extractor or assembler for the old clients. Maybe someone can help a little with the vmaps differences between TBC and LK clients and point us on the right path.

Schmoozerd commented 11 years ago

Please somebody test

diff --git a/contrib/vmap_extractor/vmapextract/gameobject_extract.cpp b/contrib/vmap_extractor/vmapextract/gameobject_extract.cpp
index 9513d2a..e132272 100644
--- a/contrib/vmap_extractor/vmapextract/gameobject_extract.cpp
+++ b/contrib/vmap_extractor/vmapextract/gameobject_extract.cpp
@@ -11,6 +11,10 @@ bool ExtractSingleModel(std::string& fname, StringSet& failedPaths)
     char* name = GetPlainName((char*)fname.c_str());
     char* ext = GetExtension(name);

+    std::string output(szWorkDirWmo);                       // Stores original name
+    output += "/";
+    output += name;
+
     // < 3.1.0 ADT MMDX section store filename.mdx filenames for corresponded .m2 file
     if (!strcmp(ext, ".mdx"))
     {
@@ -21,14 +25,12 @@ bool ExtractSingleModel(std::string& fname, StringSet& failedPaths)
     // >= 3.1.0 ADT MMDX section store filename.m2 filenames for corresponded .m2 file
     // nothing do

-    std::string output(szWorkDirWmo);
-    output += "/";
-    output += name;
+    // NOTE name and ext may be invalid now

     if (FileExists(output.c_str()))
         return true;

-    Model mdl(fname);
+    Model mdl(fname);                                       // Possible changed fname
     if (!mdl.open(failedPaths))
         return false;

Huge thanks to vladimir for directly catching the problem, i only hope i wasn't to stupid to implement a solution based on his finding!

Remember: you will need to reextract the vmaps for this, also i suggest to directly reextract at least the one mmap tile tfor compare :)

Belarrius commented 11 years ago

Schmoozerd: after this test >>>

north01

HAHA :D

Schmoozerd commented 11 years ago

Is this proper?

Dramacydal commented 11 years ago

that seems ok

xfurry commented 11 years ago

No. that look even worse. The result should be the same or very close to the image linked by @Patman64

RedcastDemo_Northshire_Abbey1

Schmoozerd commented 11 years ago

Well, do test ingame - i also was confused about the screen, but this might be some view-point thingy rather than actual ingame change.

vermie commented 11 years ago

Looks like two problems:

VMAP contains triangles that aren't collision-enabled, like the tree tops Not sure if this was caused by VMAP changes, or if the older clients' models had different collision data.
Some M2 triangle vertex indices are reversed in old client version TerrainBuilder::copyIndices needs better detect when to flip (simple change is to just write the indices in both orders, but it might make the build process slower and might cause artifacts in the output navmesh.

Schmoozerd commented 11 years ago

according to @Dramacydal there is an vmap option

maybe that guy did a 'large' vmap extraction, that is an option of vmap extractor? (And it shows trees then)

This would certainly explain this image :)

Also the patch i proposed had some glitch at least with wotlk-client (which i think i ironed out in the recently pushed commit https://github.com/cmangos/mangos-wotlk/commit/c444465f55a7be69883c902cd40a83768aea317c )

For the flipping i think i must focus on https://github.com/cmangos/issues/issues/109 I think i know what this patch tries to change now - but i want to at least be sure that it is required only for tbc/classic and not for wotlk/cata as well.

Schmoozerd commented 11 years ago

This one must be fixed with https://github.com/cmangos/mangos-tbc/commit/a67553cc2bb7208a096257b4f1385df35

Special thanks to @VladimirMangos for finding this error - i forgot to mention him within the commit message, and now forgot this again with the backport-commit msg.

However pending the BWL-fix from patman, the "flip" fix of #109 I think we will get some more vmap/mmap update soon, so we should await until we suggest reextracting the data :)

Edit: Patman's change https://github.com/cmangos/mangos-classic/pull/33

Schmoozerd commented 11 years ago

Dammn, this appears to be complicated :(

The patch listed here "works" in terms that trees are spawned with the vmaps. However they are spawned in a "whole"

Also with the patch here, lots of error messages are triggered. However the change https://github.com/cmangos/mangos-tbc/commit/a67553cc2bb7208a096257b4f1385df35 will have no error messages, but also no trees.

So i conclude there are two errors at hand, and so i need to search for the second problem :(

Schmoozerd commented 11 years ago

These issues should be fixed with https://github.com/cmangos/mangos-tbc/pull/20

Please test and give some feedback :)

PatSmuk commented 11 years ago

Results now are definitely better but unfortunately there's still a lot of non-collision data being included in the mesh. New pictures: Northshire 1 Northshire 2 If I had any idea how to tell which geometry is collision enabled and fix it I would but I really can't make heads or tails of what little documentation there is.

Schmoozerd commented 11 years ago

You don't see the tree for lots of forest :)

This is now perfectly fine - just in contrast to wotlk the trees are not cropped, but fully added into the mmaps - hence the weird looking, but also the stems of the trees are recognized for collision :)

technoir42 commented 11 years ago

I'm wondering how this will affect memory usage and performance of the collision detection.

cala commented 11 years ago

Looks pretty nice. I'm eager to test it. Thank you very much! :+1:

Ve1vet commented 10 years ago

After some research i can tell, that VMaps still very bugged(tested on tbc). First. Visibility.Distance.Instances = 200 and will unnable to enter Karazhan( bebore last patch i could enter it ), because pathfind get cycled. I had 250... If set default 120 all ok. Second. Command goname( it works bad before ) now when you teleporting to instance send you to the outer space & to graveyard... Third. I have patch "collision detection wrappers" fot charge, fear, confuse, leap forward & teleport behind effects by vermie( if i not mistake ) whitch using vmaps to prevent falling underground. Before last patch for vmaps all seems to be ok. But now when you fear someone his client shows right position for him, but all others can't see where he stand & unable to attack.

249CAAFE40 commented 10 years ago

I fixed the issue with non-collision data being included in the vmaps. Here's what it looks like now (on classic, tbc probably is the same), compared to the screenshots above:

vmaps

The fix comes from using boundingVertices in model.cpp instead of vertices, that way we exclude all of the actual model geometry and only use the collision geometry. Here's the diffs:

diff --git a/contrib/vmap_extractor/vmapextract/model.cpp b/contrib/vmap_extractor/vmapextract/model.cpp
--- a/contrib/vmap_extractor/vmapextract/model.cpp
+++ b/contrib/vmap_extractor/vmapextract/model.cpp
@@ -46,25 +46,20 @@ bool Model::open(StringSet& failedPaths)
     memcpy(&header, f.getBuffer(), sizeof(ModelHeader));
     if (header.nBoundingTriangles > 0)
     {
-        origVertices = (ModelVertex*)(f.getBuffer() + header.ofsVertices);
-        vertices = new Vec3D[header.nVertices];
+        boundingVertices = (ModelBoundingVertex*)(f.getBuffer() + header.ofsBoundingVertices);
+        vertices = new Vec3D[header.nBoundingVertices];

-        for (size_t i = 0; i < header.nVertices; i++)
+        for (size_t i = 0; i < header.nBoundingVertices; i++)
         {
-            vertices[i] = fixCoordSystem(origVertices[i].pos);;
+            vertices[i] = fixCoordSystem(boundingVertices[i].pos);
         }

-        ModelView* view = (ModelView*)(f.getBuffer() + header.ofsViews);
+        uint16* triangles = (uint16*)(f.getBuffer() + header.ofsBoundingTriangles);

-        uint16* indexLookup = (uint16*)(f.getBuffer() + view->ofsIndex);
-        uint16* triangles = (uint16*)(f.getBuffer() + view->ofsTris);
-
-        nIndices = view->nTris;
+        nIndices = header.nBoundingTriangles; // refers to the number of int16's, not the number of triangles
         indices = new uint16[nIndices];
-        for (size_t i = 0; i < nIndices; i++)
-        {
-            indices[i] = indexLookup[triangles[i]];
-        }
+        memcpy(indices, triangles, nIndices * 2);
+
         f.close();
     }
     else
@@ -87,7 +82,8 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     }
     fwrite(szRawVMAPMagic, 8, 1, output);
     uint32 nVertices = 0;
-    nVertices = header.nVertices;
+    nVertices = header.nBoundingVertices;
+
     fwrite(&nVertices, sizeof(int), 1, output);
     uint32 nofgroups = 1;
     fwrite(&nofgroups, sizeof(uint32), 1, output);

diff --git a/contrib/vmap_extractor/vmapextract/model.h b/contrib/vmap_extractor/vmapextract/model.h
--- a/contrib/vmap_extractor/vmapextract/model.h
+++ b/contrib/vmap_extractor/vmapextract/model.h
@@ -35,7 +35,7 @@ class Model
 {
     public:
         ModelHeader header;
-        ModelVertex* origVertices;
+        ModelBoundingVertex* boundingVertices;
         Vec3D* vertices;
         uint16* indices;
         size_t nIndices;

diff --git a/contrib/vmap_extractor/vmapextract/modelheaders.h b/contrib/vmap_extractor/vmapextract/modelheaders.h
--- a/contrib/vmap_extractor/vmapextract/modelheaders.h
+++ b/contrib/vmap_extractor/vmapextract/modelheaders.h
@@ -101,24 +101,9 @@ struct ModelHeader

 };

-struct ModelVertex
+struct ModelBoundingVertex
 {
     Vec3D pos;
-    uint8 weights[4];
-    uint8 bones[4];
-    Vec3D normal;
-    Vec2D texcoords;
-    int unk1, unk2; // always 0,0 so this is probably unused
-};
-
-struct ModelView
-{
-    uint32 nIndex, ofsIndex; // Vertices in this model (index into vertices[])
-    uint32 nTris, ofsTris;   // indices
-    uint32 nProps, ofsProps; // additional vtx properties
-    uint32 nSub, ofsSub;     // materials/renderops/submeshes
-    uint32 nTex, ofsTex;     // material properties/textures
-    int32 lod;               // LOD bias?
 };

 #pragma pack(pop)

diff --git a/contrib/vmap_extractor/vmapextract/wmo.cpp b/contrib/vmap_extractor/vmapextract/wmo.cpp
--- a/contrib/vmap_extractor/vmapextract/wmo.cpp
+++ b/contrib/vmap_extractor/vmapextract/wmo.cpp
@@ -347,13 +347,18 @@ int WMOGroup::ConvertToVMAPGroupWmo(FILE* output, WMORoot* rootWMO, bool pPrecis
             if (MOPY[2 * i]&WMO_MATERIAL_NO_COLLISION ||
                     !(MOPY[2 * i] & (WMO_MATERIAL_HINT | WMO_MATERIAL_COLLIDE_HIT)))
                 continue;
-            // Use this triangle
-            for (int j = 0; j < 3; ++j)
+
+            // use simplified model data
+            if (MOPY[2*i + 1] != 0xFF)
             {
-                IndexRenum[MOVI[3 * i + j]] = 1;
-                MoviEx[3 * nColTriangles + j] = MOVI[3 * i + j];
+                // Use this triangle
+                for (int j = 0; j < 3; ++j)
+                {
+                    IndexRenum[MOVI[3 * i + j]] = 1;
+                    MoviEx[3 * nColTriangles + j] = MOVI[3 * i + j];
+                }
+                ++nColTriangles;
             }
-            ++nColTriangles;
         }

         // assign new vertex index numbers

If I'm correct, this should also fix a bunch of annoying LOS issues in places with lots of low-hanging trees or spider webs, but I haven't tested it yet. Also the fix in wmo.cpp is optional since all that does is use lower resolution geometry on the wmos, maybe increasing performance? I haven't tested if there's any side effects to using that. Anyway, it's unrelated to the VMAP issue.

PatSmuk commented 10 years ago

Very nice. :+1:

249CAAFE40 commented 10 years ago

There still remains at least one more issue however. Some geometry is being incorrectly flipped. Example:

ingame invmap

The fallen tree log goes between the trees in the vmap but should be going away from the trees as shown in game. I'm not sure what's causing this or if it's even related to my fix.

249CAAFE40 commented 10 years ago

Ok this is weird. It isn't just that fallen tree that's rotated but it pretty much every tree that had enough detail to tell that it was rotated. Fallen logs too, lots of examples all over stonetalon. So then I went to tanaris and found more rotation problems there too.

tanaris-game tanaris-vmaps

Then I checked elwynn forest and everything seemed to be fine, and I checked stv and things seemed fine there too

stv-game stv-vmaps

So... models in kalimdor are rotated 180 degrees for some reason? But models in azeroth are fine? Weird. Also this isn't related to my fix, the problem seems to be there already.

Edit: I spoke too soon, there's some problem trees in stv too

stvbroken-game stvbroken-vmaps

I'm thinking the problem has something to do with the rotation vector's coordinates getting screwed up.

249CAAFE40 commented 10 years ago

More progress. The models weren't being rotated incorrectly, they are being flipped incorrectly on y-axis somewhere, I haven't found out where yet. Upon flipping it on the y-axis before sending it through the scale/rotate/position transforms, it resulted in some triangles being flipped (i.e. facing inside the geometry instead of outside, so I hack-fixed that with this https://github.com/Schmoozerd/mangos-tbc/commit/c6c96549f5c85ca5dc6a6377aac0eaf8b94ebd4b). The real problem still is likely in the vmaps, but I have no way to directly visualize what those look like so I will look through there until unmodified mmaps code produces the correct output, or until i give up (too many coordinate systems used here...)

Anyway here's the diff

diff --git a/contrib/mmap/src/TerrainBuilder.cpp b/contrib/mmap/src/TerrainBuilder.cpp

--- a/contrib/mmap/src/TerrainBuilder.cpp
+++ b/contrib/mmap/src/TerrainBuilder.cpp
@@ -712,7 +712,11 @@ namespace MMAP
         for (vector<Vector3>::iterator it = source.begin(); it != source.end(); ++it)
         {
             // apply tranform, then mirror along the horizontal axes
-            Vector3 v((*it) * rotation * scale + position);
+            Vector3 tmp;
+            tmp.x = (*it).x;
+            tmp.y = -(*it).y;
+            tmp.z = (*it).z;
+            Vector3 v((tmp) * scale * rotation + position); // * rotation * scale + position);
             v.x *= -1.f;
             v.y *= -1.f;
             transformedVertices.push_back(v);
@@ -737,6 +741,13 @@ namespace MMAP
         {
             for (vector<MeshTriangle>::iterator it = source.begin(); it != source.end(); ++it)
             {
+                dest.push_back((*it).idx0 + offset);
+                dest.push_back((*it).idx1 + offset);
+                dest.push_back((*it).idx2 + offset);
+            }
+
+            for (vector<MeshTriangle>::iterator it = source.begin(); it != source.end(); ++it)
+            {
                 dest.push_back((*it).idx2 + offset);
                 dest.push_back((*it).idx1 + offset);
                 dest.push_back((*it).idx0 + offset);
@@ -746,6 +757,13 @@ namespace MMAP
         {
             for (vector<MeshTriangle>::iterator it = source.begin(); it != source.end(); ++it)
             {
+                dest.push_back((*it).idx2 + offset);
+                dest.push_back((*it).idx1 + offset);
+                dest.push_back((*it).idx0 + offset);
+            }
+
+            for (vector<MeshTriangle>::iterator it = source.begin(); it != source.end(); ++it)
+            {
                 dest.push_back((*it).idx0 + offset);
                 dest.push_back((*it).idx1 + offset);
                 dest.push_back((*it).idx2 + offset);

mmaps-fixed

249CAAFE40 commented 10 years ago

Success! Same output as above without the hack-fixes. Using the movemapgen and vmap_assembler binaries provided in the repo, only modifying vmap_extractor. The extractor was already doing a y-z swap so I inverted the y there instead of doing it in movemapgen, but that caused assertion errors in vmap_assembler. Then I realized it wasn't also flipping the indices as well, so i swapped those too. That fixed the assertion errors and gives correct output in recast demo. This is probably ready for testing/committing now. Updated diff of model.cpp below, use above diffs of model.h, and modelheaders.h (and wmo.cpp if you like) for the other files, and ignore the diff of terrainbuilder.cpp, that's not needed anymore.

diff --git a/contrib/vmap_extractor/vmapextract/model.cpp b/contrib/vmap_extractor/vmapextract/model.cpp
old mode 100644
new mode 100755
index 8b9841f..627f506
--- a/contrib/vmap_extractor/vmapextract/model.cpp
+++ b/contrib/vmap_extractor/vmapextract/model.cpp
@@ -46,25 +46,20 @@ bool Model::open(StringSet& failedPaths)
     memcpy(&header, f.getBuffer(), sizeof(ModelHeader));
     if (header.nBoundingTriangles > 0)
     {
-        origVertices = (ModelVertex*)(f.getBuffer() + header.ofsVertices);
-        vertices = new Vec3D[header.nVertices];
+        boundingVertices = (ModelBoundingVertex*)(f.getBuffer() + header.ofsBoundingVertices);
+        vertices = new Vec3D[header.nBoundingVertices];

-        for (size_t i = 0; i < header.nVertices; i++)
+        for (size_t i = 0; i < header.nBoundingVertices; i++)
         {
-            vertices[i] = fixCoordSystem(origVertices[i].pos);;
+            vertices[i] = fixCoordSystem(boundingVertices[i].pos);
         }

-        ModelView* view = (ModelView*)(f.getBuffer() + header.ofsViews);
+        uint16* triangles = (uint16*)(f.getBuffer() + header.ofsBoundingTriangles);

-        uint16* indexLookup = (uint16*)(f.getBuffer() + view->ofsIndex);
-        uint16* triangles = (uint16*)(f.getBuffer() + view->ofsTris);
-
-        nIndices = view->nTris;
+        nIndices = header.nBoundingTriangles; // refers to the number of int16's, not the number of triangles
         indices = new uint16[nIndices];
-        for (size_t i = 0; i < nIndices; i++)
-        {
-            indices[i] = indexLookup[triangles[i]];
-        }
+        memcpy(indices, triangles, nIndices * 2);
+
         f.close();
     }
     else
@@ -87,7 +82,8 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     }
     fwrite(szRawVMAPMagic, 8, 1, output);
     uint32 nVertices = 0;
-    nVertices = header.nVertices;
+    nVertices = header.nBoundingVertices;
+
     fwrite(&nVertices, sizeof(int), 1, output);
     uint32 nofgroups = 1;
     fwrite(&nofgroups, sizeof(uint32), 1, output);
@@ -108,6 +104,15 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     fwrite(&nIndexes, sizeof(uint32), 1, output);
     if (nIndexes > 0)
     {
+        for (uint32 i = 0; i < nIndices; ++i)
+        {
+            if ((i % 3) - 1 == 0)
+            {
+                uint16 tmp = indices[i];
+                indices[i] = indices[i+1];
+                indices[i+1] = tmp;
+            }
+        }
         fwrite(indices, sizeof(unsigned short), nIndexes, output);
     }
     fwrite("VERT", 4, 1, output);
@@ -118,7 +123,10 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     {
         for (uint32 vpos = 0; vpos < nVertices; ++vpos)
         {
-            std::swap(vertices[vpos].y, vertices[vpos].z);
+             //std::swap(vertices[vpos].y, vertices[vpos].z);
+            float tmp = vertices[vpos].y;
+            vertices[vpos].y = -vertices[vpos].z;
+            vertices[vpos].z = tmp;
         }
         fwrite(vertices, sizeof(float) * 3, nVertices, output);
     }

cala commented 10 years ago

This is above my knowledge level, but this sounds very nice! You tested this against Classic, I read, but you think this should also be applied to TBC?

249CAAFE40 commented 10 years ago

It was above my knowledge level too until about 2 days ago :P and I still don't understand lots of it. I have a 2.4.3 client sitting around somewhere that I'll test against soon.

249CAAFE40 commented 10 years ago

Yes Classic, TBC and WotLK all need to be patched. WotLK diff is shorter because it was already using bounding geometry, so it only needed vertex/index flip fix.

WotLK diff:

diff --git a/contrib/vmap_extractor/vmapextract/model.cpp b/contrib/vmap_extractor/vmapextract/model.cpp

--- a/contrib/vmap_extractor/vmapextract/model.cpp
+++ b/contrib/vmap_extractor/vmapextract/model.cpp
@@ -102,6 +102,15 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     fwrite(&nIndexes, sizeof(uint32), 1, output);
     if (nIndexes > 0)
     {
+        for (uint32 i = 0; i < nIndexes; ++i)
+        {
+            if ((i % 3) - 1 == 0)
+            {
+                uint16 tmp = indices[i];
+                indices[i] = indices[i+1];
+                indices[i+1] = tmp;
+            }
+        }
         fwrite(indices, sizeof(unsigned short), nIndexes, output);
     }
     fwrite("VERT", 4, 1, output);
@@ -112,7 +121,9 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     {
         for (uint32 vpos = 0; vpos < nVertices; ++vpos)
         {
-            std::swap(vertices[vpos].y, vertices[vpos].z);
+            float tmp = vertices[vpos].y;
+            vertices[vpos].y = -vertices[vpos].z;
+            vertices[vpos].z = tmp;
         }
         fwrite(vertices, sizeof(float) * 3, nVertices, output);
     }

TBC diff:

diff --git a/contrib/vmap_extractor/vmapextract/model.cpp b/contrib/vmap_extractor/vmapextract/model.cpp

--- a/contrib/vmap_extractor/vmapextract/model.cpp
+++ b/contrib/vmap_extractor/vmapextract/model.cpp
@@ -46,25 +46,20 @@ bool Model::open(StringSet& failedPaths)
     memcpy(&header, f.getBuffer(), sizeof(ModelHeader));
     if (header.nBoundingTriangles > 0)
     {
-        origVertices = (ModelVertex*)(f.getBuffer() + header.ofsVertices);
-        vertices = new Vec3D[header.nVertices];
+        boundingVertices = (ModelBoundingVertex*)(f.getBuffer() + header.ofsBoundingVertices);
+        vertices = new Vec3D[header.nBoundingVertices];

-        for (size_t i = 0; i < header.nVertices; i++)
+        for (size_t i = 0; i < header.nBoundingVertices; i++)
         {
-            vertices[i] = fixCoordSystem(origVertices[i].pos);;
+            vertices[i] = fixCoordSystem(boundingVertices[i].pos);
         }

-        ModelView* view = (ModelView*)(f.getBuffer() + header.ofsViews);
+        uint16* triangles = (uint16*)(f.getBuffer() + header.ofsBoundingTriangles);

-        uint16* indexLookup = (uint16*)(f.getBuffer() + view->ofsIndex);
-        uint16* triangles = (uint16*)(f.getBuffer() + view->ofsTris);
-
-        nIndices = view->nTris;
+        nIndices = header.nBoundingTriangles;
         indices = new uint16[nIndices];
-        for (size_t i = 0; i < nIndices; i++)
-        {
-            indices[i] = indexLookup[triangles[i]];
-        }
+        memcpy(indices, triangles, nIndices * 2);
+
         f.close();
     }
     else
@@ -87,7 +82,7 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     }
     fwrite(szRawVMAPMagic, 8, 1, output);
     uint32 nVertices = 0;
-    nVertices = header.nVertices;
+    nVertices = header.nBoundingVertices;
     fwrite(&nVertices, sizeof(int), 1, output);
     uint32 nofgroups = 1;
     fwrite(&nofgroups, sizeof(uint32), 1, output);
@@ -108,6 +103,15 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     fwrite(&nIndexes, sizeof(uint32), 1, output);
     if (nIndexes > 0)
     {
+        for (uint32 i = 0; i < nIndices; ++i)
+        {
+            if ((i % 3) - 1 == 0)
+            {
+                uint16 tmp = indices[i];
+                indices[i] = indices[i+1];
+                indices[i+1] = tmp;
+            }
+        }
         fwrite(indices, sizeof(unsigned short), nIndexes, output);
     }
     fwrite("VERT", 4, 1, output);
@@ -118,7 +122,9 @@ bool Model::ConvertToVMAPModel(const char* outfilename)
     {
         for (uint32 vpos = 0; vpos < nVertices; ++vpos)
         {
-            std::swap(vertices[vpos].y, vertices[vpos].z);
+            float tmp = vertices[vpos].y;
+            vertices[vpos].y = -vertices[vpos].z;
+            vertices[vpos].z = tmp;
         }
         fwrite(vertices, sizeof(float) * 3, nVertices, output);
     }
diff --git a/contrib/vmap_extractor/vmapextract/model.h b/contrib/vmap_extractor/vmapextract/model.h

--- a/contrib/vmap_extractor/vmapextract/model.h
+++ b/contrib/vmap_extractor/vmapextract/model.h
@@ -35,7 +35,7 @@ class Model
 {
     public:
         ModelHeader header;
-        ModelVertex* origVertices;
+        ModelBoundingVertex* boundingVertices;
         Vec3D* vertices;
         uint16* indices;
         size_t nIndices;
diff --git a/contrib/vmap_extractor/vmapextract/modelheaders.h b/contrib/vmap_extractor/vmapextract/modelheaders.h

--- a/contrib/vmap_extractor/vmapextract/modelheaders.h
+++ b/contrib/vmap_extractor/vmapextract/modelheaders.h
@@ -101,25 +101,11 @@ struct ModelHeader

 };

-struct ModelVertex
+struct ModelBoundingVertex
 {
     Vec3D pos;
-    uint8 weights[4];
-    uint8 bones[4];
-    Vec3D normal;
-    Vec2D texcoords;
-    int unk1, unk2; // always 0,0 so this is probably unused
 };

-struct ModelView
-{
-    uint32 nIndex, ofsIndex; // Vertices in this model (index into vertices[])
-    uint32 nTris, ofsTris;   // indices
-    uint32 nProps, ofsProps; // additional vtx properties
-    uint32 nSub, ofsSub;     // materials/renderops/submeshes
-    uint32 nTex, ofsTex;     // material properties/textures
-    int32 lod;               // LOD bias?
-};

 #pragma pack(pop)
 #endif

cyberium commented 10 years ago

That seem realy interesting, i cannot test it right now but i'll do it asap.

Is someone could help on that post? ->https://github.com/cmangos/issues/issues/297#issuecomment-43458639

Schmoozerd commented 10 years ago

@249CAAFE40 I strongly suggest the following precedure: 1) Use a branch with the different commits, and especially create good commit notes Do not hesitate to make very small commits and lots of explanations 2) Create a pull-request with these commits 3) Reference this PR here ;)

249CAAFE40 commented 10 years ago

Pull requests

https://github.com/cmangos/mangos-classic/pull/77 https://github.com/cmangos/mangos-tbc/pull/27 https://github.com/cmangos/mangos-wotlk/pull/134

cmangos / issues

Pathfind - npc can go through tree objects #189