MovableObjectFactory has too much virtuality Topic is solved

[bishopnator]

Having MovableObjectFactory::createInstance and createInstanceImpl doesn't make sense if both methods are virtual. I can create custom implementation of MovableObjectFactory, override createInstance and createInstanceImpl is not called at all.

I would suggest, that either createInstance shouldn't be virtual or remove createInstanceImpl (for me removing createInstanceImpl sounds as better option).

What is the history of this class? Why there is createInstanceImpl method?

[dark_sylinc]

What is the history of this class? Why there is createInstanceImpl method?

It goes all way back to 1999 with Steve (sinbad) trying out the latest OOP (Object Oriented Programming) trends. Hence OGRE (OGRE was actually O-OGRE which means Object-Oriented Graphics Rendering Engine). It's like Java where instances are created through factories.

It used to be way worse before the OgreNext refactor.

It is indeed overkill. I honestly forget it's there.

There are many issues with object creation. Ideally (if I were to write this from scratch), we'd want users to be able to create objects from any thread; and once it's loaded the object would "join" the "scene" and only that join method needs to be thread safe.

Probably once an object becomes part of a scene, all objects in that scene must be manipulated in the same thread. But before joining the scene? Object creation should be possible from any thread.

However Ogre/OgreNext's design prevents this in many ways (another issue is that some of the older APIs like OpenGL and D3D11, but specially OpenGL, make this incredibly difficult).

[dark_sylinc]

Having MovableObjectFactory::createInstance and createInstanceImpl doesn't make sense if both methods are virtual. I can create custom implementation of MovableObjectFactory, override createInstance and createInstanceImpl is not called at all.

I forgot: we had a lot of these "lots of virtuals" that don't make sense, because a random guy in 2002 needed it and got merged.

OgreNext introduced the concept of compile-time OGRE_FLEXIBILITY_LEVEL macro:

Code: Select all

#if OGRE_FLEXIBILITY_LEVEL >= 0
#    define virtual_l0 virtual
#else
#    define virtual_l0
#endif
#if OGRE_FLEXIBILITY_LEVEL > 1
#    define virtual_l1 virtual
#else
#    define virtual_l1
#endif
#if OGRE_FLEXIBILITY_LEVEL > 2
#    define virtual_l2 virtual
#else
#    define virtual_l2
#endif

This way users that needed <X> function to be virtual could just increase the OGRE_FLEXIBILITY_LEVEL without penalizing everyone else who don't need this.
It sounds like MovableObjectFactory::createInstance should be either level 1 or 2.