Merge pull request #573 from ShFil119/raii_effects

Refactor VisualFX and fix memory leaks

rwengine/src/engine/GameWorld.cpp

@@ -497,15 +497,30 @@ void GameWorld::destroyQueuedObjects() {
     }
 }
 
-VisualFX* GameWorld::createEffect(VisualFX::EffectType type) {
+LightFX& GameWorld::createLightEffect() {
-    auto effect = new VisualFX(type);
+    auto effect = std::make_unique<LightFX>();
-    effects.push_back(effect);
+    auto& ref = *effect;
-    return effect;
+    effects.push_back(std::move(effect));
+    return ref;
 }
 
-void GameWorld::destroyEffect(VisualFX* effect) {
+ParticleFX& GameWorld::createParticleEffect() {
+    auto effect = std::make_unique<ParticleFX>();
+    auto& ref = *effect;
+    effects.push_back(std::move(effect));
+    return ref;
+}
+
+TrailFX& GameWorld::createTrailEffect() {
+    auto effect = std::make_unique<TrailFX>();
+    auto& ref = *effect;
+    effects.push_back(std::move(effect));
+    return ref;
+}
+
+void GameWorld::destroyEffect(VisualFX& effect) {
     for (auto it = effects.begin(); it != effects.end();) {
-        if (*it == effect) {
+        if (it->get() == &effect) {
             it = effects.erase(it);
         } else {
             it++;
@@ -833,6 +848,20 @@ bool GameWorld::isPaused() const {
     return paused;
 }
 
+void GameWorld::updateEffects() {
+    for (int i = 0; i < static_cast<int>(effects.size()); ++i) {
+        auto& effect = effects[i];
+        if (effect->getType() == Particle) {
+            auto particle = static_cast<ParticleFX*>(effect.get());
+            if (particle->lifetime < 0.f) continue;
+            if (getGameTime() >= particle->starttime + particle->lifetime) {
+                destroyEffect(*particle);
+                --i;
+            }
+        }
+    }
+}
+
 VehicleObject* GameWorld::tryToSpawnVehicle(VehicleGenerator& gen) {
     constexpr float kMinClearRadius = 10.f;
 

rwengine/src/engine/GameWorld.hpp

@@ -179,14 +179,24 @@ public:
     void doWeaponScan(const WeaponScan& scan);
 
     /**
-     * Allocates a new VisualFX of the given type
+     * Allocates a new Light Effect
      */
-    VisualFX* createEffect(VisualFX::EffectType type);
+    LightFX& createLightEffect();
+
+    /**
+     * Allocates a new Particle Effect
+     */
+    ParticleFX& createParticleEffect();
+
+    /**
+     * Allocates a new Trail Effect
+     */
+    TrailFX& createTrailEffect();
 
     /**
      * Immediately destoys the given effect
      */
-    void destroyEffect(VisualFX* effect);
+    void destroyEffect(VisualFX& effect);
 
     /**
      * Returns the current hour
@@ -296,7 +306,7 @@ public:
      * Visual Effects
      * @todo Consider using lighter handing mechanism
      */
-    std::vector<VisualFX*> effects;
+    std::vector<std::unique_ptr<VisualFX>> effects;
 
     /**
      * Randomness Engine
@@ -364,6 +374,11 @@ public:
     void setPaused(bool pause);
     bool isPaused() const;
 
+    /**
+     * Clean up old VisualFX
+     */
+    void updateEffects();
+
     /**
      * Attempt to spawn a vehicle at a vehicle generator
      */

rwengine/src/objects/PickupObject.cpp

@@ -92,6 +92,7 @@ PickupObject::BehaviourFlags PickupObject::defaultBehaviourFlags(
 PickupObject::PickupObject(GameWorld* world, const glm::vec3& position,
                            BaseModelInfo* modelinfo, PickupType type)
     : GameObject(world, position, glm::quat{1.0f, 0.0f, 0.0f, 0.0f}, modelinfo)
+    , m_corona(world->createParticleEffect())
     , m_type(type) {
     btTransform tf;
     tf.setIdentity();
@@ -134,19 +135,18 @@ PickupObject::PickupObject(GameWorld* world, const glm::vec3& position,
     else if (modelinfo->name == "health" || modelinfo->name == "bonus")
         m_colourId = 13;
 
-    m_corona = world->createEffect(VisualFX::Particle);
+    m_corona.position = getPosition();
-    m_corona->particle.position = getPosition();
+    m_corona.direction = glm::vec3(0.f, 0.f, 1.f);
-    m_corona->particle.direction = glm::vec3(0.f, 0.f, 1.f);
+    m_corona.orientation = ParticleFX::Camera;
-    m_corona->particle.orientation = VisualFX::ParticleData::Camera;
 
     // @todo float package should float on the water
     if (m_type == FloatingPackage) {
         // verify offset and texture?
-        m_corona->particle.position += glm::vec3(0.f, 0.f, 0.7f);
+        m_corona.position += glm::vec3(0.f, 0.f, 0.7f);
-        m_corona->particle.texture =
+        m_corona.texture =
             engine->data->findSlotTexture("particle", "coronastar");
     } else {
-        m_corona->particle.texture =
+        m_corona.texture =
             engine->data->findSlotTexture("particle", "coronaringa");
     }
 
@@ -162,7 +162,6 @@ PickupObject::~PickupObject() {
     if (m_ghost) {
         setEnabled(false);
         engine->destroyEffect(m_corona);
-        delete m_corona;
         delete m_ghost;
         delete m_shape;
     }
@@ -197,7 +196,7 @@ void PickupObject::tick(float dt) {
     float red = (*colour >> 16) & 0xFF;
     float green = (*colour >> 8) & 0xFF;
     float blue = *colour & 0xFF;
-    m_corona->particle.colour =
+    m_corona.colour =
         glm::vec4(red / 255.f, green / 255.f, blue / 255.f, 1.f) * colourValue;
 
     if (m_enabled) {
@@ -258,10 +257,10 @@ void PickupObject::setEnabled(bool enabled) {
     if (!m_enabled && enabled) {
         engine->dynamicsWorld->addCollisionObject(
             m_ghost, btBroadphaseProxy::SensorTrigger);
-        m_corona->particle.size = glm::vec2(1.5f, 1.5f);
+        m_corona.size = glm::vec2(1.5f, 1.5f);
     } else if (m_enabled && !enabled) {
         engine->dynamicsWorld->removeCollisionObject(m_ghost);
-        m_corona->particle.size = glm::vec2(0.f, 0.f);
+        m_corona.size = glm::vec2(0.f, 0.f);
     }
 
     m_enabled = enabled;

rwengine/src/objects/PickupObject.hpp

@@ -6,8 +6,10 @@
 
 #include <rw/debug.hpp>
 
+#include <render/VisualFX.hpp>
 #include <objects/GameObject.hpp>
 
+
 class btPairCachingGhostObject;
 class btSphereShape;
 
@@ -119,7 +121,7 @@ private:
     bool m_enabled = false;
     float m_enableTimer = 0.f;
     bool m_collected = false;
-    VisualFX* m_corona = nullptr;
+    ParticleFX& m_corona;
     short m_colourId = 0;
     bool respawn = false;
     float respawnTime{};

rwengine/src/objects/ProjectileObject.cpp

@@ -7,7 +7,6 @@
 #include "data/WeaponData.hpp"
 #include "engine/GameData.hpp"
 #include "engine/GameWorld.hpp"
-#include "render/VisualFX.hpp"
 
 void ProjectileObject::checkPhysicsContact() {
     btManifoldArray manifoldArray;
@@ -77,17 +76,17 @@ void ProjectileObject::explode() {
                            0.f});
         }
 
-        auto tex = engine->data->findSlotTexture("particle", "explo02");
+        auto& explosion = engine->createParticleEffect();
 
-        auto explosion = engine->createEffect(VisualFX::Particle);
+        auto tex = engine->data->findSlotTexture("particle", "explo02");
-        explosion->particle.size = glm::vec2(exp_size);
+        explosion.texture = tex;
-        explosion->particle.texture = tex;
+        explosion.size = glm::vec2(exp_size);
-        explosion->particle.starttime = engine->getGameTime();
+        explosion.starttime = engine->getGameTime();
-        explosion->particle.lifetime = 0.5f;
+        explosion.lifetime = 0.5f;
-        explosion->particle.orientation = VisualFX::ParticleData::Camera;
+        explosion.orientation = ParticleFX::Camera;
-        explosion->particle.colour = glm::vec4(1.0f);
+        explosion.colour = glm::vec4(1.0f);
-        explosion->particle.position = getPosition();
+        explosion.position = getPosition();
-        explosion->particle.direction = glm::vec3(0.f, 0.f, 1.f);
+        explosion.direction = glm::vec3(0.f, 0.f, 1.f);
 
         _exploded = true;
         engine->destroyObjectQueued(this);

rwengine/src/objects/ProjectileObject.hpp

@@ -4,6 +4,8 @@
 #include <glm/glm.hpp>
 
 #include <objects/GameObject.hpp>
+#include "render/VisualFX.hpp"
+
 
 class GameWorld;
 class btPairCachingGhostObject;

rwengine/src/render/GameRenderer.cpp

@@ -452,26 +452,25 @@ void GameRenderer::renderEffects(GameWorld* world) {
     auto& effects = world->effects;
 
     std::sort(effects.begin(), effects.end(),
-              [&](const VisualFX* a, const VisualFX* b) {
+              [&](const auto& a, const auto& b) {
-                  return glm::distance(a->getPosition(), cpos) >
+                  return glm::distance(a->position, cpos) >
-                         glm::distance(b->getPosition(), cpos);
+                         glm::distance(b->position, cpos);
               });
 
-    for (VisualFX* fx : effects) {
+    for (auto& fx : effects) {
         // Other effects not implemented yet
-        if (fx->getType() != VisualFX::Particle) continue;
+        if (fx->getType() != Particle) continue;
+        auto particle = static_cast<ParticleFX*>(fx.get());
 
-        auto& particle = fx->particle;
+        auto& p = particle->position;
-
-        auto& p = particle.position;
 
         // Figure the direction to the camera center.
         auto amp = cpos - p;
-        glm::vec3 ptc = particle.up;
+        glm::vec3 ptc = particle->up;
 
-        if (particle.orientation == VisualFX::ParticleData::UpCamera) {
+        if (particle->orientation == ParticleFX::UpCamera) {
             ptc = glm::normalize(amp - (glm::dot(amp, cfwd)) * cfwd);
-        } else if (particle.orientation == VisualFX::ParticleData::Camera) {
+        } else if (particle->orientation == ParticleFX::Camera) {
             ptc = amp;
         }
 
@@ -485,12 +484,12 @@ void GameRenderer::renderEffects(GameWorld* world) {
             glm::vec3(0.0f,0.0f,1.0f));
 
         transformMat = glm::scale(glm::translate(transformMat,p),
-            glm::vec3(particle.size,1.0f)) * glm::inverse(lookMat);
+            glm::vec3(particle->size,1.0f)) * glm::inverse(lookMat);
 
         Renderer::DrawParameters dp;
-        dp.textures = {particle.texture->getName()};
+        dp.textures = {particle->texture->getName()};
         dp.ambient = 1.f;
-        dp.colour = glm::u8vec4(particle.colour * 255.f);
+        dp.colour = glm::u8vec4(particle->colour * 255.f);
         dp.start = 0;
         dp.count = 4;
         dp.blendMode = BlendMode::BLEND_ADDITIVE;

rwengine/src/render/VisualFX.cpp

@@ -1,47 +1 @@
 #include "render/VisualFX.hpp"
-
-#include <new>
-
-VisualFX::LightData::~LightData() = default;
-
-VisualFX::ParticleData::~ParticleData() = default;
-
-VisualFX::TrailData::~TrailData() = default;
-
-VisualFX::VisualFX(VisualFX::EffectType type) : type(type) {
-    switch (type) {
-        case VisualFX::Light:
-            new (&light) LightData;
-            break;
-        case VisualFX::Particle:
-            new (&particle) ParticleData;
-            break;
-        case VisualFX::Trail:
-            new (&trail) TrailData;
-            break;
-    }
-}
-
-VisualFX::~VisualFX() {
-    switch (type) {
-        case VisualFX::Light:
-            light.~LightData();
-            break;
-        case VisualFX::Particle:
-            particle.~ParticleData();
-            break;
-        case VisualFX::Trail:
-            trail.~TrailData();
-            break;
-    }
-}
-
-const glm::vec3& VisualFX::getPosition() const {
-    static glm::vec3 errorRef{};
-    switch (type) {
-        case VisualFX::Particle:
-            return particle.position;
-        default:
-            return errorRef;
-    }
-}

rwengine/src/render/VisualFX.hpp

@@ -5,82 +5,76 @@
 
 #include <gl/TextureData.hpp>
 
+enum EffectType { Light, Particle, Trail };
+
 /**
  * Represents a scene effect: lighting, particles etc.
  */
-class VisualFX {
+struct VisualFX {
-public:
+    VisualFX() = default;
-    enum EffectType { Light, Particle, Trail };
+    virtual ~VisualFX() = default;
 
-    struct LightData {
+    virtual EffectType getType() const = 0;
-        ~LightData();
-    };
-    struct ParticleData {
-        /** Initial world position */
-        glm::vec3 position{};
 
-        /** Direction of particle */
+    /** Initial world position */
-        glm::vec3 direction{};
+    glm::vec3 position{};
+};
 
-        /** Particle orientation modes */
+struct LightFX final : public VisualFX {
-        enum Orientation {
+    LightFX() = default;
-            Free,    /** faces direction using up */
+    ~LightFX() = default;
-            Camera,  /** Faces towards the camera @todo implement */
-            UpCamera /** Face closes point in camera's look direction */
-        };
-        Orientation orientation;
 
-        /** Game time at particle instantiation */
+    EffectType getType() const override {
-        float starttime;
+        return Light;
-        /** Number of seconds particle should exist for, negative values =
-         * forever */
-        float lifetime;
-
-        /** Texture name */
-        TextureData::Handle texture;
-
-        /** Size of particle */
-        glm::vec2 size;
-
-        /** Up direction (only used in Free mode) */
-        glm::vec3 up;
-
-        /** Render tint colour */
-        glm::vec4 colour;
-
-        /** Constructs a particle */
-        ParticleData()
-            : orientation(Free)
-            , starttime(0.f)
-            , lifetime(-1.f)
-            , size(1.f, 1.f)
-            , up(0.f, 0.f, 1.f)
-            , colour(1.f, 1.f, 1.f, 1.f) {
-        }
-        ~ParticleData();
-    };
-    struct TrailData {
-        ~TrailData();
-    };
-
-    /// @todo stop abusing unions
-    union {
-        LightData light;
-        ParticleData particle;
-        TrailData trail;
-    };
-
-    VisualFX(EffectType type);
-    ~VisualFX();
-
-    EffectType getType() const {
-        return type;
     }
+};
 
-    const glm::vec3& getPosition() const;
+struct ParticleFX final : public VisualFX {
+    /** Direction of particle */
+    glm::vec3 direction{};
 
-private:
+    /** Particle orientation modes */
-    EffectType type;
+    enum Orientation {
+        Free,    /** faces direction using up */
+        Camera,  /** Faces towards the camera @todo implement */
+        UpCamera /** Face closes point in camera's look direction */
+    };
+    Orientation orientation{Free};
+
+    /** Game time at particle instantiation */
+    float starttime{0.f};
+    /** Number of seconds particle should exist for, negative values =
+     * forever */
+    float lifetime{-1.f};
+
+    /** Texture name */
+    TextureData::Handle texture;
+
+    /** Size of particle */
+    glm::vec2 size{1.f, 1.f};
+
+    /** Up direction (only used in Free mode) */
+    glm::vec3 up{0.f, 0.f, 1.f};
+
+    /** Render tint colour */
+    glm::vec4 colour{1.f, 1.f, 1.f, 1.f};
+
+    /** Constructs a particle */
+    ParticleFX() = default;
+    ~ParticleFX() = default;
+
+    EffectType getType() const override {
+        return Particle;
+    }
+};
+
+struct TrailFX final : public VisualFX {
+    TrailFX() = default;
+    ~TrailFX() = default;
+
+    EffectType getType() const override {
+        return Trail;
+    }
 };
 
 #endif

rwgame/RWGame.cpp

@@ -528,18 +528,7 @@ void RWGame::tick(float dt) {
             }
         }
 
-        // Clean up old VisualFX
+        world->updateEffects();
-        for (int i = 0; i < static_cast<int>(world->effects.size()); ++i) {
-            VisualFX* effect = world->effects[i];
-            if (effect->getType() == VisualFX::Particle) {
-                auto& part = effect->particle;
-                if (part.lifetime < 0.f) continue;
-                if (world->getGameTime() >= part.starttime + part.lifetime) {
-                    world->destroyEffect(effect);
-                    --i;
-                }
-            }
-        }
 
         for (auto& object : world->allObjects) {
             object->_updateLastTransform();
@@ -772,9 +761,9 @@ void RWGame::renderDebugPaths(float time) {
 
             btVector3 position1(pos1.x, pos1.y, pos1.z);
             btVector3 position2(pos2.x, pos2.y, pos2.z);
-		
+
             debug.drawLine(position1, position2, color);
-	}
+    }
     }
 
     debug.flush(&renderer);

tests/test_VisualFX.cpp

@@ -4,9 +4,9 @@
 BOOST_AUTO_TEST_SUITE(VisualFXTests)
 
 BOOST_AUTO_TEST_CASE(test_light_data) {
-    VisualFX fx(VisualFX::Light);
+    auto fx = std::make_unique<LightFX>();
 
-    BOOST_CHECK_EQUAL(fx.getType(), VisualFX::Light);
+    BOOST_CHECK_EQUAL(fx->getType(), Light);
 }
 
 BOOST_AUTO_TEST_SUITE_END()