XTZGZoReX

And yet again, new patch against latest master: http://paste2.org/p/746655 This one has some bug fixes for grid reference counting. It *should* work now...

New patch: http://paste2.org/p/746088 Mostly just cleanups (suggestions from vladimir) and random small fixes.

Nope. It only uses the functions that the VMap API provides. But, I'll have Lynx3d look at that crash...

Sure thing. In fact, anything above 1 player will do .

New patch: http://paste2.org/p/745250 Some random cleanups + re-organization of GridMap and related classes. Also should fix Linux build issues.

New patch for current master (untested, but has necessary changes, so should apply): http://paste2.org/p/745173

Grids consist of two things - objects and map data (or rather, they did, before this patch); either of which can keep a grid loaded. GridUnload in config simply sets an explicit lock on the grid (which is never unlocked, thus, keeping the grid constantly active). I'm not sure what would cause the problems you're facing; those are spawn-related and not really related to this patch as such, so... Anyway, with my patch, map data is handled separately. It is not directly tied to the grid system at all - in fact, you could just go ahead and do something like this anywhere you wanted (almost; of course not inside the terrain code itself): ... TerrainData* data = sTerrainMgr.GetData(571); float height = data->GetHeight(100.0f, 200.0f, 300.0f); sTerrainMgr.DisposeData(data); ... Basically meaning that grid unloading is not directly related to map data anymore - memory management of map data is handled solely by the TerrainMgr, nothing else. Of course, bugs can happen if the code that uses TerrainMgr doesn't properly dispose of map data when done with it (i.e. reduce the reference count), but I haven't encountered such issues so far. Strictly speaking, there's no difference from before. From a developer's point of view, though, the memory management is much nicer now.

Why would the process crash every 5 minutes in the first place? Then something is clearly wrong elsewhere... But yes, if that really would be the case, it would be a disadvantage. But really, it's up to you. On-demand loading is still there, if you really want to use it. Anyway, we can't always take possible crashes into account everywhere in the core.

In MaNGOS as it is now, there's no significant gain in this - the actual gain will be clear when MaNGOS gets map multithreading. Depending on player count, a huge advantage! Whenever a grid is unloaded, map data is freed, and thus needs to be reloaded the next time that grid is loaded. In fact, map file I/O happens very often on high population servers. You would gain a lot of performance from not having to do this at runtime. While it has nothing to do with movement pathing as such, it certainly should make the job easier for whoever wants to implement it.

Right, forgot to mention... I didn't test the Linux build. If anyone could check that out for me, that'd be great. If not, I'll get around to it... soon.

Hi, This is the result of 3 days of non-stop work, consisting of countless hours of pulling my hair trying to understand the grid system . It implements a new terrain management system in MaNGOS which will have several benefits, particularly for future multithreading. Somewhat full list of changes: * Added new TerrainMgr with reference-counted TerrainData objects (as well as reference counting for each GridMap allocated within it). It will serve as the proxy for obtaining terrain information, and can also do terrain calculations when you don't have a Map object available. * Added support for pre-loading all maps. Yes, this means that you people with loads of RAM will benefit, as you can avoid map file I/O. The current memory usage of both maps and VMaps fully loaded in memory is approximately 2.5 GB. Loading on-demand is still supported. Loading all map data takes around 5 minutes on my i7 960. * Rewrote MapManager almost completely. It now only has the functions it *really* needs to have, though some more cleanup will also happen in the future. * Removed the MapInstanced class and all "base map" functionality completely. Now that TerrainMgr can give us terrain information without loading a Map object, that's how it should be done. This also gives us the advantage that multiple instances of the same map ID can be updated concurrently in the future. * Made several functions related to area/zone ID retrieval from map files static. These functions never needed a Map object to begin with, and thus unnecessarily created Map/MapInstanced objects in memory - just to use DBC files. * Lots of random cleanups, some typo fixes, and nicer code because of the above . What I need most of all right now is some live testing on servers with players. I've done lots of local tests in different scenarios, and everything looked to be good, but of course, one single player isn't enough to determine the stability of this patch. Link to full patch: http://paste2.org/p/743853 Comments, suggestions, bug reports, etc are all welcome. Be advised, this patch should NOT be run on a server that uses some multithreaded maps patch, as it was not written with this in mind - yet. Also, thanks to everyone who's helped me in some way in the process (I won't give specifics, in case I forget someone.....)!

As said, I'm pretty sure these freezes originate from elsewhere...

After thorough investigations in the client code, it definitely expects a packed GUID here, nothing else. For properly showing caster/target names for DoTs for example.Theoretically speaking, the GUID isn't 'needed' - but then again, a lot of stuff isn't . But to be blizzlike (and to fix what I just mentioned), we have to include it. My bet is that something is going wrong elsewhere - though I don't know where.

double->float cast can lose precision, afaik.

This header is either not standard, or old. Including stdio should be enough.

Thanks for the testing, guys. If possible, try to get some more info on when/how the (much less frequent) freezes happen. Not related.

Can you try to give me an exact way to reproduce this? Would be much easier to check that way... Also, try the two patches I posted above.

I'll check that function, but please also test the above patch. EDIT: Also, here's QAston's patch to fix appendPackGUID: diff --git a/src/shared/ByteBuffer.h b/src/shared/ByteBuffer.h index eec35c8..152917f 100644 --- a/src/shared/ByteBuffer.h +++ b/src/shared/ByteBuffer.h @@ -385,21 +385,22 @@ class ByteBuffer void appendPackGUID(uint64 guid) { - if (_storage.size() < _wpos + sizeof(guid) + 1) - _storage.resize(_wpos + sizeof(guid) + 1); - - size_t mask_position = wpos(); - *this << uint8(0); - for(uint8 i = 0; i < 8; ++i) + uint8 packGUID[8+1]; + packGUID[0] = 0; + size_t size = 1; + for (uint8 i = 0; guid != 0; ++i) { if(guid & 0xFF) { - _storage[mask_position] |= uint8(1 << i); - *this << uint8(guid & 0xFF); + packGUID[0] |= uint8(1 << i); + packGUID[size] = uint8(guid & 0xFF); + ++size; } guid >>= 8; } + + append(packGUID, size); } void put(size_t pos, const uint8 *src, size_t cnt)

Please try this patch and test if the functionality still is as it should be and if the crashes are gone: diff --git a/src/game/SpellAuras.cpp b/src/game/SpellAuras.cpp index 3ee1892..56ba367 100644 --- a/src/game/SpellAuras.cpp +++ b/src/game/SpellAuras.cpp @@ -1029,7 +1029,7 @@ void Aura::_AddAura() if(slot < MAX_AURAS) // slot found send data to client { SetAura(false); - SetAuraFlags((1 << GetEffIndex()) | AFLAG_NOT_CASTER | ((GetAuraMaxDuration() > 0) ? AFLAG_DURATION : AFLAG_NONE) | (IsPositive() ? AFLAG_POSITIVE : AFLAG_NEGATIVE)); + SetAuraFlags((1 << GetEffIndex()) | ((GetCasterGUID() == GetTarget()->GetGUID()) ? AFLAG_NOT_CASTER : AFLAG_NONE) | ((GetAuraMaxDuration() > 0) ? AFLAG_DURATION : AFLAG_NONE) | (IsPositive() ? AFLAG_POSITIVE : AFLAG_NEGATIVE)); SetAuraLevel(caster ? caster->getLevel() : sWorld.getConfig(CONFIG_UINT32_MAX_PLAYER_LEVEL)); SendAuraUpdate(false); }

diff --git a/src/game/Player.cpp b/src/game/Player.cpp index c45fc3f..e2bb2a0 100644 --- a/src/game/Player.cpp +++ b/src/game/Player.cpp @@ -19350,9 +19350,9 @@ void Player::SendAurasForTarget(Unit *target) else data << uint8(aura->GetStackAmount()); - if(!(auraFlags & AFLAG_NOT_CASTER)) + if(!(auraFlags & AFLAG_NOT_CASTER)) // packed GUID of caster { - data << uint8(0); // packed GUID of someone (caster?) + data.appendPackGUID(aura->GetCasterGUID()); } if(auraFlags & AFLAG_DURATION) // include aura duration diff --git a/src/game/SpellAuras.cpp b/src/game/SpellAuras.cpp index 18b4af6..3ee1892 100644 --- a/src/game/SpellAuras.cpp +++ b/src/game/SpellAuras.cpp @@ -1249,7 +1249,7 @@ void Aura::SendAuraUpdate(bool remove) if(!(auraFlags & AFLAG_NOT_CASTER)) { - data << uint8(0); // pguid + data.appendPackGUID(GetCasterGUID()); } if(auraFlags & AFLAG_DURATION) Test, report back, etc.

NoFantasy and I came to the conclusion that an index was needed. However, I think ApoC is in the process of doing further research.

I'm sure you have the best hardware in the world. /sarcasm

No objections (already said this at IRC, but just posting here also).

Lastly, can you please provide a proper SQL update? I.e. alter the existing tables instead of completely dropping/re-creating them.

Btw, according to TOM_RUS: unk2 = floorId.