Index: ps/trunk/source/graphics/Decal.h =================================================================== --- ps/trunk/source/graphics/Decal.h +++ ps/trunk/source/graphics/Decal.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -61,7 +61,7 @@ return this; } - virtual CModelAbstract* Clone() const; + virtual std::unique_ptr Clone() const; virtual void SetTerrainDirty(ssize_t i0, ssize_t j0, ssize_t i1, ssize_t j1); Index: ps/trunk/source/graphics/Decal.cpp =================================================================== --- ps/trunk/source/graphics/Decal.cpp +++ ps/trunk/source/graphics/Decal.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -23,10 +23,9 @@ #include "maths/MathUtil.h" #include "ps/CStrInternStatic.h" -CModelAbstract* CModelDecal::Clone() const +std::unique_ptr CModelDecal::Clone() const { - CModelDecal* clone = new CModelDecal(m_Terrain, m_Decal); - return clone; + return std::make_unique(m_Terrain, m_Decal); } void CModelDecal::CalcVertexExtents(ssize_t& i0, ssize_t& j0, ssize_t& i1, ssize_t& j1) Index: ps/trunk/source/graphics/Model.h =================================================================== --- ps/trunk/source/graphics/Model.h +++ ps/trunk/source/graphics/Model.h @@ -49,28 +49,26 @@ public: struct Prop { - Prop() : m_MinHeight(0.f), m_MaxHeight(0.f), m_Point(0), m_Model(0), m_ObjectEntry(0), m_Hidden(false), m_Selectable(true) {} - - float m_MinHeight; - float m_MaxHeight; + float m_MinHeight{0.0f}; + float m_MaxHeight{0.0f}; /** * Location of the prop point within its parent model, relative to either a bone in the parent model or to the * parent model's origin. See the documentation for @ref SPropPoint for more details. * @see SPropPoint */ - const SPropPoint* m_Point; + const SPropPoint* m_Point{nullptr}; /** * Pointer to the model associated with this prop. Note that the transform matrix held by this model is the full object-to-world * space transform, taking into account all parent model positioning (see @ref CModel::ValidatePosition for positioning logic). * @see CModel::ValidatePosition */ - CModelAbstract* m_Model; - CObjectEntry* m_ObjectEntry; + std::unique_ptr m_Model; + CObjectEntry* m_ObjectEntry{nullptr}; - bool m_Hidden; ///< Should this prop be temporarily removed from rendering? - bool m_Selectable; /// < should this prop count in the selection size? + bool m_Hidden{false}; ///< Should this prop be temporarily removed from rendering? + bool m_Selectable{true}; /// < should this prop count in the selection size? }; public: @@ -182,13 +180,13 @@ /** * Add a prop to the model on the given point. */ - void AddProp(const SPropPoint* point, CModelAbstract* model, CObjectEntry* objectentry, float minHeight = 0.f, float maxHeight = 0.f, bool selectable = true); + void AddProp(const SPropPoint* point, std::unique_ptr model, CObjectEntry* objectentry, float minHeight = 0.f, float maxHeight = 0.f, bool selectable = true); /** * Add a prop to the model on the given point, and treat it as the ammo prop. * The prop will be hidden by default. */ - void AddAmmoProp(const SPropPoint* point, CModelAbstract* model, CObjectEntry* objectentry); + void AddAmmoProp(const SPropPoint* point, std::unique_ptr model, CObjectEntry* objectentry); /** * Show the ammo prop (if any), and hide any other props on that prop point. @@ -210,7 +208,7 @@ const std::vector& GetProps() const { return m_Props; } // return a clone of this model - virtual CModelAbstract* Clone() const; + virtual std::unique_ptr Clone() const; /** * Ensure that both the transformation and the bone Index: ps/trunk/source/graphics/Model.cpp =================================================================== --- ps/trunk/source/graphics/Model.cpp +++ ps/trunk/source/graphics/Model.cpp @@ -60,9 +60,6 @@ CModel::~CModel() { rtl_FreeAligned(m_BoneMatrices); - - for (Prop& prop : m_Props) - delete prop.m_Model; } ///////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -385,7 +382,7 @@ ///////////////////////////////////////////////////////////////////////////////////////////////////////////// // AddProp: add a prop to the model on the given point -void CModel::AddProp(const SPropPoint* point, CModelAbstract* model, CObjectEntry* objectentry, float minHeight, float maxHeight, bool selectable) +void CModel::AddProp(const SPropPoint* point, std::unique_ptr model, CObjectEntry* objectentry, float minHeight, float maxHeight, bool selectable) { // position model according to prop point position @@ -395,17 +392,17 @@ Prop prop; prop.m_Point = point; - prop.m_Model = model; + prop.m_Model = std::move(model); prop.m_ObjectEntry = objectentry; prop.m_MinHeight = minHeight; prop.m_MaxHeight = maxHeight; prop.m_Selectable = selectable; - m_Props.push_back(prop); + m_Props.push_back(std::move(prop)); } -void CModel::AddAmmoProp(const SPropPoint* point, CModelAbstract* model, CObjectEntry* objectentry) +void CModel::AddAmmoProp(const SPropPoint* point, std::unique_ptr model, CObjectEntry* objectentry) { - AddProp(point, model, objectentry); + AddProp(point, std::move(model), objectentry); m_AmmoPropPoint = point; m_AmmoLoadedProp = m_Props.size() - 1; m_Props[m_AmmoLoadedProp].m_Hidden = true; @@ -448,7 +445,7 @@ CModelAbstract* CModel::FindFirstAmmoProp() { if (m_AmmoPropPoint) - return m_Props[m_AmmoLoadedProp].m_Model; + return m_Props[m_AmmoLoadedProp].m_Model.get(); for (size_t i = 0; i < m_Props.size(); ++i) { @@ -466,9 +463,9 @@ ///////////////////////////////////////////////////////////////////////////////////////////////////////////// // Clone: return a clone of this model -CModelAbstract* CModel::Clone() const +std::unique_ptr CModel::Clone() const { - CModel* clone = new CModel(m_Simulation, m_Material, m_pModelDef); + std::unique_ptr clone = std::make_unique(m_Simulation, m_Material, m_pModelDef); clone->m_ObjectBounds = m_ObjectBounds; clone->SetAnimation(m_Anim); clone->SetFlags(m_Flags); Index: ps/trunk/source/graphics/ModelAbstract.h =================================================================== --- ps/trunk/source/graphics/ModelAbstract.h +++ ps/trunk/source/graphics/ModelAbstract.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -23,6 +23,8 @@ #include "maths/BoundingBoxOriented.h" #include "simulation2/helpers/Player.h" +#include + class CModelDummy; class CModel; class CModelDecal; @@ -71,7 +73,7 @@ delete m_CustomSelectionShape; // allocated and set externally by CCmpVisualActor, but our responsibility to clean up } - virtual CModelAbstract* Clone() const = 0; + virtual std::unique_ptr Clone() const = 0; /// Dynamic cast virtual CModelDummy* ToCModelDummy() { return nullptr; } Index: ps/trunk/source/graphics/ModelDummy.h =================================================================== --- ps/trunk/source/graphics/ModelDummy.h +++ ps/trunk/source/graphics/ModelDummy.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -32,13 +32,13 @@ CModelDummy() = default; virtual ~CModelDummy() = default; - virtual CModelAbstract* Clone() const { return new CModelDummy(); } + virtual std::unique_ptr Clone() const { return std::make_unique(); } virtual CModelDummy* ToCModelDummy() { return this; } - virtual void CalcBounds() {}; + virtual void CalcBounds() {} virtual void SetTerrainDirty(ssize_t, ssize_t, ssize_t, ssize_t) {} - virtual void ValidatePosition() {}; - virtual void InvalidatePosition() {}; + virtual void ValidatePosition() {} + virtual void InvalidatePosition() {} }; #endif // INCLUDED_MODELDUMMY Index: ps/trunk/source/graphics/ObjectEntry.cpp =================================================================== --- ps/trunk/source/graphics/ObjectEntry.cpp +++ ps/trunk/source/graphics/ObjectEntry.cpp @@ -245,13 +245,13 @@ const SPropPoint* proppoint = modeldef->FindPropPoint(ppn.c_str()); if (proppoint) { - CModelAbstract* propmodel = oe->m_Model->Clone(); - if (isAmmo) - model->AddAmmoProp(proppoint, propmodel, oe); - else - model->AddProp(proppoint, propmodel, oe, prop.m_minHeight, prop.m_maxHeight, prop.m_selectable); + std::unique_ptr propmodel = oe->m_Model->Clone(); if (propmodel->ToCModel()) propmodel->ToCModel()->SetAnimation(oe->GetRandomAnimation("idle")); + if (isAmmo) + model->AddAmmoProp(proppoint, std::move(propmodel), oe); + else + model->AddProp(proppoint, std::move(propmodel), oe, prop.m_minHeight, prop.m_maxHeight, prop.m_selectable); } else LOGERROR("Failed to find matching prop point called \"%s\" in model \"%s\" for actor \"%s\"", ppn, m_ModelName.string8(), m_Base->GetIdentifier()); Index: ps/trunk/source/graphics/ParticleEmitter.h =================================================================== --- ps/trunk/source/graphics/ParticleEmitter.h +++ ps/trunk/source/graphics/ParticleEmitter.h @@ -197,7 +197,7 @@ return this; } - virtual CModelAbstract* Clone() const; + virtual std::unique_ptr Clone() const; virtual void SetTerrainDirty(ssize_t UNUSED(i0), ssize_t UNUSED(j0), ssize_t UNUSED(i1), ssize_t UNUSED(j1)) { Index: ps/trunk/source/graphics/ParticleEmitter.cpp =================================================================== --- ps/trunk/source/graphics/ParticleEmitter.cpp +++ ps/trunk/source/graphics/ParticleEmitter.cpp @@ -281,9 +281,9 @@ m_Emitter->SetEntityVariable(name, value); } -CModelAbstract* CModelParticleEmitter::Clone() const +std::unique_ptr CModelParticleEmitter::Clone() const { - return new CModelParticleEmitter(m_Type); + return std::make_unique(m_Type); } void CModelParticleEmitter::CalcBounds() Index: ps/trunk/source/graphics/Unit.h =================================================================== --- ps/trunk/source/graphics/Unit.h +++ ps/trunk/source/graphics/Unit.h @@ -22,6 +22,7 @@ #include "simulation2/system/Entity.h" // entity_id_t #include +#include #include class CActorDef; @@ -83,8 +84,8 @@ const CActorDef& m_Actor; // object from which unit was created; never NULL once fully created. CObjectEntry* m_Object = nullptr; - // object model representation; never NULL once fully created. - CModelAbstract* m_Model = nullptr; + // object model representation; never nullptr once fully created. + std::unique_ptr m_Model; CUnitAnimation* m_Animation = nullptr; Index: ps/trunk/source/graphics/Unit.cpp =================================================================== --- ps/trunk/source/graphics/Unit.cpp +++ ps/trunk/source/graphics/Unit.cpp @@ -45,7 +45,6 @@ CUnit::~CUnit() { delete m_Animation; - delete m_Model; } CUnit* CUnit::Create(const CStrW& actorName, const entity_id_t id, const uint32_t seed, CObjectManager& objectManager) @@ -128,7 +127,7 @@ else if (m_Object && newObject != m_Object) { // Clone the new object's base (non-instance) model - CModelAbstract* newModel = newObject->m_Model->Clone(); + std::unique_ptr newModel = newObject->m_Model->Clone(); // Copy the old instance-specific settings from the old model to the new instance newModel->SetTransform(m_Model->GetTransform()); @@ -142,8 +141,7 @@ newModel->ToCModel()->AddFlagsRec(instanceFlags); } - delete m_Model; - m_Model = newModel; + m_Model = std::move(newModel); m_Object = newObject; if (m_Model->ToCModel()) Index: ps/trunk/source/graphics/tests/test_Model.h =================================================================== --- ps/trunk/source/graphics/tests/test_Model.h +++ ps/trunk/source/graphics/tests/test_Model.h @@ -0,0 +1,83 @@ +/* Copyright (C) 2023 Wildfire Games. + * This file is part of 0 A.D. + * + * 0 A.D. is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * 0 A.D. is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with 0 A.D. If not, see . + */ + +#include "lib/self_test.h" + +#include "graphics/Material.h" +#include "graphics/Model.h" +#include "graphics/ModelDef.h" +#include "graphics/ShaderDefines.h" +#include "ps/CStrInternStatic.h" +#include "scriptinterface/ScriptInterface.h" +#include "simulation2/Simulation2.h" + +#include + +class TestModel : public CxxTest::TestSuite +{ +public: + bool HasShaderDefine(const CShaderDefines& defines, CStrIntern define) + { + const auto& map = defines.GetMap(); + const auto it = map.find(define); + return it != map.end() && it->second == str_1; + } + + bool HasMaterialDefine(CModel* model, CStrIntern define) + { + return HasShaderDefine(model->GetMaterial().GetShaderDefines(), define); + } + + void test_model_with_flags() + { + CMaterial material{}; + CSimulation2 simulation{nullptr, g_ScriptContext, nullptr}; + + // TODO: load a proper mock for modeldef. + CModelDefPtr modeldef = std::make_shared(); + + std::unique_ptr model = std::make_unique(simulation, material, modeldef); + + SPropPoint propPoint{}; + model->AddProp(&propPoint, std::make_unique(simulation, material, modeldef), nullptr); + + model->AddFlagsRec(MODELFLAG_IGNORE_LOS); + model->RemoveShadowsRec(); + + TS_ASSERT(HasMaterialDefine(model.get(), str_DISABLE_RECEIVE_SHADOWS)); + TS_ASSERT(HasMaterialDefine(model.get(), str_IGNORE_LOS)); + for (const CModel::Prop& prop : model->GetProps()) + { + TS_ASSERT(prop.m_Model->ToCModel()); + TS_ASSERT(HasMaterialDefine(prop.m_Model->ToCModel(), str_DISABLE_RECEIVE_SHADOWS)); + TS_ASSERT(HasMaterialDefine(prop.m_Model->ToCModel(), str_IGNORE_LOS)); + } + + std::unique_ptr clonedModel = model->Clone(); + TS_ASSERT(clonedModel->ToCModel()); + TS_ASSERT(HasMaterialDefine(clonedModel->ToCModel(), str_DISABLE_RECEIVE_SHADOWS)); + TS_ASSERT(HasMaterialDefine(clonedModel->ToCModel(), str_IGNORE_LOS)); + + TS_ASSERT_EQUALS(model->GetProps().size(), clonedModel->ToCModel()->GetProps().size()); + for (const CModel::Prop& prop : clonedModel->ToCModel()->GetProps()) + { + TS_ASSERT(prop.m_Model->ToCModel()); + TS_ASSERT(HasMaterialDefine(prop.m_Model->ToCModel(), str_DISABLE_RECEIVE_SHADOWS)); + TS_ASSERT(HasMaterialDefine(prop.m_Model->ToCModel(), str_IGNORE_LOS)); + } + } +}; Index: ps/trunk/source/renderer/Scene.cpp =================================================================== --- ps/trunk/source/renderer/Scene.cpp +++ ps/trunk/source/renderer/Scene.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2021 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -35,7 +35,7 @@ for (size_t i = 0; i < props.size(); i++) { if (!props[i].m_Hidden) - SubmitRecursive(props[i].m_Model); + SubmitRecursive(props[i].m_Model.get()); } } else if (model->ToCModelDecal()) Index: ps/trunk/source/tools/atlas/GameInterface/ActorViewer.cpp =================================================================== --- ps/trunk/source/tools/atlas/GameInterface/ActorViewer.cpp +++ ps/trunk/source/tools/atlas/GameInterface/ActorViewer.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -118,7 +118,8 @@ SOverlayLine SelectionBoxOverlay; SOverlayLine AxesMarkerOverlays[3]; - std::vector Props; + + std::vector PropModels; std::vector PropPointOverlays; // Simplistic implementation of the Scene interface @@ -159,16 +160,16 @@ } // add prop point overlays - if (PropPointsMode > 0 && Props.size() > 0) + if (PropPointsMode > 0 && PropModels.size() > 0) { PropPointOverlays.clear(); // doesn't clear capacity, but should be ok since the number of prop points is usually pretty limited - for (size_t i = 0; i < Props.size(); ++i) + for (size_t i = 0; i < PropModels.size(); ++i) { - CModel::Prop& prop = Props[i]; - if (prop.m_Model) // should always be the case + CModelAbstract* model = PropModels[i]; + if (model) // should always be the case { // prop point positions are automatically updated during animations etc. by CModel::ValidatePosition - const CMatrix3D& propCoordSystem = prop.m_Model->GetTransform(); + const CMatrix3D& propCoordSystem = model->GetTransform(); SOverlayLine pointGimbal; pointGimbal.m_Color = CColor(1.f, 0.f, 1.f, 1.f); @@ -231,7 +232,7 @@ void ActorViewerImpl::UpdatePropList() { - Props.clear(); + PropModels.clear(); CmpPtr cmpVisual(Simulation2, Entity); if (cmpVisual) @@ -255,9 +256,9 @@ std::vector& modelProps = model->GetProps(); for (CModel::Prop& modelProp : modelProps) { - Props.push_back(modelProp); + PropModels.push_back(modelProp.m_Model.get()); if (modelProp.m_Model) - UpdatePropListRecursive(modelProp.m_Model); + UpdatePropListRecursive(modelProp.m_Model.get()); } } }