C++11 / Boost / Shared_Ptr

Are there any plans to introduce smart pointers to the code (e.g. shared_ptr, weak_ptr, shared_array, etc.)? Those template classes perform pretty well and applied correctly, most of the pointer issues should be gone. With c++11 this will also be standard of C++.

You don’t go to see the use of this until gcc and vs with the support of c++11 is maxively implemented on mayor distros /emoticons/default_smile.png

You don’t need to wait, because everything is already available in the boost library. I use it in almost all my projects already.

C++11 will take some time to be published and available on all systems/distro’s. As for “pointer problems” I wasn’t aware we had them. For convenience when dealing with ie. QueryResult / Transaction auto cleanup/rollback we use ACE_Refcounted_Auto_Ptr<LockType, PointerType*>, but real “problems” I’m unaware of.

Right now I don’t see the need to adopt yet another library.

I’ve participated, years ago, from AspireCore and Sun++ migration from standard pointers to shared pointers and some related stuff. I didn’t see any improvement in the performance overall, the concept of shared pointers is good, maybe I’m wrong, but it will probably not improve the performance or the stability to implement it on TrinityCore.

One of the biggest problems in TC is that script devs and some core devs like to use a pointer instead of GUID to store a object in a class. When the object is deleted, the pointer becomes invalid and causes crash. Another problem is to abuse cast, such as ((Player*)unit)->DoSomething(), while unit may not be a player. These two bugs may cause memory corruption and are very difficult to fix, because you cannot see what is wrong by looking at the crash dump. Memory leak is not really an issue. I do not know if smart pointers can solve memory corruption problems.

For the bad casts, the solution is fairly simply, use a macro/function to cast, which uses static_cast in release and dynamic_cast in debug. Dynamic_cast throws std::bad_cast on a bad cast, so it’s easy to see the bad casts.

Smart pointers can solve memory corruption problems that happen due to writing to free’d memory, however it’s cost is that it can cause memory leaks if used wrong. Since it uses reference counting, if the last reference is never removed, it will never free the memory it points to ( until program termination that is ).

Such macro dynamic_cast has already been used in scripts. It was implemented when two years ago someone used a bad cast in a script and costed devs months to find out the reason of crashes. However, it may not be realistic to use dynamic_cast everywhere in the core. Very few large servers are willing to compile the code in debug mode. And if the core is compiled in debug mode, the number of players it can accommodate may not be enough to replicate the crash.

About bad scripting, I did not find a way to restrict the capability of the scripters. We cannot prohibit them from storing a pointer instead of a GUID in a class, or from using functions such as “RemoveFromWorld” which are not supposed to be used in scripts.

Usually the crash dump at least tells you where to look, and then you can just turn on debug mode an the dynamic_cast will do the rest. Experience shows that crashes that depend on player count are usually thread issue related ( at least this is my experience, but then again I’ve never used Trinity ), not bad casts.

Well with SAI you already achieved some safety, regarding scripting. Also you could use the idea I had for my own project, basically I want to create a scripting interface (basically it would be just a dumb proxy class) and expose only that to plugins (sadly no time to actually do it ). They would never take pointers only GUIDs. It also helps decoupling plugins from the core somewhat, so that when there are changes in the “core”, you only have to update the interface but not the plugins. So on the long term it’s worth it.

Something like ((Player*)unit)->SetValue(0) could cause memory corruption without crashing immediately if SetValue is a one-line inline function.

Even if you create a scripting interface, how can you prevent scripters from using core functions? For example, RemoveFromWorld is public. They can call it anywhere in script. You cannot hide it from the scripts. TC’s scripts used to be a dll, and you can only use those exported classes (but still, you can use every public method of them). Now it is a static library, and you have access to anything.

Very simply you just don’t expose the “core” stuff to plugins. Only the scripting interface. /emoticons/default_wink.png

But you can always include a header file and use everything in it.

Yes, you can do that. However the Trinity people can always just reject such patches, and not worry about crashes caused by 3rd party scripts.

It’s more than enough to worry about Trinity’s crashes /emoticons/default_wink.png

This is why we (should) have proper class design: public/protected/private/friend modifiers, and documentation on why something can or cannot be used outside of the core environment.

casting:

we have special methods for that, ToUnit(), ToPlayer(), ToCreature(). these will return NULL if the cast is invalid