Merge pull request #544 from Wargus/clean_up

Clean up

src/ai/script_ai.cpp

@@ -207,8 +207,6 @@ static std::vector<CUnitType *> getRefineryUnits()
 */
 static void InitAiHelper(AiHelper &aiHelper)
 {
-	extern std::vector<ButtonAction *> UnitButtonTable;
-
 	std::vector<CUnitType *> reparableUnits = getReparableUnits();
 	std::vector<CUnitType *> supplyUnits = getSupplyUnits();
 	std::vector<CUnitType *> mineUnits = getRefineryUnits();
@@ -232,7 +230,7 @@ static void InitAiHelper(AiHelper &aiHelper)
 		}
 	}
 
-	for (const ButtonAction *button : UnitButtonTable) {
+	for (const auto &button : UnitButtonTable) {
 		const std::vector<CUnitType *> &unitmask = getUnitTypeFromString(button->UnitMask);
 
 		switch (button->Action) {

src/game/replay.cpp

@@ -120,7 +120,7 @@ class FullReplay
 //----------------------------------------------------------------------------
 
 bool CommandLogDisabled;           /// True if command log is off
-ReplayType ReplayGameType;         /// Replay game type
+EReplayType ReplayGameType;        /// Replay game type
 static bool DisabledLog;           /// Disabled log for replay
 static std::unique_ptr<CFile> LogFile; /// Replay log file
 static fs::path LastLogFileName;   /// Last log file name
@@ -200,9 +200,9 @@ static void ApplyReplaySettings()
 		ExitNetwork1();
 		NetPlayers = 2;
 		NetLocalPlayerNumber = CurrentReplay->LocalPlayer;
-		ReplayGameType = ReplayMultiPlayer;
+		ReplayGameType = EReplayType::MultiPlayer;
 	} else {
-		ReplayGameType = ReplaySinglePlayer;
+		ReplayGameType = EReplayType::SinglePlayer;
 	}
 	GameSettings = CurrentReplay->ReplaySettings;
 
@@ -570,7 +570,7 @@ static int CclReplayLog(lua_State *l)
 */
 bool IsReplayGame()
 {
-	return ReplayGameType != ReplayNone;
+	return ReplayGameType != EReplayType::None;
 }
 
 /**
@@ -591,7 +591,7 @@ void SaveReplayList(CFile &file)
 static void LoadReplay(const fs::path &name)
 {
 	CleanReplayLog();
-	ReplayGameType = ReplaySinglePlayer;
+	ReplayGameType = EReplayType::SinglePlayer;
 	LuaLoadFile(name);
 
 	NextLogCycle = ~0UL;
@@ -632,7 +632,7 @@ void CleanReplayLog()
 	// }
 	GameObserve = false;
 	NetPlayers = 0;
-	ReplayGameType = ReplayNone;
+	ReplayGameType = EReplayType::None;
 }
 
 /**
@@ -814,7 +814,7 @@ static void ReplayEachCycle()
 */
 void SinglePlayerReplayEachCycle()
 {
-	if (ReplayGameType == ReplaySinglePlayer) {
+	if (ReplayGameType == EReplayType::SinglePlayer) {
 		ReplayEachCycle();
 	}
 }
@@ -824,7 +824,7 @@ void SinglePlayerReplayEachCycle()
 */
 void MultiPlayerReplayEachCycle()
 {
-	if (ReplayGameType == ReplayMultiPlayer) {
+	if (ReplayGameType == EReplayType::MultiPlayer) {
 		ReplayEachCycle();
 	}
 }

src/include/font.h

@@ -98,7 +98,6 @@ class CFont : public gcn::Font
 
 	void Load();
 	void Reload() const;
-	void Clean();
 
 	CGraphic *GetFontColorGraphic(const CFontColor &fontColor) const;
 

src/include/interface.h

@@ -201,7 +201,7 @@ constexpr unsigned int ScrollRightDown = ScrollRight | ScrollLeft; /// scroll ri
 /*----------------------------------------------------------------------------
 --  Variables
 ----------------------------------------------------------------------------*/
-extern std::vector<ButtonAction *> UnitButtonTable;
+extern std::vector<std::unique_ptr<ButtonAction>> UnitButtonTable;
 
 /// Flag telling if the game is running
 extern bool GameRunning;

src/include/iolib.h

@@ -121,7 +121,7 @@ class CFile
 	int printf(const char *format, ...) PRINTF_VAARG_ATTRIBUTE(2, 3); // Don't forget to count this
 private:
 	class PImpl;
-	PImpl *pimpl;
+	std::unique_ptr<PImpl> pimpl;
 };
 
 #define CL_OPEN_READ 0x1

src/include/replay.h

@@ -41,11 +41,11 @@
 --  Declarations
 ----------------------------------------------------------------------------*/
 
-enum ReplayType {
-	ReplayNone,          /// No replay
-	ReplaySinglePlayer,  /// Single player replay
-	ReplayMultiPlayer    /// Multi player replay
-};                       /// Replay types
+enum class EReplayType {
+	None,          /// No replay
+	SinglePlayer,  /// Single player replay
+	MultiPlayer    /// Multi player replay
+};                 /// Replay types
 
 class CFile;
 class CUnit;
@@ -55,7 +55,7 @@ class CUnit;
 ----------------------------------------------------------------------------*/
 
 extern bool CommandLogDisabled;    /// True, if command log is off
-extern ReplayType ReplayGameType;  /// Replay game type
+extern EReplayType ReplayGameType;  /// Replay game type
 
 /*----------------------------------------------------------------------------
 --  Functions

src/include/spells.h

@@ -94,23 +94,6 @@ enum class ECondition
 	ShouldBeTrue
 };
 
-/*
-** *******************
-** Target definition.
-** *******************
-*/
-
-class Target
-{
-public:
-	Target(TargetType type, CUnit *unit, const Vec2i &pos) :
-		Type(type), Unit(unit), targetPos(pos) {}
-
-	TargetType Type;                  /// type of target.
-	CUnit *Unit;                      /// Unit target.
-	Vec2i targetPos;
-};
-
 /*
 ** *******************
 ** Conditions definition.

src/include/ui.h

@@ -78,11 +78,11 @@ enum class ECondition;
 --  Definitions
 ----------------------------------------------------------------------------*/
 
-enum TextAlignment {
-	TextAlignUndefined,
-	TextAlignCenter,
-	TextAlignLeft,
-	TextAlignRight
+enum class ETextAlignment {
+	Undefined,
+	Center,
+	Left,
+	Right
 };
 
 class ButtonStyleProperties
@@ -95,7 +95,7 @@ class ButtonStyleProperties
 	CColor BorderColorRGB;
 	IntColor BorderColor = 0;
 	int BorderSize = 0;
-	TextAlignment TextAlign = TextAlignUndefined; /// Text alignment
+	ETextAlignment TextAlign = ETextAlignment::Undefined; /// Text alignment
 	PixelPos TextPos{0, 0};         /// Text location
 	std::string TextNormalColor;    /// Normal text color
 	std::string TextReverseColor;   /// Reverse text color
@@ -111,7 +111,7 @@ class ButtonStyle
 	CFont *Font = nullptr;          /// Font
 	std::string TextNormalColor;    /// Normal text color
 	std::string TextReverseColor;   /// Reverse text color
-	TextAlignment TextAlign = TextAlignUndefined; /// Text alignment
+	ETextAlignment TextAlign = ETextAlignment::Undefined; /// Text alignment
 	int TextX = 0;                  /// Text X location
 	int TextY = 0;                  /// Text Y location
 	ButtonStyleProperties Default;  /// Default button properties

src/include/unit.h

@@ -37,6 +37,7 @@
 ----------------------------------------------------------------------------*/
 
 #include "actions.h"
+#include "pathfinder.h"
 #include "settings.h"
 
 #ifndef __UNITTYPE_H__
@@ -137,7 +138,7 @@ class CUnit
 {
 public:
 	CUnit() { Init(); }
-	~CUnit();
+	~CUnit() = default;
 
 	void Init();
 
@@ -345,7 +346,7 @@ class CUnit
 	int         CurrentSightRange; /// Unit's Current Sight Range
 
 	// Pathfinding stuff:
-	PathFinderData *pathFinderData = nullptr;
+	std::unique_ptr<PathFinderData> pathFinderData;
 
 	// DISPLAY:
 	int Frame;             /// Image frame: <0 is mirrored

src/include/unittype.h

@@ -824,7 +824,7 @@ class CUnitTypeVar
 
 	//EventType *Event;                  /// Array of functions sets to call when en event occurs.
 	std::vector<CVariable> Variable;   /// Array of user defined variables (default value for unittype).
-	std::vector<CDecoVar *> DecoVar;   /// Array to describe how showing variable.
+	std::vector<std::unique_ptr<CDecoVar>> DecoVar;   /// Array to describe how showing variable.
 };
 
 extern CUnitTypeVar UnitTypeVar;

src/include/upgrade_structs.h

@@ -37,6 +37,7 @@
 --  Includes
 ----------------------------------------------------------------------------*/
 
+#include <memory>
 #include <vector>
 
 /*----------------------------------------------------------------------------
@@ -226,7 +227,7 @@ class CUpgradeTimers
 --  Variables
 ----------------------------------------------------------------------------*/
 
-extern std::vector<CUpgrade *> AllUpgrades;  /// the main user usable upgrades
+extern std::vector<std::unique_ptr<CUpgrade>> AllUpgrades;  /// the main user usable upgrades
 
 //@}
 

src/spell/spells.cpp

@@ -68,22 +68,6 @@ std::vector<std::unique_ptr<SpellType>> SpellTypeTable;
 -- Functions
 ----------------------------------------------------------------------------*/
 
-// ****************************************************************************
-// Target constructor
-// ****************************************************************************
-
-/**
-**  Target constructor for unit.
-**
-**  @param unit  Target unit.
-**
-**  @return the new target.
-*/
-static std::unique_ptr<Target> NewTargetUnit(CUnit &unit)
-{
-	return std::make_unique<Target>(TargetUnit, &unit, unit.tilePos);
-}
-
 // ****************************************************************************
 // Main local functions
 // ****************************************************************************
@@ -238,11 +222,11 @@ class AutoCastPrioritySort
 **  @param caster    Unit who would cast the spell.
 **  @param spell     Spell-type pointer.
 **
-**  @return          Target* chosen target or Null if spell can't be cast.
+**  @return          chosen target or nullopt if spell can't be casted.
 **  @todo FIXME: should be global (for AI) ???
 **  @todo FIXME: write for position target.
 */
-static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const SpellType &spell)
+static std::optional<std::pair<CUnit*, Vec2i>> SelectTargetUnitsOfAutoCast(CUnit &caster, const SpellType &spell)
 {
 	AutoCastInfo *autocast;
 
@@ -276,17 +260,17 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 							})
 			!= table.end();
 		if ((autocast->Combat == ECondition::ShouldBeTrue) ^ (inCombat)) {
-			return nullptr;
+			return std::nullopt;
 		}
 	}
 
 	switch (spell.Target) {
 		case TargetSelf :
 			if (PassCondition(caster, spell, &caster, pos, spell.Condition.get())
 				&& PassCondition(caster, spell, &caster, pos, autocast->Condition.get())) {
-				return NewTargetUnit(caster);
+				return std::pair{&caster, caster.tilePos};
 			}
-			return nullptr;
+			return std::nullopt;
 		case TargetPosition: {
 			if (autocast->PositionAutoCast && table.empty() == false) {
 				size_t count = 0;
@@ -319,11 +303,11 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 					const auto [x, y] = autocast->PositionAutoCast->call<int, int>(array);
 					Vec2i resPos(x, y);
 					if (Map.Info.IsPointOnMap(resPos)) {
-						return std::make_unique<Target>(TargetPosition, nullptr, resPos);
+						return std::pair{nullptr, resPos};
 					}
 				}
 			}
-			return nullptr;
+			return std::nullopt;
 		}
 		case TargetUnit: {
 			// The units are already selected.
@@ -360,23 +344,25 @@ static std::unique_ptr<Target> SelectTargetUnitsOfAutoCast(CUnit &caster, const
 			// Now select the best unit to target.
 			if (n != 0) {
 				// For the best target???
+				CUnit *unit;
 				if (autocast->PriorytyVar != ACP_NOVALUE) {
 					std::sort(table.begin(), table.begin() + n,
 							  AutoCastPrioritySort(caster, autocast->PriorytyVar, autocast->ReverseSort));
-					return NewTargetUnit(*table[0]);
+					unit = table[0];
 				} else { // Use the old behavior
-					return NewTargetUnit(*table[SyncRand() % n]);
+					unit = table[SyncRand() % n];
 				}
+				return std::pair{unit, unit->tilePos};
 			}
 			break;
 		}
 		default:
 			// Something is wrong
 			DebugPrint("Spell is screwed up, unknown target type\n");
 			Assert(0);
-			return nullptr;
+			return std::nullopt;
 	}
-	return nullptr; // Can't spell the auto-cast.
+	return std::nullopt; // Can't spell the auto-cast.
 }
 
 // ****************************************************************************
@@ -466,16 +452,17 @@ bool AutoCastSpell(CUnit &caster, const SpellType &spell)
 		return false;
 	}
 	auto target = SelectTargetUnitsOfAutoCast(caster, spell);
-	if (target == nullptr) {
+	if (target == std::nullopt) {
 		return false;
 	} else {
 		// Save previous order
 		std::unique_ptr<COrder> savedOrder;
 		if (caster.CurrentAction() != UnitAction::Still && caster.CanStoreOrder(caster.CurrentOrder())) {
 			savedOrder = caster.CurrentOrder()->Clone();
 		}
+		auto [targetUnit, targetPos] = *target;
 		// Must move before?
-		CommandSpellCast(caster, target->targetPos, target->Unit, spell, FlushCommands, true);
+		CommandSpellCast(caster, targetPos, targetUnit, spell, FlushCommands, true);
 		if (savedOrder != nullptr) {
 			caster.SavedOrder = std::move(savedOrder);
 		}

src/stratagus/iolib.cpp

@@ -93,15 +93,11 @@ class CFile::PImpl
 #endif // !USE_BZ2LIB
 };
 
-CFile::CFile() : pimpl(new CFile::PImpl)
+CFile::CFile() : pimpl(std::make_unique<CFile::PImpl>())
 {
 }
 
-CFile::~CFile()
-{
-	delete pimpl;
-}
-
+CFile::~CFile() = default;
 
 /**
 **  CLopen Library file open
@@ -220,10 +216,9 @@ static size_t sdl_write(SDL_RWops *context, const void *ptr, size_t size, size_t
 
 static int sdl_close(SDL_RWops *context)
 {
-	CFile *self = reinterpret_cast<CFile*>(context->hidden.unknown.data1);
+	std::unique_ptr<CFile> self{reinterpret_cast<CFile *>(context->hidden.unknown.data1)};
 	const int res = self->close();
 	SDL_FreeRW(context);
-	delete self;
 	return res;
 }