Big code cleanup/refactor for 3.3.0

Similar to what I did for 3.2.0

- Re-run clang-format over everything
- Apply static code analysis suggestions (except where they can't be)
- Remove as many #includes from .h files as possible (still a bunch more can be done here)
- Clean up Main.h/common(2).h

cmake/unix.cmake

@@ -243,7 +243,7 @@ endif()
 
 if (NOT NO_COTIRE)
 	set_target_properties(slade PROPERTIES
-		COTIRE_CXX_PREFIX_HEADER_INIT "common.h"
+		COTIRE_CXX_PREFIX_HEADER_INIT "Application/Main.h"
 		# Enable multithreaded unity builds by default
 		# because otherwise probably no one would realize how
 		COTIRE_UNITY_SOURCE_MAXIMUM_NUMBER_OF_INCLUDES -j

cmake/win_msvc.cmake

@@ -78,7 +78,7 @@ set_target_properties(slade
 )
 
 # Precompiled Header
-target_precompile_headers(slade PRIVATE "common.h")
+target_precompile_headers(slade PRIVATE "Application/Main.h")
 
 # Link
 target_link_libraries(slade

src/.clang-format

@@ -62,3 +62,9 @@ TabWidth: 4
 UseTab: ForContinuationAndIndentation
 PenaltyReturnTypeOnItsOwnLine: 1000
 PenaltyBreakAssignment: 1000
+
+AlignConsecutiveShortCaseStatements:
+  Enabled: true
+  AcrossEmptyLines: true
+  AcrossComments: true
+  AlignCaseColons: false

src/Application/App.cpp

@@ -1,7 +1,7 @@
 
 // -----------------------------------------------------------------------------
 // SLADE - It's a Doom Editor
-// Copyright(C) 2008 - 2022 Simon Judd
+// Copyright(C) 2008 - 2024 Simon Judd
 //
 // Email:       [email protected]
 // Web:         http://slade.mancubus.net
@@ -33,7 +33,8 @@
 #include "Main.h"
 #include "App.h"
 #include "Archive/ArchiveManager.h"
-#include "Game/Configuration.h"
+#include "Game/Game.h"
+#include "Game/SpecialPreset.h"
 #include "General/Clipboard.h"
 #include "General/ColourConfiguration.h"
 #include "General/Console.h"
@@ -60,12 +61,17 @@
 #include "UI/WxUtils.h"
 #include "Utility/StringUtils.h"
 #include "Utility/Tokenizer.h"
+#include <FreeImage.h>
 #include <dumb.h>
 #include <filesystem>
 #ifdef __WXOSX__
 #include <ApplicationServices/ApplicationServices.h>
 #endif
 
+#ifndef _WIN32
+#undef _WINDOWS_ // Undefine _WINDOWS_ that has been defined by FreeImage
+#endif
+
 using namespace slade;
 
 
@@ -748,12 +754,12 @@ string app::path(string_view filename, Dir dir)
 {
 	switch (dir)
 	{
-	case Dir::User: return fmt::format("{}{}{}", dir_user, dir_separator, filename);
-	case Dir::Data: return fmt::format("{}{}{}", dir_data, dir_separator, filename);
+	case Dir::User:       return fmt::format("{}{}{}", dir_user, dir_separator, filename);
+	case Dir::Data:       return fmt::format("{}{}{}", dir_data, dir_separator, filename);
 	case Dir::Executable: return fmt::format("{}{}{}", dir_app, dir_separator, filename);
-	case Dir::Resources: return fmt::format("{}{}{}", dir_res, dir_separator, filename);
-	case Dir::Temp: return fmt::format("{}{}{}", dir_temp, dir_separator, filename);
-	default: return string{ filename };
+	case Dir::Resources:  return fmt::format("{}{}{}", dir_res, dir_separator, filename);
+	case Dir::Temp:       return fmt::format("{}{}{}", dir_temp, dir_separator, filename);
+	default:              return string{ filename };
 	}
 }
 

src/common.h → src/Application/Common.h

@@ -1,5 +1,6 @@
-#ifndef COMMON_H
-#define COMMON_H
+
+#ifndef __COMMON_H__
+#define __COMMON_H__
 
 // wxWidgets -------------------------------------------------------------------
 
@@ -102,21 +103,6 @@
 
 // Other Libraries -------------------------------------------------------------
 
-// Fluidsynth
-#ifndef NO_FLUIDSYNTH
-#include <fluidsynth.h>
-#endif
-
-// SFML
-#include <SFML/System.hpp>
-
-// Freeimage
-#define FREEIMAGE_LIB
-#include <FreeImage.h>
-#ifndef _WIN32
-#undef _WINDOWS_ // Undefine _WINDOWS_ that has been defined by FreeImage
-#endif
-
 // fmt
 #include <fmt/core.h>
 
@@ -134,6 +120,7 @@
 #include <optional>
 #include <set>
 #include <unordered_map>
+#include <variant>
 #include <vector>
 
-#endif // COMMON_H
+#endif // __COMMON_H__

src/Application/Main.h

@@ -2,8 +2,7 @@
 #ifndef __MAIN_H__
 #define __MAIN_H__
 
-#include "common.h"
-#include "common2.h"
+#include "Common.h"
 
 #if defined _MSC_VER && _MSC_VER < 1900
 #define _CRT_SECURE_NO_WARNINGS 1

src/Application/SLADEWxApp.cpp

@@ -1,7 +1,7 @@
 
 // -----------------------------------------------------------------------------
 // SLADE - It's a Doom Editor
-// Copyright(C) 2008 - 2022 Simon Judd
+// Copyright(C) 2008 - 2024 Simon Judd
 //
 // Email:       [email protected]
 // Web:         http://slade.mancubus.net
@@ -71,7 +71,7 @@ string sc_rev;
 #ifdef DEBUG
 bool debug = true;
 #else
-bool   debug = false;
+bool debug = false;
 #endif
 
 int win_version_major = 0;
@@ -442,7 +442,7 @@ bool SLADEWxApp::OnInit()
 #ifdef __WINDOWS__
 	wxApp::SetAppName("SLADE3");
 #else
-    wxApp::SetAppName("slade3");
+	wxApp::SetAppName("slade3");
 #endif
 
 	// Handle exceptions using wxDebug stuff, but only in release mode
@@ -457,11 +457,11 @@ bool SLADEWxApp::OnInit()
 	// Should be constant, wxWidgets Cocoa backend scales everything under the hood
 	const double ui_scale = 1.0;
 #else  // !__APPLE__
-    // Calculate scaling factor (from system ppi)
-    wxMemoryDC dc;
-    double     ui_scale = static_cast<double>(dc.GetPPI().x) / 96.0;
-    if (ui_scale < 1.)
-        ui_scale = 1.;
+	// Calculate scaling factor (from system ppi)
+	wxMemoryDC dc;
+	double     ui_scale = static_cast<double>(dc.GetPPI().x) / 96.0;
+	if (ui_scale < 1.)
+		ui_scale = 1.;
 #endif // __APPLE__
 
 	// Get Windows version

src/Archive/Archive.cpp

@@ -1,7 +1,7 @@
 
 // -----------------------------------------------------------------------------
 // SLADE - It's a Doom Editor
-// Copyright(C) 2008 - 2022 Simon Judd
+// Copyright(C) 2008 - 2024 Simon Judd
 //
 // Email:       [email protected]
 // Web:         http://slade.mancubus.net
@@ -36,6 +36,7 @@
 #include "Utility/FileUtils.h"
 #include "Utility/Parser.h"
 #include "Utility/StringUtils.h"
+#include <SFML/System/Clock.hpp>
 #include <filesystem>
 
 using namespace slade;
@@ -72,8 +73,7 @@ class EntryRenameUS : public UndoStep
 	bool doUndo() override
 	{
 		// Get entry parent dir
-		const auto dir = archive_->dirAtPath(entry_path_);
-		if (dir)
+		if (const auto dir = archive_->dirAtPath(entry_path_))
 		{
 			// Rename entry
 			const auto entry = dir->entryAt(entry_index_);
@@ -86,8 +86,7 @@ class EntryRenameUS : public UndoStep
 	bool doRedo() override
 	{
 		// Get entry parent dir
-		const auto dir = archive_->dirAtPath(entry_path_);
-		if (dir)
+		if (const auto dir = archive_->dirAtPath(entry_path_))
 		{
 			// Rename entry
 			const auto entry = dir->entryAt(entry_index_);
@@ -151,15 +150,17 @@ class EntrySwapUS : public UndoStep
 {
 public:
 	EntrySwapUS(const ArchiveDir* dir, unsigned index1, unsigned index2) :
-		archive_{ dir->archive() }, path_{ dir->path() }, index1_{ index1 }, index2_{ index2 }
+		archive_{ dir->archive() },
+		path_{ dir->path() },
+		index1_{ index1 },
+		index2_{ index2 }
 	{
 	}
 
 	bool doSwap() const
 	{
 		// Get parent dir
-		auto dir = archive_->dirAtPath(path_);
-		if (dir)
+		if (const auto dir = archive_->dirAtPath(path_))
 			return dir->swapEntries(index1_, index2_);
 		return false;
 	}
@@ -196,8 +197,7 @@ class EntryCreateDeleteUS : public UndoStep
 	bool createEntry() const
 	{
 		// Get parent dir
-		const auto dir = archive_->dirAtPath(path_);
-		if (dir)
+		if (const auto dir = archive_->dirAtPath(path_))
 		{
 			archive_->addEntry(std::make_shared<ArchiveEntry>(*entry_copy_), index_, dir);
 			return true;
@@ -223,7 +223,9 @@ class DirCreateDeleteUS : public UndoStep
 {
 public:
 	DirCreateDeleteUS(bool created, ArchiveDir* dir) :
-		created_{ created }, archive_{ dir->archive() }, path_{ dir->path() }
+		created_{ created },
+		archive_{ dir->archive() },
+		path_{ dir->path() }
 	{
 		strutil::removePrefixIP(path_, '/');
 
@@ -282,7 +284,7 @@ class DirCreateDeleteUS : public UndoStep
 
 // -----------------------------------------------------------------------------
 //
-// Archive::MapDesc Class Functions
+// MapDesc Struct Functions
 //
 // -----------------------------------------------------------------------------
 
@@ -291,7 +293,7 @@ class DirCreateDeleteUS : public UndoStep
 // Returns a list of all data entries (eg. LINEDEFS, TEXTMAP) for the map.
 // If [include_head] is true, the map header entry is also added
 // -----------------------------------------------------------------------------
-vector<ArchiveEntry*> Archive::MapDesc::entries(const Archive& parent, bool include_head) const
+vector<ArchiveEntry*> MapDesc::entries(const Archive& parent, bool include_head) const
 {
 	vector<ArchiveEntry*> list;
 
@@ -313,7 +315,7 @@ vector<ArchiveEntry*> Archive::MapDesc::entries(const Archive& parent, bool incl
 // -----------------------------------------------------------------------------
 // Sets the appropriate 'MapFormat' extra property in all entries for the map
 // -----------------------------------------------------------------------------
-void Archive::MapDesc::updateMapFormatHints() const
+void MapDesc::updateMapFormatHints() const
 {
 	// Get parent archive
 	Archive* parent = nullptr;
@@ -326,13 +328,13 @@ void Archive::MapDesc::updateMapFormatHints() const
 	string fmt_name;
 	switch (format)
 	{
-	case MapFormat::Doom: fmt_name = "doom"; break;
-	case MapFormat::Hexen: fmt_name = "hexen"; break;
-	case MapFormat::Doom64: fmt_name = "doom64"; break;
-	case MapFormat::UDMF: fmt_name = "udmf"; break;
+	case MapFormat::Doom:    fmt_name = "doom"; break;
+	case MapFormat::Hexen:   fmt_name = "hexen"; break;
+	case MapFormat::Doom64:  fmt_name = "doom64"; break;
+	case MapFormat::UDMF:    fmt_name = "udmf"; break;
 	case MapFormat::Doom32X: fmt_name = "doom32x"; break;
 	case MapFormat::Unknown:
-	default: fmt_name = "unknown"; break;
+	default:                 fmt_name = "unknown"; break;
 	}
 
 	// Set format hint in map entries
@@ -747,7 +749,7 @@ void Archive::entryStateChanged(ArchiveEntry* entry)
 // -----------------------------------------------------------------------------
 // Adds the directory structure starting from [start] to [list]
 // -----------------------------------------------------------------------------
-void Archive::putEntryTreeAsList(vector<ArchiveEntry*>& list, ArchiveDir* start) const
+void Archive::putEntryTreeAsList(vector<ArchiveEntry*>& list, const ArchiveDir* start) const
 {
 	// If no start dir is specified, use the root dir
 	if (!start)
@@ -766,7 +768,7 @@ void Archive::putEntryTreeAsList(vector<ArchiveEntry*>& list, ArchiveDir* start)
 // -----------------------------------------------------------------------------
 // Adds the directory structure starting from [start] to [list]
 // -----------------------------------------------------------------------------
-void Archive::putEntryTreeAsList(vector<shared_ptr<ArchiveEntry>>& list, ArchiveDir* start) const
+void Archive::putEntryTreeAsList(vector<shared_ptr<ArchiveEntry>>& list, const ArchiveDir* start) const
 {
 	// If no start dir is specified, use the root dir
 	if (!start)
@@ -1429,12 +1431,11 @@ ArchiveEntry* Archive::findFirst(SearchOptions& options)
 	{
 		for (unsigned a = 0; a < dir->numSubdirs(); a++)
 		{
-			auto opt         = options;
-			opt.dir          = dir->subdirAt(a).get();
-			const auto match = findFirst(opt);
+			auto opt = options;
+			opt.dir  = dir->subdirAt(a).get();
 
 			// If a match was found in this subdir, return it
-			if (match)
+			if (const auto match = findFirst(opt))
 				return match;
 		}
 	}
@@ -1499,12 +1500,11 @@ ArchiveEntry* Archive::findLast(SearchOptions& options)
 	{
 		for (int a = static_cast<int>(dir->numSubdirs()) - 1; a >= 0; a--)
 		{
-			auto opt         = options;
-			opt.dir          = dir->subdirAt(a).get();
-			const auto match = findLast(opt);
+			auto opt = options;
+			opt.dir  = dir->subdirAt(a).get();
 
 			// If a match was found in this subdir, return it
-			if (match)
+			if (const auto match = findLast(opt))
 				return match;
 		}
 	}