[Map/Grid] New Map/Terrain Management System

[XTZGZoReX]

Unless anyone reports any serious issues, this patch will be committed in ~2-3 hours.

[xeross155]

Awesome work, won't bother testing any more as it would be inserted tomorrow and by then you'd have it in the main branch already.

[XTZGZoReX]

Found some critical bugs, so not committing just yet. Everything I know of has been fixed now.

Latest patch: http://paste2.org/p/749446

[wow-masters]

Can you tell us wat bugs you have fixed etc,

[yasuno]

Found some critical bugs, so not committing just yet. Everything I know of has been fixed now.

Latest patch: http://paste2.org/p/749446

Does that mean its being committed soon then ?!?!

[wow-masters]

I don't think this will be committed yet.

Am going to test this myself.

[cyrex]

Going to test also. Give me about 4 to 5 hours.

[VladimirMangos]

It still have stability and functionality problems.

[Stab]

Will this fix non-working collsion type things in Mango's like dismount on entering an inn, cave, WSG tunnels etc?

[xeross155]

Yeh zor reworked some of the locking code now to fix a crash someone had, so it does need some more development but it's definitely heading the good way.

[hunuza]

Will this fix non-working collsion type things in Mango's like dismount on entering an inn, cave, WSG tunnels etc?

You are probably looking for iomaps by arrai and Lynx3d http://github.com/arrai/mangos/commits/lynx3d_vmap_rewrite

[wow-masters]

WoW,

Didn't know anyone has done such a vmap rewrite. thats quite good. mangos are one step ahead of a blizz like core.

[ApoC]

I looked on to the patch (very quick look) and there are some problems i think. For example

bool getUnloadLock() const { return i_unloadActiveLockCount || i_unloadExplicitLock; }
...
void incUnloadActiveLock() { ++i_unloadActiveLockCount; }

If we want to have this working under multithreaded environment it will need real locking (mutex or other primitive) because this and similar operations are not atomic what can cause problems in real multithreaded environment. Even simple ++ -- operations are not atomic! It is good to keep this in mind. It can be solved by some fast mutex or using atomic operations (special CPU instructions for this purposes).

[XTZGZoReX]

If we want to have this working under multithreaded environment it will need real locking (mutex or other primitive) because this and similar operations are not atomic what can cause problems in real multithreaded environment. Even simple ++ -- operations are not atomic! It is good to keep this in mind. It can be solved by some fast mutex or using atomic operations (special CPU instructions for this purposes).

It's not needed. Simply because terrain data no longer locks grids (as it's now detached from the grid system) and because we never wanted grid-level threading to begin with. That's also why I plan to completely remove all the threading-related code in the grid/cell code.

[wow-masters]

It's not needed. Simply because terrain data no longer locks grids (as it's now detached from the grid system) and because we never wanted grid-level threading to begin with. That's also why I plan to completely remove all the threading-related code in the grid/cell code.

Quite true you dont need mutex locks in the grid sys.

[XTZGZoReX]

http://paste2.org/p/757553

"Release candidate" patch. Should have fixed all known issues...

SD2 patch (thx Skirnir): http://gist.github.com/356341

[XTZGZoReX]

Okay - really, REALLY fixed version: http://paste2.org/p/757750

Had a small mistake that broke instance map creation in there...

[wow-masters]

Could you try to update us, with wat you have fixed etc, or it will never get into the master branch, cause its hard to keep track.

[Stillhard]

Waiting for this.. commited in the master branch...

[XTZGZoReX]

Yet another; http://paste2.org/p/758162

Please try teleporting to groupmates inside instances. It should be fixed, but I can't confirm myself. Before, there was a bug where an instance copy would be made instead of being ported to the existing instance.

[Jordz]

Nice Job,

Commit Commit.

[Skirnir]

XTZGZoReX: I was going to try this patch today, but I ran into a compile error:

../../../src/game/MapManager.cpp: In member function ‘void MapManager::Update(uint32)’:

../../../src/game/MapManager.cpp:417: error: no matching function for call to ‘MapManager::_deleteMap(std::tr1::__detail::_Hashtable_iterator<std::Pair<const unsigned int, Map*>, false, false>, std::tr1::unordered_map<unsigned int, Map*, std::tr1::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::Pair<const unsigned int, Map*> > >&, NULL)’

../../../src/game/MapManager.h:119: note: candidates are: void MapManager::_deleteMap(std::tr1::__detail::_Hashtable_iterator<std::Pair<const unsigned int, Map*>, false, false>&, std::tr1::unordered_map<unsigned int, Map*, std::tr1::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::Pair<const unsigned int, Map*> > >&, ACE_Thread_Mutex*)

make[3]: *** [MapManager.o]

I had to patch World.h (a config option was added) and MiscHandler.cpp (a comment was added) manually.

[XTZGZoReX]

Yes, I was notified of those errors a while ago... And for the life of me, I just can't figure out why GCC won't compile it...

[Thyros]

please udpate this to actually rev.

[Wyk3d]

It's not needed. Simply because terrain data no longer locks grids (as it's now detached from the grid system) and because we never wanted grid-level threading to begin with. That's also why I plan to completely remove all the threading-related code in the grid/cell code.

Yes but keep in mind that no matter where you put the terrain data load/unload code, it may be called from two maps, hence threads, at the same time since one of the main points of this system is not to have duplicate terrain data for each instance of the maps. So you definitely need a proper locked reference count at load/unload but multiple threads should still be able to read the data without any locking while it's loaded.