Cleanups

Source/.clang-tidy

@@ -49,7 +49,8 @@ Checks: >
   -modernize-avoid-c-arrays,
   -modernize-use-trailing-return-type,
   -modernize-concat-nested-namespaces,
-  -modernize-avoid-bind
+  -modernize-avoid-bind,
+  -readability-magic-numbers
 
 HeaderFilterRegex: "^(Source|test)\\.h$"
 

Source/appfat.cpp

@@ -11,8 +11,8 @@
 #include "diablo.h"
 #include "storm/storm.h"
 #include "utils/language.h"
-#include "utils/ui_fwd.h"
 #include "utils/sdl_thread.h"
+#include "utils/ui_fwd.h"
 
 namespace devilution {
 

Source/control.cpp

@@ -90,7 +90,6 @@ std::optional<CelSprite> pSBkIconCels;
 std::optional<CelSprite> pSpellBkCel;
 std::optional<CelSprite> pSpellCels;
 
-
 bool PanelButtons[8];
 int PanelButtonIndex;
 char TalkSave[8][80];
@@ -1296,7 +1295,7 @@ void DrawInfoBox(const Surface &out)
 					PrintMonstHistory(monster.MType->mtype);
 				}
 			} else if (pcursitem == -1) {
-				string_view townerName = Towners[pcursmonst]._tName;
+				string_view townerName = Towners[pcursmonst].name;
 				strncpy(infostr, townerName.data(), townerName.length());
 				infostr[townerName.length()] = '\0';
 			}

Source/controls/keymapper.cpp

@@ -11,8 +11,8 @@
 #endif
 
 #include "control.h"
-#include "utils/log.hpp"
 #include "options.h"
+#include "utils/log.hpp"
 
 namespace devilution {
 

Source/diablo.h

@@ -53,7 +53,6 @@ enum class MouseActionType : int {
 	AttackMonsterTarget,
 	AttackPlayerTarget,
 	OperateObject,
-	Other,
 };
 
 extern SDL_Window *ghMainWnd;

Source/dx.cpp

@@ -11,8 +11,8 @@
 #include "options.h"
 #include "storm/storm.h"
 #include "utils/display.h"
-#include "utils/sdl_mutex.h"
 #include "utils/log.hpp"
+#include "utils/sdl_mutex.h"
 
 #ifdef __3DS__
 #include <3ds.h>

Source/engine/animationinfo.cpp

@@ -7,8 +7,8 @@
 #include "animationinfo.h"
 #include "appfat.h"
 #include "nthread.h"
-#include "utils/stdcompat/algorithm.hpp"
 #include "utils/log.hpp"
+#include "utils/stdcompat/algorithm.hpp"
 
 namespace devilution {
 

Source/engine/point.hpp

@@ -153,7 +153,7 @@ struct Point {
 		return std::max<int>(offset.deltaX, offset.deltaY);
 	}
 
-	#ifdef RUN_TESTS
+#ifdef RUN_TESTS
 	/**
 	 * @brief Format points nicely in test failure messages
 	 * @param stream output stream, expected to have overloads for int and char*
@@ -164,7 +164,7 @@ struct Point {
 	{
 		return stream << "(x: " << point.x << ", y: " << point.y << ")";
 	}
-	#endif
+#endif
 };
 
 } // namespace devilution

Source/items.cpp

@@ -2921,7 +2921,7 @@ void CalcPlrItemVals(int playerId, bool loadgfx)
 	case ITYPE_STAFF:
 		animWeaponId = PlayerWeaponGraphic::Staff;
 		break;
-    default:
+	default:
 		break;
 	}
 

Source/menu.cpp

@@ -76,7 +76,7 @@ void PlayIntro()
 	RefreshMusic();
 }
 
-bool Dummy_GetHeroInfo(_uiheroinfo *pInfo)
+bool DummyGetHeroInfo(_uiheroinfo * /*pInfo*/)
 {
 	return true;
 }
@@ -87,7 +87,7 @@ bool mainmenu_select_hero_dialog(GameData *gameData)
 {
 	_selhero_selections dlgresult = SELHERO_NEW_DUNGEON;
 	if (demoMode) {
-		pfile_ui_set_hero_infos(Dummy_GetHeroInfo);
+		pfile_ui_set_hero_infos(DummyGetHeroInfo);
 		gbLoadGame = true;
 	} else if (!gbIsMultiplayer) {
 		UiSelHeroSingDialog(

Source/miniwin/misc_msg.cpp

@@ -1,12 +1,11 @@
 #include <SDL.h>
 #include <cstdint>
 #include <deque>
-#include <string>
 #include <sstream>
+#include <string>
 
 #include "control.h"
 #include "controls/controller.h"
-#include "utils/paths.h"
 #include "controls/controller_motion.h"
 #include "controls/game_controls.h"
 #include "controls/plrctrls.h"
@@ -16,15 +15,16 @@
 #include "engine/rectangle.hpp"
 #include "hwcursor.hpp"
 #include "inv.h"
+#include "menu.h"
 #include "miniwin/miniwin.h"
 #include "movie.h"
+#include "nthread.h"
+#include "storm/storm.h"
 #include "utils/display.h"
 #include "utils/log.hpp"
+#include "utils/paths.h"
 #include "utils/sdl_compat.h"
 #include "utils/stubs.h"
-#include "menu.h"
-#include "nthread.h"
-#include "storm/storm.h"
 
 #ifdef __SWITCH__
 #include "platform/switch/docking.h"
@@ -631,11 +631,11 @@ bool LoadDemoMessages(int i)
 		return false;
 	}
 
-	std::string line, number;
-
+	std::string line;
 	std::getline(demofile, line);
 	std::stringstream header(line);
 
+	std::string number;
 	std::getline(header, number, ','); // Demo version
 	if (std::stoi(number) != 0) {
 		return false;
@@ -657,7 +657,7 @@ bool LoadDemoMessages(int i)
 
 		std::getline(command, number, ',');
 		int typeNum = std::stoi(number);
-		DemoMsgType type = static_cast<DemoMsgType>(typeNum);
+		auto type = static_cast<DemoMsgType>(typeNum);
 
 		std::getline(command, number, ',');
 		float progressToNextGameTick = std::stof(number);
@@ -689,7 +689,7 @@ bool LoadDemoMessages(int i)
 bool DemoMessage(tagMSG *lpMsg)
 {
 	SDL_Event e;
-	if (SDL_PollEvent(&e)) {
+	if (SDL_PollEvent(&e) != 0) {
 		if (e.type == SDL_QUIT) {
 			lpMsg->message = DVL_WM_QUIT;
 			lpMsg->lParam = 0;

Source/scrollrt.cpp

@@ -1352,7 +1352,6 @@ Displacement GetOffsetForWalking(const AnimationInfo &animationInfo, const Direc
 	//                                           DIR_S,        DIR_SW,       DIR_W,	       DIR_NW,        DIR_N,        DIR_NE,        DIR_E,        DIR_SE,
 	constexpr Displacement StartOffset[8]    = { {   0, -32 }, {  32, -16 }, {  32, -16 }, {   0,   0 }, {   0,   0 }, {  0,    0 },  { -32, -16 }, { -32, -16 } };
 	constexpr Displacement MovingOffset[8]   = { {   0,  32 }, { -32,  16 }, { -64,   0 }, { -32, -16 }, {   0, -32 }, {  32, -16 },  {  64,   0 }, {  32,  16 } };
-	constexpr bool IsDiagionalWalk[8]        = {        false,         true,        false,         true,        false,         true,         false,         true };
 	// clang-format on
 
 	float fAnimationProgress = animationInfo.GetAnimationProgress();

Source/stores.cpp

@@ -2073,7 +2073,7 @@ void TalkEnter()
 	}
 
 	if (stextsel == sn - 2) {
-		SetRndSeed(Towners[talker]._tSeed);
+		SetRndSeed(Towners[talker].seed);
 		auto tq = static_cast<_speech_id>(gossipstart + GenerateRnd(gossipend - gossipstart + 1));
 		InitQTextMsg(tq);
 		return;

Source/towners.cpp

@@ -50,7 +50,7 @@ void InitTownerInfo(int i, const TownerInit &initData)
 	towner._ttype = initData.type;
 	towner.position = initData.position;
 	towner.talk = initData.talk;
-	towner._tSeed = AdvanceRndSeed(); // TODO: Narrowing conversion, tSeed might need to be uint16_t
+	towner.seed = AdvanceRndSeed(); // TODO: Narrowing conversion, tSeed might need to be uint16_t
 
 	dMonster[towner.position.x][towner.position.y] = i + 1;
 
@@ -59,9 +59,9 @@ void InitTownerInfo(int i, const TownerInit &initData)
 
 void LoadTownerAnimations(TownerStruct &towner, const char *path, int frames, Direction dir, int delay)
 {
-	towner._tNData = LoadFileInMem(path);
+	towner.data = LoadFileInMem(path);
 	for (auto &animation : towner._tNAnim) {
-		animation = towner._tNData.get();
+		animation = towner.data.get();
 	}
 	NewTownerAnim(towner, towner._tNAnim[dir], frames, delay);
 }
@@ -86,7 +86,7 @@ void InitSmith(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = AnimOrder;
 	towner.animOrderSize = sizeof(AnimOrder);
 	LoadTownerAnimations(towner, "Towners\\Smith\\SmithN.CEL", 16, initData.dir, 3);
-	towner._tName = _("Griswold the Blacksmith");
+	towner.name = _("Griswold the Blacksmith");
 }
 
 void InitBarOwner(TownerStruct &towner, const TownerInit &initData)
@@ -108,7 +108,7 @@ void InitBarOwner(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = AnimOrder;
 	towner.animOrderSize = sizeof(AnimOrder);
 	LoadTownerAnimations(towner, "Towners\\TwnF\\TwnFN.CEL", 16, initData.dir, 3);
-	towner._tName = _("Ogden the Tavern owner");
+	towner.name = _("Ogden the Tavern owner");
 }
 
 void InitTownDead(TownerStruct &towner, const TownerInit &initData)
@@ -117,7 +117,7 @@ void InitTownDead(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = nullptr;
 	towner.animOrderSize = 0;
 	LoadTownerAnimations(towner, "Towners\\Butch\\Deadguy.CEL", 8, initData.dir, 6);
-	towner._tName = _("Wounded Townsman");
+	towner.name = _("Wounded Townsman");
 }
 
 void InitWitch(TownerStruct &towner, const TownerInit &initData)
@@ -139,7 +139,7 @@ void InitWitch(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = AnimOrder;
 	towner.animOrderSize = sizeof(AnimOrder);
 	LoadTownerAnimations(towner, "Towners\\TownWmn1\\Witch.CEL", 19, initData.dir, 6);
-	towner._tName = _("Adria the Witch");
+	towner.name = _("Adria the Witch");
 }
 
 void InitBarmaid(TownerStruct &towner, const TownerInit &initData)
@@ -148,7 +148,7 @@ void InitBarmaid(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = nullptr;
 	towner.animOrderSize = 0;
 	LoadTownerAnimations(towner, "Towners\\TownWmn1\\WmnN.CEL", 18, initData.dir, 6);
-	towner._tName = _("Gillian the Barmaid");
+	towner.name = _("Gillian the Barmaid");
 }
 
 void InitBoy(TownerStruct &towner, const TownerInit &initData)
@@ -157,7 +157,7 @@ void InitBoy(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = nullptr;
 	towner.animOrderSize = 0;
 	LoadTownerAnimations(towner, "Towners\\TownBoy\\PegKid1.CEL", 20, initData.dir, 6);
-	towner._tName = _("Wirt the Peg-legged boy");
+	towner.name = _("Wirt the Peg-legged boy");
 }
 
 void InitHealer(TownerStruct &towner, const TownerInit &initData)
@@ -179,7 +179,7 @@ void InitHealer(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = AnimOrder;
 	towner.animOrderSize = sizeof(AnimOrder);
 	LoadTownerAnimations(towner, "Towners\\Healer\\Healer.CEL", 20, initData.dir, 6);
-	towner._tName = _("Pepin the Healer");
+	towner.name = _("Pepin the Healer");
 }
 
 void InitTeller(TownerStruct &towner, const TownerInit &initData)
@@ -196,7 +196,7 @@ void InitTeller(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = AnimOrder;
 	towner.animOrderSize = sizeof(AnimOrder);
 	LoadTownerAnimations(towner, "Towners\\Strytell\\Strytell.CEL", 25, initData.dir, 3);
-	towner._tName = _("Cain the Elder");
+	towner.name = _("Cain the Elder");
 }
 
 void InitDrunk(TownerStruct &towner, const TownerInit &initData)
@@ -212,7 +212,7 @@ void InitDrunk(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = AnimOrder;
 	towner.animOrderSize = sizeof(AnimOrder);
 	LoadTownerAnimations(towner, "Towners\\Drunk\\TwnDrunk.CEL", 18, initData.dir, 3);
-	towner._tName = _("Farnham the Drunk");
+	towner.name = _("Farnham the Drunk");
 }
 
 void InitCows(TownerStruct &towner, const TownerInit &initData)
@@ -225,7 +225,7 @@ void InitCows(TownerStruct &towner, const TownerInit &initData)
 	}
 	NewTownerAnim(towner, towner._tNAnim[initData.dir], 12, 3);
 	towner._tAnimFrame = GenerateRnd(11) + 1;
-	towner._tName = _("Cow");
+	towner.name = _("Cow");
 
 	const Point position = initData.position;
 	const Point offset = position + CowOffsets[initData.dir];
@@ -244,7 +244,7 @@ void InitFarmer(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = nullptr;
 	towner.animOrderSize = 0;
 	LoadTownerAnimations(towner, "Towners\\Farmer\\Farmrn2.CEL", 15, initData.dir, 3);
-	towner._tName = _("Lester the farmer");
+	towner.name = _("Lester the farmer");
 }
 
 void InitCowFarmer(TownerStruct &towner, const TownerInit &initData)
@@ -257,7 +257,7 @@ void InitCowFarmer(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = nullptr;
 	towner.animOrderSize = 0;
 	LoadTownerAnimations(towner, celPath, 15, initData.dir, 3);
-	towner._tName = _("Complete Nut");
+	towner.name = _("Complete Nut");
 }
 
 void InitGirl(TownerStruct &towner, const TownerInit &initData)
@@ -270,7 +270,7 @@ void InitGirl(TownerStruct &towner, const TownerInit &initData)
 	towner.animOrder = nullptr;
 	towner.animOrderSize = 0;
 	LoadTownerAnimations(towner, celPath, 20, initData.dir, 6);
-	towner._tName = "Celia";
+	towner.name = "Celia";
 }
 
 void TownDead(TownerStruct &towner)
@@ -283,7 +283,7 @@ void TownDead(TownerStruct &towner)
 
 	if ((Quests[Q_BUTCHER]._qactive == QUEST_DONE || Quests[Q_BUTCHER]._qvar1 == 1) && towner._tAnimLen != 1) {
 		towner._tAnimLen = 1;
-		towner._tName = _("Slain Townsman");
+		towner.name = _("Slain Townsman");
 	}
 }
 
@@ -830,7 +830,7 @@ void InitTowners()
 void FreeTownerGFX()
 {
 	for (auto &towner : Towners) {
-		towner._tNData = nullptr;
+		towner.data = nullptr;
 	}
 
 	CowCels = nullptr;

Source/towners.h

@@ -37,24 +37,27 @@ enum _talker_id : uint8_t {
 
 struct TownerStruct {
 	byte *_tNAnim[8];
-	std::unique_ptr<byte[]> _tNData;
+	std::unique_ptr<byte[]> data;
 	byte *_tAnimData;
 	/** Used to get a voice line and text related to active quests when the player speaks to a town npc */
-	int16_t _tSeed;
+	int16_t seed;
 	/** Tile position of NPC */
 	Point position;
 	int16_t _tAnimWidth;
-	int16_t _tAnimDelay; // Tick length of each frame in the current animation
-	int16_t _tAnimCnt;   // Increases by one each game tick, counting how close we are to _pAnimDelay
-	uint8_t _tAnimLen;   // Number of frames in current animation
-	uint8_t _tAnimFrame; // Current frame of animation.
+	/** Tick length of each frame in the current animation */
+	int16_t _tAnimDelay;
+	/** Increases by one each game tick, counting how close we are to _pAnimDelay */
+	int16_t _tAnimCnt;
+	/** Number of frames in current animation */
+	uint8_t _tAnimLen;
+	/** Current frame of animation. */
+	uint8_t _tAnimFrame;
 	uint8_t _tAnimFrameCnt;
-	string_view _tName;
+	string_view name;
 	/** Specifies the animation frame sequence. */
 	const uint8_t *animOrder; // unowned
 	std::size_t animOrderSize;
-	void (*talk)(PlayerStruct &player, TownerStruct &barOwner);
-	bool _tbtcnt;
+	void (*talk)(PlayerStruct &player, TownerStruct &towner);
 	_talker_id _ttype;
 };
 

Source/utils/language.cpp

@@ -242,10 +242,7 @@ bool HasTranslation(const std::string &locale)
 		return true;
 
 	std::string moPath = paths::LangPath() + locale + ".mo";
-	if (FileExists(moPath.c_str()))
-		return true;
-
-	return false;
+	return FileExists(moPath.c_str());
 }
 
 void LanguageInitialize()

Source/utils/log.hpp

@@ -39,7 +39,7 @@ enum class LogPriority {
 namespace detail {
 
 template <typename... Args>
-std::string format(const char *fmt, Args &&...args)
+std::string format(const char *fmt, Args &&... args)
 {
 	FMT_TRY
 	{
@@ -56,99 +56,99 @@ std::string format(const char *fmt, Args &&...args)
 } // namespace detail
 
 template <typename... Args>
-void Log(const char *fmt, Args &&...args)
+void Log(const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_Log("%s", str.c_str());
 }
 
 template <typename... Args>
-void LogVerbose(LogCategory category, const char *fmt, Args &&...args)
+void LogVerbose(LogCategory category, const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_LogVerbose(static_cast<int>(category), "%s", str.c_str());
 }
 
 template <typename... Args>
-void LogVerbose(const char *fmt, Args &&...args)
+void LogVerbose(const char *fmt, Args &&... args)
 {
 	LogVerbose(defaultCategory, fmt, std::forward<Args>(args)...);
 }
 
 template <typename... Args>
-void LogDebug(LogCategory category, const char *fmt, Args &&...args)
+void LogDebug(LogCategory category, const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_LogDebug(static_cast<int>(category), "%s", str.c_str());
 }
 
 template <typename... Args>
-void LogDebug(const char *fmt, Args &&...args)
+void LogDebug(const char *fmt, Args &&... args)
 {
 	LogDebug(defaultCategory, fmt, std::forward<Args>(args)...);
 }
 
 template <typename... Args>
-void LogInfo(LogCategory category, const char *fmt, Args &&...args)
+void LogInfo(LogCategory category, const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_LogInfo(static_cast<int>(category), "%s", str.c_str());
 }
 
 template <typename... Args>
-void LogInfo(const char *fmt, Args &&...args)
+void LogInfo(const char *fmt, Args &&... args)
 {
 	LogInfo(defaultCategory, fmt, std::forward<Args>(args)...);
 }
 
 template <typename... Args>
-void LogWarn(LogCategory category, const char *fmt, Args &&...args)
+void LogWarn(LogCategory category, const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_LogWarn(static_cast<int>(category), "%s", str.c_str());
 }
 
 template <typename... Args>
-void LogWarn(const char *fmt, Args &&...args)
+void LogWarn(const char *fmt, Args &&... args)
 {
 	LogWarn(defaultCategory, fmt, std::forward<Args>(args)...);
 }
 
 template <typename... Args>
-void LogError(LogCategory category, const char *fmt, Args &&...args)
+void LogError(LogCategory category, const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_LogError(static_cast<int>(category), "%s", str.c_str());
 }
 
 template <typename... Args>
-void LogError(const char *fmt, Args &&...args)
+void LogError(const char *fmt, Args &&... args)
 {
 	LogError(defaultCategory, fmt, std::forward<Args>(args)...);
 }
 
 template <typename... Args>
-void LogCritical(LogCategory category, const char *fmt, Args &&...args)
+void LogCritical(LogCategory category, const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_LogCritical(static_cast<int>(category), "%s", str.c_str());
 }
 
 template <typename... Args>
-void LogCritical(const char *fmt, Args &&...args)
+void LogCritical(const char *fmt, Args &&... args)
 {
 	LogCritical(defaultCategory, fmt, std::forward<Args>(args)...);
 }
 
 template <typename... Args>
-void LogMessageV(LogCategory category, LogPriority priority, const char *fmt, Args &&...args)
+void LogMessageV(LogCategory category, LogPriority priority, const char *fmt, Args &&... args)
 {
 	auto str = detail::format(fmt, std::forward<Args>(args)...);
 	SDL_LogMessageV(static_cast<int>(category), static_cast<SDL_LogPriority>(priority), "%s", str.c_str());
 }
 
 template <typename... Args>
-void LogMessageV(const char *fmt, Args &&...args)
+void LogMessageV(const char *fmt, Args &&... args)
 {
 	LogMessageV(defaultCategory, fmt, std::forward<Args>(args)...);
 }

Source/utils/sdl_cond.h

@@ -46,4 +46,3 @@ private:
 };
 
 } // namespace devilution
-

Source/utils/sdl_thread.h

@@ -10,20 +10,20 @@
 namespace devilution {
 
 namespace this_sdl_thread {
-	inline SDL_threadID get_id()
-	{
-		return SDL_GetThreadID(nullptr);
-	}
+inline SDL_threadID get_id()
+{
+	return SDL_GetThreadID(nullptr);
+}
 } //namespace this_sdl_thread
 
 class SdlThread final {
 	static int SDLCALL ThreadTranslate(void *ptr);
 	static void ThreadDeleter(SDL_Thread *thread);
 
-	std::unique_ptr<SDL_Thread, void(*)(SDL_Thread *)> thread { nullptr, ThreadDeleter };
+	std::unique_ptr<SDL_Thread, void (*)(SDL_Thread *)> thread { nullptr, ThreadDeleter };
 
 public:
-	SdlThread(int (SDLCALL *handler)(void *), void *data)
+	SdlThread(int(SDLCALL *handler)(void *), void *data)
 #ifdef USE_SDL1
 	    : thread(SDL_CreateThread(handler, data), ThreadDeleter)
 #else
@@ -64,4 +64,3 @@ public:
 };
 
 } // namespace devilution
-