PardCode
commented
1 year ago Thank you for your feedbacks and suggestions, mate! I'll try to fix every single issue you have pointed out in the next tutorials.
Open Lecrapouille opened 1 year ago
PardCode
commented
1 year ago Thank you for your feedbacks and suggestions, mate! I'll try to fix every single issue you have pointed out in the next tutorials.
In https://github.com/PardCode/OpenGL-3D-Game-Tutorial-Series/blob/4d381770a8e1cf3b2850f4cc236568856de7fa5d/Tutorial8_EntitySystem_Basics/Game/MyGame.cpp#L41 not missing calling OEntitySystem::update ?
In https://github.com/PardCode/OpenGL-3D-Game-Tutorial-Series/blob/4d381770a8e1cf3b2850f4cc236568856de7fa5d/Tutorial8_EntitySystem_Basics/OGL3D/source/OGL3D/Entity/OEntitySystem.cpp#L41 I find sad to set each field this way, this is error-prone (missing one, i.e.) better to use the constructor to pass these parameters: the compiler will warn you in case of missing one.
In https://github.com/PardCode/OpenGL-3D-Game-Tutorial-Series/blob/4d381770a8e1cf3b2850f4cc236568856de7fa5d/Tutorial8_EntitySystem_Basics/OGL3D/source/OGL3D/Entity/OEntitySystem.cpp#L54 You use m_entitiesToDestroy for not interfering deleting elements when iterating on them, but why not for createEntityInternal ? I guess you can create entities during the entity->onUpdate ? I guess a fake solution is to use mutex because creating entities at run-time will create a dead lock. But point to take into account when threading your game. Another solution would be unordered_map (confer section Iterator validity https://cplusplus.com/reference/unordered_map/unordered_map/erase/ vs https://cplusplus.com/reference/map/map/erase/)
Risk of recursivity https://github.com/PardCode/OpenGL-3D-Game-Tutorial-Series/blob/4d381770a8e1cf3b2850f4cc236568856de7fa5d/Tutorial8_EntitySystem_Basics/OGL3D/source/OGL3D/Entity/OEntitySystem.cpp#L44 entity->onCreate() creating entity calling onCreate() creating entity callin onCreate ...
In https://github.com/PardCode/OpenGL-3D-Game-Tutorial-Series/blob/4d381770a8e1cf3b2850f4cc236568856de7fa5d/Tutorial8_EntitySystem_Basics/OGL3D/source/OGL3D/Entity/OEntity.cpp#L32 your destructor does not call release() and release() should be private.
In https://github.com/PardCode/OpenGL-3D-Game-Tutorial-Series/blob/4d381770a8e1cf3b2850f4cc236568856de7fa5d/Tutorial8_EntitySystem_Basics/OGL3D/include/OGL3D/Entity/OEntity.h#L44 maybe m_entitySystem could be static: no more getEntitySystem()