MWRender Refactoring
Re: MWRender Refactoring
Only in mwclass/*.cpp please. Those people with slower computers will thank you for it (avoiding includes in hpp files where possible can improve compile time a lot).
Re: MWRender Refactoring
Almost:
Including objects.hpp in mwclass/creature.cpp and mwclass/npc.cpp is pointless, since you won't be using it.
You added insert functions for itemlevlist.hpp. But this class is currently not rendered (we still need to figure out how to handle it). Also, adding the function only to the hpp file, will result in a link time error. Best remove them again for now.
The include guards are mostly okay. We use a slightly different naming scheme (compare it with other files). Also the underscore at the beginning is theoretically not allowed in C++ (reserved name), though I don't know of any compiler that would reject it. You can leave the new include guards unchanged, but please keep these points in mind when writing new files again.
The renderinginterface.hpp include in renderingmanager.cpp is redundant, since you are including this file already in the header.
Including objects.hpp in mwclass/creature.cpp and mwclass/npc.cpp is pointless, since you won't be using it.
You added insert functions for itemlevlist.hpp. But this class is currently not rendered (we still need to figure out how to handle it). Also, adding the function only to the hpp file, will result in a link time error. Best remove them again for now.
The include guards are mostly okay. We use a slightly different naming scheme (compare it with other files). Also the underscore at the beginning is theoretically not allowed in C++ (reserved name), though I don't know of any compiler that would reject it. You can leave the new include guards unchanged, but please keep these points in mind when writing new files again.
The renderinginterface.hpp include in renderingmanager.cpp is redundant, since you are including this file already in the header.
Re: MWRender Refactoring
Changed a lot of things in scene and world. Ignore the backup files, committed them on accident.
Please let me know of anything that is wrong. There are 3 new commits.
Please let me know of anything that is wrong. There are 3 new commits.
Re: MWRender Refactoring
That was a pretty large commit. Not sure if I found all problems. But this is what I see:
1. the loadCell function is incomplete. You still need to iterate over all MW-references in the cell and call RenderingManager::addObject on it.
2. As far as I can see the mwRoot argument in the Scene's constructor is unused.
3. The enable and disable functions won't work. You don't actually enable/disable anything in the rendering system. Tricky. It seems we overlooked this feature and since I am planning to do some changes to the non-rendering part of enabling/disabling after you are finished with mwrender, anything we adjust now will only cause problems later.
I suggest you add a doxygen To-Do comment in both functions (/// \todo enable/disable reference in the OGRE scene). I will then take care of it later.
4. All those looping constructs are unfortunate. This kinda of repetition reduces the maintainability of the code and also these loops express the intend of the code badly. Maybe it would have been better to use a std::set instead of a std::list and utilise the build-in ordering to check if a cell is listed.
5. The removeObject code is wrong. Shouldn't even compile. I repeat, the RenderingInterface is meant for use by the MWClass-hierarchy. Do not try to make use of it from somewhere else. Please use the RenderingManager instead.
6. You still have plenty of out-commented code in the source. Please make sure you remove it.
1. the loadCell function is incomplete. You still need to iterate over all MW-references in the cell and call RenderingManager::addObject on it.
2. As far as I can see the mwRoot argument in the Scene's constructor is unused.
3. The enable and disable functions won't work. You don't actually enable/disable anything in the rendering system. Tricky. It seems we overlooked this feature and since I am planning to do some changes to the non-rendering part of enabling/disabling after you are finished with mwrender, anything we adjust now will only cause problems later.
I suggest you add a doxygen To-Do comment in both functions (/// \todo enable/disable reference in the OGRE scene). I will then take care of it later.
4. All those looping constructs are unfortunate. This kinda of repetition reduces the maintainability of the code and also these loops express the intend of the code badly. Maybe it would have been better to use a std::set instead of a std::list and utilise the build-in ordering to check if a cell is listed.
5. The removeObject code is wrong. Shouldn't even compile. I repeat, the RenderingInterface is meant for use by the MWClass-hierarchy. Do not try to make use of it from somewhere else. Please use the RenderingManager instead.
6. You still have plenty of out-commented code in the source. Please make sure you remove it.
Re: MWRender Refactoring
So make new insertCell() and insertCellRefList() functions like the ones in cellimp?Zini wrote:That was a pretty large commit. Not sure if I found all problems. But this is what I see:
1. the loadCell function is incomplete. You still need to iterate over all MW-references in the cell and call RenderingManager::addObject on it.
Do you mean PhysicsSystem::removeObject() or World::deleteObject() or Object::deleteObject()? I didn't use the RenderingInterface in any of these.Zini wrote:
5. The removeObject code is wrong. Shouldn't even compile. I repeat, the RenderingInterface is meant for use by the MWClass-hierarchy. Do not try to make use of it from somewhere else. Please use the RenderingManager instead.
Re: MWRender Refactoring
Yes. The signatures will probably change a bit, but otherwise you can copy and paste these functions.So make new insertCell() and insertCellRefList() functions like the ones in cellimp?
I see. Okay. The call to PhysicsSystem::removeObject is correct, but you still need to remove the object from the RenderingManager.Do you mean PhysicsSystem::removeObject() or World::deleteObject() or Object::deleteObject()? I didn't use the RenderingInterface in any of these.
Re: MWRender Refactoring
I am getting this error with my latest commit:
Error 28 error C2243: 'type cast' : conversion from 'MWRender::RenderingManager *' to 'MWRender::RenderingInterface &' exists, but is inaccessible C:\c\openmw\apps\openmw\mwworld\scene.cpp 63
The only way I know of to fix this is public inheritance from RenderingInterface, it was specified earlier in this topic that it needed to be private.
Error 28 error C2243: 'type cast' : conversion from 'MWRender::RenderingManager *' to 'MWRender::RenderingInterface &' exists, but is inaccessible C:\c\openmw\apps\openmw\mwworld\scene.cpp 63
The only way I know of to fix this is public inheritance from RenderingInterface, it was specified earlier in this topic that it needed to be private.
Re: MWRender Refactoring
Okay. I repeat again (and this time rightfully and not because I misread your code ): Do not use the RenderingInterface outside of the MWClass hierarchy. Rendering requests should be run through the RenderingManager (RenderingManager::addObject).