chore: Update render component and delete unused code (#1429)

* Update a few components to use smart pointers for memory management * 'final' keyword added to classes * removed duplicate 'const' * removed unused code * Updated render component to store effects directly in a vector * Use move instead of copy * make pointers const * attribute tags * nitpicking * delete default effect constructor * Added a vector size check to the RemoveEffect() function * use empty() instead of size()
DarkflameUniverse · Jan 31, 2024 · b23981e · b23981e
1 parent d78b508
commit b23981e
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 125 deletions.
diff --git a/dGame/dComponents/CMakeLists.txt b/dGame/dComponents/CMakeLists.txt
@@ -27,7 +27,6 @@ set(DGAME_DCOMPONENTS_SOURCES
 	"PlayerForcedMovementComponent.cpp"
 	"PossessableComponent.cpp"
 	"PossessorComponent.cpp"
-	"PropertyComponent.cpp"
 	"PropertyEntranceComponent.cpp"
 	"PropertyManagementComponent.cpp"
 	"PropertyVendorComponent.cpp"

diff --git a/dGame/dComponents/PropertyComponent.cpp b/dGame/dComponents/PropertyComponent.cpp
diff --git a/dGame/dComponents/PropertyComponent.h b/dGame/dComponents/PropertyComponent.h
@@ -1,34 +1,22 @@
 /*
  * Darkflame Universe
- * Copyright 2018
+ * Copyright 2024
  */
 
 #ifndef PROPERTYCOMPONENT_H
 #define PROPERTYCOMPONENT_H
 
-#include "BitStream.h"
 #include "Entity.h"
 #include "Component.h"
 #include "eReplicaComponentType.h"
 
-struct PropertyState {
-	LWOOBJID ownerID;
-	LWOOBJID propertyID;
-	bool rented;
-};
-
 /**
  * This component is unused and has no functionality
  */
 class PropertyComponent final : public Component {
 public:
 	static constexpr eReplicaComponentType ComponentType = eReplicaComponentType::PROPERTY;
-	explicit PropertyComponent(Entity* parentEntity);
-	~PropertyComponent() override;
-	[[nodiscard]] PropertyState* GetPropertyState() const { return m_PropertyState; };
-private:
-	PropertyState* m_PropertyState;
-	std::string m_PropertyName;
+	explicit PropertyComponent(Entity* const parentEntity) noexcept : Component{ parentEntity } {}
 };
 
-#endif // PROPERTYCOMPONENT_H
+#endif // !PROPERTYCOMPONENT_H
diff --git a/dGame/dComponents/RenderComponent.cpp b/dGame/dComponents/RenderComponent.cpp
@@ -1,7 +1,9 @@
 #include "RenderComponent.h"
 
+#include <algorithm>
 #include <sstream>
 #include <string>
+#include <utility>
 #include <iomanip>
 
 #include "Entity.h"
@@ -14,8 +16,7 @@
 
 std::unordered_map<int32_t, float> RenderComponent::m_DurationCache{};
 
-RenderComponent::RenderComponent(Entity* parent, int32_t componentId): Component(parent) {
-	m_Effects = std::vector<Effect*>();
+RenderComponent::RenderComponent(Entity* const parentEntity, const int32_t componentId) : Component{ parentEntity } {
 	m_LastAnimationName = "";
 	if (componentId == -1) return;
 
@@ -42,100 +43,51 @@ RenderComponent::RenderComponent(Entity* parent, int32_t componentId): Component
 	result.finalize();
 }
 
-RenderComponent::~RenderComponent() {
-	for (Effect* eff : m_Effects) {
-		if (eff) {
-			delete eff;
-			eff = nullptr;
-		}
-	}
-
-	m_Effects.clear();
-}
-
 void RenderComponent::Serialize(RakNet::BitStream* outBitStream, bool bIsInitialUpdate) {
 	if (!bIsInitialUpdate) return;
 
 	outBitStream->Write<uint32_t>(m_Effects.size());
 
-	for (Effect* eff : m_Effects) {
-		// we still need to write 0 as the size for name if it is a nullptr
-		if (!eff) {
-			outBitStream->Write<uint8_t>(0);
-			continue;
-		}
-
-		outBitStream->Write<uint8_t>(eff->name.size());
+	for (auto& eff : m_Effects) {
+		outBitStream->Write<uint8_t>(eff.name.size());
 		// if there is no name, then we don't write anything else
-		if (eff->name.empty()) continue;
+		if (eff.name.empty()) continue;
 
-		for (const auto& value : eff->name) outBitStream->Write<uint8_t>(value);
+		for (const auto& value : eff.name) outBitStream->Write<uint8_t>(value);
 
-		outBitStream->Write(eff->effectID);
+		outBitStream->Write(eff.effectID);
 
-		outBitStream->Write<uint8_t>(eff->type.size());
-		for (const auto& value : eff->type) outBitStream->Write<uint16_t>(value);
+		outBitStream->Write<uint8_t>(eff.type.size());
+		for (const auto& value : eff.type) outBitStream->Write<uint16_t>(value);
 
-		outBitStream->Write<float_t>(eff->priority);
-		outBitStream->Write<int64_t>(eff->secondary);
+		outBitStream->Write<float_t>(eff.priority);
+		outBitStream->Write<int64_t>(eff.secondary);
 	}
 }
 
-Effect* RenderComponent::AddEffect(const int32_t effectId, const std::string& name, const std::u16string& type, const float priority) {
-	auto* eff = new Effect();
-
-	eff->effectID = effectId;
-	eff->name = name;
-	eff->type = type;
-	eff->priority = priority;
-	m_Effects.push_back(eff);
-
-	return eff;
+Effect& RenderComponent::AddEffect(const int32_t effectId, const std::string& name, const std::u16string& type, const float priority) {
+	return m_Effects.emplace_back(Effect{ effectId, name, type, priority });
 }
 
 void RenderComponent::RemoveEffect(const std::string& name) {
-	uint32_t index = -1;
-
-	for (auto i = 0u; i < m_Effects.size(); ++i) {
-		auto* eff = m_Effects[i];
-
-		if (eff->name == name) {
-			index = i;
+	if (m_Effects.empty()) return;
 
-			delete eff;
-
-			break;
-		}
-	}
+	const auto effectToRemove = std::ranges::find_if(m_Effects, [&name](auto&& effect) { return effect.name == name; });
+	if (effectToRemove == m_Effects.end()) return; // Return early if effect is not present
 
-	if (index == -1) {
-		return;
-	}
-
-	m_Effects.erase(m_Effects.begin() + index);
+	const auto lastEffect = m_Effects.rbegin();
+	*effectToRemove = std::move(*lastEffect); // Move-overwrite
+	m_Effects.pop_back();
 }
 
-void RenderComponent::Update(const float deltaTime) {
-	std::vector<Effect*> dead;
+void RenderComponent::Update(const float deltaTime) {	
+	for (auto& effect : m_Effects) {
+		if (effect.time == 0) continue; // Skip persistent effects
 
-	for (auto* effect : m_Effects) {
-		if (effect->time == 0) {
-			continue; // Skip persistent effects
-		}
+		const auto result = effect.time - deltaTime;
+		if (result <= 0) continue;
 
-		const auto result = effect->time - deltaTime;
-
-		if (result <= 0) {
-			dead.push_back(effect);
-
-			continue;
-		}
-
-		effect->time = result;
-	}
-
-	for (auto* effect : dead) {
-		//        StopEffect(effect->name);
+		effect.time = result;
 	}
 }
 
@@ -144,12 +96,12 @@ void RenderComponent::PlayEffect(const int32_t effectId, const std::u16string& e
 
 	GameMessages::SendPlayFXEffect(m_Parent, effectId, effectType, name, secondary, priority, scale, serialize);
 
-	auto* effect = AddEffect(effectId, name, effectType, priority);
+	auto& effect = AddEffect(effectId, name, effectType, priority);
 
 	const auto& pair = m_DurationCache.find(effectId);
 
 	if (pair != m_DurationCache.end()) {
-		effect->time = pair->second;
+		effect.time = pair->second;
 
 		return;
 	}
@@ -168,16 +120,16 @@ void RenderComponent::PlayEffect(const int32_t effectId, const std::u16string& e
 
 		m_DurationCache[effectId] = 0;
 
-		effect->time = 0; // Persistent effect
+		effect.time = 0; // Persistent effect
 
 		return;
 	}
 
-	effect->time = static_cast<float>(result.getFloatField(0));
+	effect.time = static_cast<float>(result.getFloatField(0));
 
 	result.finalize();
 
-	m_DurationCache[effectId] = effect->time;
+	m_DurationCache[effectId] = effect.time;
 }
 
 void RenderComponent::StopEffect(const std::string& name, const bool killImmediate) {
@@ -186,11 +138,6 @@ void RenderComponent::StopEffect(const std::string& name, const bool killImmedia
 	RemoveEffect(name);
 }
 
-std::vector<Effect*>& RenderComponent::GetEffects() {
-	return m_Effects;
-}
-
-
 float RenderComponent::PlayAnimation(Entity* self, const std::u16string& animation, float priority, float scale) {
 	if (!self) return 0.0f;
 	return RenderComponent::PlayAnimation(self, GeneralUtils::UTF16ToWTF8(animation), priority, scale);

diff --git a/dGame/dComponents/RenderComponent.h b/dGame/dComponents/RenderComponent.h
@@ -17,7 +17,12 @@ class Entity;
  * here.
  */
 struct Effect {
-	Effect() { priority = 1.0f; }
+	explicit Effect(const int32_t effectID, const std::string& name, const std::u16string& type, const float priority = 1.0f) noexcept
+		: effectID{ effectID }
+		, name{ name }
+		, type{ type }
+		, priority{ priority } {
+	}
 
 	/**
 	 * The ID of the effect
@@ -58,8 +63,7 @@ class RenderComponent final : public Component {
 public:
 	static constexpr eReplicaComponentType ComponentType = eReplicaComponentType::RENDER;
 
-	RenderComponent(Entity* entity, int32_t componentId = -1);
-	~RenderComponent() override;
+	RenderComponent(Entity* const parentEntity, const int32_t componentId = -1);
 
 	void Serialize(RakNet::BitStream* outBitStream, bool bIsInitialUpdate) override;
 	void Update(float deltaTime) override;
@@ -72,7 +76,7 @@ class RenderComponent final : public Component {
 	 * @param priority the priority of the effect
 	 * @return if successful, the effect that was created
 	 */
-	Effect* AddEffect(int32_t effectId, const std::string& name, const std::u16string& type, const float priority);
+	[[maybe_unused]] Effect& AddEffect(const int32_t effectId, const std::string& name, const std::u16string& type, const float priority);
 
 	/**
 	 * Removes an effect for this entity
@@ -99,12 +103,6 @@ class RenderComponent final : public Component {
 	 */
 	void StopEffect(const std::string& name, bool killImmediate = true);
 
-	/**
-	 * Returns the list of currently active effects
-	 * @return
-	 */
-	std::vector<Effect*>& GetEffects();
-
 	/**
 	 * Verifies that an animation can be played on this entity by checking
 	 * if it has the animation assigned to its group.  If it does, the animation is echo'd
@@ -125,18 +123,18 @@ class RenderComponent final : public Component {
 
 	static float PlayAnimation(Entity* self, const std::u16string& animation, float priority = 0.0f, float scale = 1.0f);
 	static float PlayAnimation(Entity* self, const std::string& animation, float priority = 0.0f, float scale = 1.0f);
-	static float GetAnimationTime(Entity* self, const std::string& animation);
-	static float GetAnimationTime(Entity* self, const std::u16string& animation);
+	[[nodiscard]] static float GetAnimationTime(Entity* self, const std::string& animation);
+	[[nodiscard]] static float GetAnimationTime(Entity* self, const std::u16string& animation);
 
-	const std::string& GetLastAnimationName() const { return m_LastAnimationName; };
+	[[nodiscard]] const std::string& GetLastAnimationName() const { return m_LastAnimationName; };
 	void SetLastAnimationName(const std::string& name) { m_LastAnimationName = name; };
 
 private:
 
 	/**
 	 * List of currently active effects
 	 */
-	std::vector<Effect*> m_Effects;
+	std::vector<Effect> m_Effects;
 
 	std::vector<int32_t> m_animationGroupIds;