refactor(code): Minor tweaks, refactors and code style changes (endle…

…ss-sky#9462)

source/AI.cpp

@@ -907,7 +907,7 @@ void AI::Step(const PlayerInfo &player, Command &activeCommands)
 					for(const auto &other : otherShips)
 						if(other->GetGovernment() == gov && other->GetSystem() == it->GetSystem() && !other->CanBeCarried())
 						{
-							if(!other->IsDisabled() && other->CanCarry(*it.get()))
+							if(!other->IsDisabled() && other->CanCarry(*it))
 								return other;
 							else
 								parentChoices.emplace_back(other);
@@ -3029,7 +3029,7 @@ void AI::DoPatrol(Ship &ship, Command &command) const
 			for(const StellarObject &object : ship.GetSystem()->Objects())
 				if(object.HasSprite() && object.GetPlanet() && object.GetPlanet()->CanLand(ship))
 					landingTargets.push_back(&object);
-			if(landingTargets.size())
+			if(!landingTargets.empty())
 			{
 				ship.SetTargetStellar(landingTargets[Random::Int(landingTargets.size())]);
 				MoveToPlanet(ship, command);

source/Account.cpp

@@ -83,7 +83,7 @@ void Account::Save(DataWriter &out) const
 	out.BeginChild();
 	{
 		out.Write("credits", credits);
-		if(salariesIncome.size() > 0)
+		if(!salariesIncome.empty())
 		{
 			out.Write("salaries income");
 			out.BeginChild();
@@ -319,7 +319,7 @@ int64_t Account::SalariesIncomeTotal() const
 
 
 
-void Account::SetSalaryIncome(string name, int64_t amount)
+void Account::SetSalaryIncome(const string &name, int64_t amount)
 {
 	if(amount == 0)
 		salariesIncome.erase(name);

source/Account.h

@@ -47,7 +47,7 @@ class Account {
 	// Structural income.
 	const std::map<std::string, int64_t> &SalariesIncome() const;
 	int64_t SalariesIncomeTotal() const;
-	void SetSalaryIncome(std::string name, int64_t amount);
+	void SetSalaryIncome(const std::string &name, int64_t amount);
 
 	// Overdue crew salaries:
 	int64_t CrewSalariesOwed() const;

source/AmmoDisplay.h

@@ -32,7 +32,7 @@ class Ship;
 // A class for handling the secondary weapon icons displayed in the HUD.
 class AmmoDisplay {
 public:
-	AmmoDisplay(PlayerInfo &player);
+	explicit AmmoDisplay(PlayerInfo &player);
 	void Update(const Ship &flagship);
 	void Draw(const Rectangle &ammoBox, const Point &iconDimensions) const;
 	bool Click(const Point &clickPoint, bool control);

source/BoardingPanel.cpp

@@ -39,6 +39,7 @@ this program. If not, see <https://www.gnu.org/licenses/>.
 #include "UI.h"
 
 #include <algorithm>
+#include <utility>
 
 using namespace std;
 

source/CategoryList.h

@@ -38,9 +38,9 @@ class CategoryList {
 	// name.
 	class Category {
 	public:
-		Category(std::string name, int precedence) : name(name), precedence(precedence) {}
+		Category(const std::string &name, int precedence) : name(name), precedence(precedence) {}
 		const std::string &Name() const { return name; }
-		const bool operator<(Category other) const { return SortHelper(*this, other); }
+		const bool operator<(const Category &other) const { return SortHelper(*this, other); }
 		const bool operator()(Category &a, Category &b) const { return SortHelper(a, b); }
 
 	private:

source/CollisionSet.cpp

@@ -45,7 +45,7 @@ namespace {
 	// than that.
 	class Closest {
 	public:
-		Closest(double closestHit)
+		explicit Closest(double closestHit)
 			: closest_dist(closestHit)
 			, closest_body(nullptr)
 		{}

source/ConditionSet.h

@@ -37,7 +37,7 @@ class ConditionSet {
 	ConditionSet() = default;
 
 	// Construct and Load() at the same time.
-	ConditionSet(const DataNode &node);
+	explicit ConditionSet(const DataNode &node);
 
 	// Load a set of conditions from the children of this node. Prints a
 	// warning if an and/or node contains assignment expressions.
@@ -108,8 +108,8 @@ class ConditionSet {
 		// sequence of "Operations" is created for runtime evaluation.
 		class SubExpression {
 		public:
-			SubExpression(const std::vector<std::string> &side);
-			SubExpression(const std::string &side);
+			explicit SubExpression(const std::vector<std::string> &side);
+			explicit SubExpression(const std::string &side);
 
 			// Interleave tokens and operators to reproduce the initial string.
 			const std::string ToString() const;

source/ConditionsStore.cpp

@@ -180,19 +180,19 @@ void ConditionsStore::Save(DataWriter &out) const
 {
 	out.Write("conditions");
 	out.BeginChild();
-	for(auto it = storage.begin(); it != storage.end(); ++it)
+	for(const auto &it : storage)
 	{
 		// We don't need to save derived conditions that have a provider.
-		if(it->second.provider)
+		if(it.second.provider)
 			continue;
 		// If the condition's value is 0, don't write it at all.
-		if(!it->second.value)
+		if(!it.second.value)
 			continue;
 		// If the condition's value is 1, don't bother writing the 1.
-		if(it->second.value == 1)
-			out.Write(it->first);
+		if(it.second.value == 1)
+			out.Write(it.first);
 		else
-			out.Write(it->first, it->second.value);
+			out.Write(it.first, it.second.value);
 	}
 	out.EndChild();
 }
@@ -387,10 +387,10 @@ void ConditionsStore::Clear()
 int64_t ConditionsStore::PrimariesSize() const
 {
 	int64_t result = 0;
-	for(auto it = storage.begin(); it != storage.end(); ++it)
+	for(const auto &it : storage)
 	{
 		// We only count primary conditions; conditions that don't have a provider.
-		if(it->second.provider)
+		if(it.second.provider)
 			continue;
 		++result;
 	}
@@ -419,7 +419,7 @@ const ConditionsStore::ConditionEntry *ConditionsStore::GetEntry(const string &n
 
 	--it;
 	// The entry is matching if we have an exact string match.
-	if(!name.compare(it->first))
+	if(name == it->first)
 		return &(it->second);
 
 	// The entry is also matching when we have a prefix entry and the prefix part in the provider matches.

source/ConditionsStore.h

@@ -105,9 +105,9 @@ class ConditionsStore {
 public:
 	// Constructors to initialize this class.
 	ConditionsStore() = default;
-	ConditionsStore(const DataNode &node);
-	ConditionsStore(std::initializer_list<std::pair<std::string, int64_t>> initialConditions);
-	ConditionsStore(const std::map<std::string, int64_t> &initialConditions);
+	explicit ConditionsStore(const DataNode &node);
+	explicit ConditionsStore(std::initializer_list<std::pair<std::string, int64_t>> initialConditions);
+	explicit ConditionsStore(const std::map<std::string, int64_t> &initialConditions);
 
 	// Serialization support for this class.
 	void Load(const DataNode &node);

source/DamageProfile.h

@@ -32,9 +32,9 @@ class Weapon;
 class DamageProfile {
 public:
 	// Constructor for damage taken from a weapon projectile.
-	DamageProfile(Projectile::ImpactInfo info);
+	explicit DamageProfile(Projectile::ImpactInfo info);
 	// Constructor for damage taken from a hazard.
-	DamageProfile(Weather::ImpactInfo info);
+	explicit DamageProfile(Weather::ImpactInfo info);
 
 	// Calculate the damage dealt to the given ship.
 	DamageDealt CalculateDamage(const Ship &ship, bool ignoreBlast = false) const;

source/DataNode.cpp

@@ -39,7 +39,7 @@ DataNode::DataNode(const DataNode *parent) noexcept(false)
 
 // Copy constructor.
 DataNode::DataNode(const DataNode &other)
-	: children(other.children), tokens(other.tokens), lineNumber(other.lineNumber)
+	: children(other.children), tokens(other.tokens), lineNumber(std::move(other.lineNumber))
 {
 	Reparent();
 }
@@ -51,15 +51,15 @@ DataNode &DataNode::operator=(const DataNode &other)
 {
 	children = other.children;
 	tokens = other.tokens;
-	lineNumber = other.lineNumber;
+	lineNumber = std::move(other.lineNumber);
 	Reparent();
 	return *this;
 }
 
 
 
 DataNode::DataNode(DataNode &&other) noexcept
-	: children(std::move(other.children)), tokens(std::move(other.tokens)), lineNumber(std::move(other.lineNumber))
+	: children(std::move(other.children)), tokens(std::move(other.tokens)), lineNumber(other.lineNumber)
 {
 	Reparent();
 }
@@ -70,7 +70,7 @@ DataNode &DataNode::operator=(DataNode &&other) noexcept
 {
 	children.swap(other.children);
 	tokens.swap(other.tokens);
-	lineNumber = std::move(other.lineNumber);
+	lineNumber = other.lineNumber;
 	Reparent();
 	return *this;
 }

source/Depreciation.cpp

@@ -297,7 +297,7 @@ int64_t Depreciation::Value(const Outfit *outfit, int day, int count) const
 
 // "Sell" an item, removing it from the given record and returning the base
 // day for its depreciation.
-int Depreciation::Sell(map<int, int> &record)
+int Depreciation::Sell(map<int, int> &record) const
 {
 	// If we're a planet, we start by selling the oldest, cheapest thing.
 	auto it = (isStock ? record.begin() : --record.end());

source/Depreciation.h

@@ -66,7 +66,7 @@ class Depreciation {
 private:
 	// "Sell" an item, removing it from the given record and returning the base
 	// day for its depreciation.
-	int Sell(std::map<int, int> &record);
+	int Sell(std::map<int, int> &record) const;
 	// Calculate depreciation:
 	double Depreciate(const std::map<int, int> &record, int day, int count = 1) const;
 	double Depreciate(int age) const;

source/Dialog.cpp

@@ -35,6 +35,7 @@ this program. If not, see <https://www.gnu.org/licenses/>.
 #include "UI.h"
 
 #include <cmath>
+#include <utility>
 
 using namespace std;
 

source/DistanceCalculationSettings.h

@@ -25,7 +25,7 @@ class DataNode;
 class DistanceCalculationSettings {
 public:
 	DistanceCalculationSettings() = default;
-	DistanceCalculationSettings(const DataNode &node);
+	explicit DistanceCalculationSettings(const DataNode &node);
 
 	bool operator!=(const DistanceCalculationSettings &other) const;
 

source/DistanceMap.h

@@ -73,7 +73,7 @@ class DistanceMap {
 	// days, how much danger you will pass through, and where you will go next.
 	class Edge {
 	public:
-		Edge(const System *system = nullptr);
+		explicit Edge(const System *system = nullptr);
 
 		// Sorting operator to prioritize the "best" edges. The priority queue
 		// returns the "largest" item, so this should return true if this item

source/Engine.cpp

@@ -659,7 +659,7 @@ void Engine::Step(bool isActive)
 
 	// Update the player's ammo amounts.
 	if(flagship)
-		ammoDisplay.Update(*flagship.get());
+		ammoDisplay.Update(*flagship);
 
 
 	// Display escort information for all ships of the "Escort" government,

source/EsUuid.cpp

@@ -31,6 +31,8 @@ this program. If not, see <https://www.gnu.org/licenses/>.
 #include <uuid/uuid.h>
 #endif
 
+
+
 namespace es_uuid {
 namespace detail {
 #if defined(_WIN32)

source/File.h

@@ -35,7 +35,7 @@ class File {
 	// Move assignment is OK though.
 	File &operator=(File &&) noexcept;
 
-	operator bool() const;
+	explicit operator bool() const;
 	operator FILE*() const;
 
 private:

source/FormationPattern.h

@@ -34,7 +34,7 @@ class FormationPattern {
 	// Iterator that provides sequential access to all formation positions.
 	class PositionIterator {
 	public:
-		PositionIterator(const FormationPattern &pattern);
+		explicit PositionIterator(const FormationPattern &pattern);
 		PositionIterator() = delete;
 
 		// Iterator traits

source/GameEvent.h

@@ -49,7 +49,7 @@ class GameEvent {
 public:
 	GameEvent() = default;
 	// Construct and Load() at the same time.
-	GameEvent(const DataNode &node);
+	explicit GameEvent(const DataNode &node);
 
 	void Load(const DataNode &node);
 	void Save(DataWriter &out) const;

source/HailPanel.cpp

@@ -38,13 +38,15 @@ this program. If not, see <https://www.gnu.org/licenses/>.
 
 #include <algorithm>
 #include <cmath>
+#include <utility>
 
 using namespace std;
 
 
 
 HailPanel::HailPanel(PlayerInfo &player, const shared_ptr<Ship> &ship, function<void(const Government *)> bribeCallback)
-	: player(player), ship(ship), bribeCallback(bribeCallback), sprite(ship->GetSprite()), facing(ship->Facing())
+	: player(player), ship(ship), bribeCallback(std::move(bribeCallback)),
+		sprite(ship->GetSprite()), facing(ship->Facing())
 {
 	SetInterruptible(false);
 

source/ImageSet.h

@@ -46,7 +46,7 @@ class ImageSet {
 public:
 	// ImageSets should be created with a name, as some image paths (e.g. plugin icons)
 	// do not contain the associated image name.
-	ImageSet(std::string name);
+	explicit ImageSet(std::string name);
 
 	// Get the name of the sprite for this image set.
 	const std::string &Name() const;

source/InfoPanelState.h

@@ -33,7 +33,7 @@ class InfoPanelState {
 public:
 	using ShipComparator = bool(const std::shared_ptr<Ship> &, const std::shared_ptr<Ship> &);
 
-	InfoPanelState(PlayerInfo &player);
+	explicit InfoPanelState(PlayerInfo &player);
 
 	int SelectedIndex() const;
 	void SetSelectedIndex(int newSelectedIndex);

source/LocationFilter.h

@@ -40,7 +40,7 @@ class LocationFilter {
 public:
 	LocationFilter() noexcept = default;
 	// Construct and Load() at the same time.
-	LocationFilter(const DataNode &node);
+	explicit LocationFilter(const DataNode &node);
 
 	// Examine all the children of the given node and load any that are filters.
 	void Load(const DataNode &node);

source/MapPanel.cpp

@@ -1276,7 +1276,7 @@ void MapPanel::DrawEscorts()
 			if(squad.second.activeShips || squad.second.parkedShips)
 				RingShader::Draw(pos, INNER - 1.f, 0.f, squad.second.activeShips ? active : parked);
 
-			if(squad.second.outfits.size())
+			if(!squad.second.outfits.empty())
 				// Stored outfits are drawn/indicated by 8 short rays out of the system center.
 				for(int i = 0; i < 8; ++i)
 				{

source/MapPlanetCard.cpp

@@ -101,8 +101,6 @@ MapPlanetCard::ClickAction MapPlanetCard::Click(int x, int y, int clicks)
 		double relativeY = (y - yCoordinate);
 		if(relativeY > 0. && relativeY < AvailableSpace())
 		{
-			isSelected = true;
-
 			// The first category is the planet name and is not selectable.
 			if(x > Screen::Left() + planetIconMaxSize &&
 					relativeY > textStart + categorySize && relativeY < textStart + categorySize * (categories + hasGovernments))

source/MapSalesPanel.cpp

@@ -179,7 +179,7 @@ bool MapSalesPanel::Hover(int x, int y)
 {
 	isDragging = (x < Screen::Left() + WIDTH);
 
-	return isDragging ? true : MapPanel::Hover(x, y);
+	return isDragging || MapPanel::Hover(x, y);
 }
 
 

source/MenuPanel.cpp

@@ -66,7 +66,7 @@ MenuPanel::MenuPanel(PlayerInfo &player, UI &gamePanels)
 		for(const auto &source : GameData::Sources())
 		{
 			auto credit = Format::Split(Files::Read(source + "credits.txt"), "\n");
-			if((credit.size() > 1) || (credit.front() != ""))
+			if((credit.size() > 1) || !credit.front().empty())
 			{
 				credits.insert(credits.end(), credit.begin(), credit.end());
 				credits.insert(credits.end(), 15, "");

source/Messages.h

@@ -18,6 +18,7 @@ this program. If not, see <https://www.gnu.org/licenses/>.
 
 #include <cstdint>
 #include <string>
+#include <utility>
 #include <vector>
 
 

source/Mission.h

@@ -58,7 +58,7 @@ class Mission {
 	~Mission() noexcept = default;
 
 	// Construct and Load() at the same time.
-	Mission(const DataNode &node);
+	explicit Mission(const DataNode &node);
 
 	// Load a mission, either from the game data or from a saved game.
 	void Load(const DataNode &node);