Enable some more clang-tidy linter checks (#4738)

* Avoid inefficient AString -> c_str() -> AString round trip

* Avoid redundant string init expressions

* Avoid unnecessary return, continue, etc.

* Add .clang-format to help with clang-tidy fix-its

* Avoid unnecessary passing by value

* Avoid unnecessary local copying

* Avoid copying in range-for loops

* Avoid over-complicated boolean expressions

* Some violations missed by my local clang-tidy

* Allow unnecessary continue statements

* Add brackets

* Another expression missed locally

* Move BindingsProcessor call into clang-tidy.sh and add space

* Fix pushd not found error

* Different grouping of CheckBlockInteractionRate

.circleci/config.yml

@@ -26,7 +26,6 @@ jobs:
     docker: *cube_docker
     steps:
     - attach_workspace: { at: ~/ }
-    - run: (cd src/Bindings && lua BindingsProcessor.lua)
     - run: ./clang-tidy.sh -j 2
 
 workflows:

.clang-format

@@ -0,0 +1,39 @@
+# Not the exact style guide but enough for basic clang-tidy fix-its
+Language: Cpp
+BasedOnStyle: LLVM
+
+AlignAfterOpenBracket: AlwaysBreak
+BreakConstructorInitializers: AfterColon
+ConstructorInitializerAllOnOneLineOrOnePerLine: true
+PointerAlignment: Middle
+SortIncludes: false
+SpacesBeforeTrailingComments: 2
+UseTab: Always
+MaxEmptyLinesToKeep: 5
+
+TabWidth: 4
+ConstructorInitializerIndentWidth: 4
+ContinuationIndentWidth: 4
+IndentWidth: 4
+IndentCaseLabels: true
+
+BreakBeforeBraces: Custom
+BraceWrapping:
+  AfterClass: true
+  AfterControlStatement: true
+  AfterEnum: true
+  AfterExternBlock: true
+  AfterFunction: true
+  AfterNamespace: true
+  AfterStruct: true
+  AfterUnion: true
+  BeforeCatch: true
+  BeforeElse: true
+  IndentBraces: false
+  SplitEmptyFunction: true
+  SplitEmptyRecord: true
+
+# Always include globals first
+IncludeCategories:
+  - Regex: 'Globals.h'
+    Priority: -1

.clang-tidy

@@ -1,4 +1,13 @@
-Checks: '-*,readability-identifier-naming'
+Checks: >-
+  -*,
+  readability-identifier-naming,
+  readability-redundant-string-cstr,
+  readability-redundant-string-init,
+  readability-simplify-boolean-expr,
+  performance-unnecessary-value-param,
+  performance-unnecessary-copy-initialization,
+  performance-for-range-copy,
+  performance-implicit-conversion-in-loop,
 CheckOptions:
 - key:             readability-identifier-naming.PrivateMemberPrefix
   value:           'm_'
@@ -13,5 +22,9 @@ CheckOptions:
 #  value:           CamelCase
 #- key:             readability-identifier-naming.EnumCase
 #  value:           camelBack
+
+- key: performance-unnecessary-value-param.AllowedTypes
+  value: 'cEntityEffect;cNoise'
 WarningsAsErrors: '*'
 HeaderFilterRegex: '/cuberite/src/\.?[^\.]'
+FormatStyle: 'file'

clang-tidy.sh

@@ -6,10 +6,14 @@ FIXES_FILE="tidy-fixes.yaml"
 REGEX="/cuberite/src/\.?[^\.]"
 ARGS="-header-filter $REGEX -quiet -export-fixes $FIXES_FILE "$@" $REGEX"
 
+# Generate the compilation database
 mkdir -p tidy-build
 cd tidy-build
 cmake --target Cuberite -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..
 
+# Ensure LuaState_Typedefs.inc has been generated
+(cd ../src/Bindings && lua BindingsProcessor.lua)
+
 if run-clang-tidy $ARGS; then
 	echo "clang-tidy: No violations found"
 else

src/Bindings/LuaServerHandle.cpp

@@ -36,8 +36,8 @@ void cLuaServerHandle::SetServerHandle(cServerHandlePtr a_ServerHandle, cLuaServ
 {
 	ASSERT(m_ServerHandle == nullptr);  // The handle can be set only once
 
-	m_ServerHandle = a_ServerHandle;
-	m_Self = a_Self;
+	m_ServerHandle = std::move(a_ServerHandle);
+	m_Self = std::move(a_Self);
 }
 
 

src/Bindings/LuaState.cpp

@@ -2014,7 +2014,7 @@ bool cLuaState::CheckParamStaticSelf(const char * a_SelfClassName)
 
 
 
-bool cLuaState::IsParamUserType(int a_ParamIdx, AString a_UserType)
+bool cLuaState::IsParamUserType(int a_ParamIdx, const AString & a_UserType)
 {
 	ASSERT(IsValid());
 

src/Bindings/LuaState.h

@@ -815,7 +815,7 @@ public:
 	bool CheckParamStaticSelf(const char * a_SelfClassName);
 
 	/** Returns true if the specified parameter is of the specified class. */
-	bool IsParamUserType(int a_ParamIdx, AString a_UserType);
+	bool IsParamUserType(int a_ParamIdx, const AString & a_UserType);
 
 	/** Returns true if the specified parameter is a number. */
 	bool IsParamNumber(int a_ParamIdx);

src/Bindings/LuaUDPEndpoint.cpp

@@ -40,7 +40,7 @@ bool cLuaUDPEndpoint::Open(UInt16 a_Port, cLuaUDPEndpointPtr a_Self)
 	ASSERT(m_Self == nullptr);  // Must not be opened yet
 	ASSERT(m_Endpoint == nullptr);
 
-	m_Self = a_Self;
+	m_Self = std::move(a_Self);
 	m_Endpoint = cNetwork::CreateUDPEndpoint(a_Port, *this);
 	return m_Endpoint->IsOpen();
 }

src/Bindings/ManualBindings.cpp

@@ -409,7 +409,7 @@ static int tolua_LOG(lua_State * tolua_S)
 	}
 
 	// Log the message:
-	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S).c_str(), LogLevel);
+	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S), LogLevel);
 	return 0;
 }
 
@@ -427,7 +427,7 @@ static int tolua_LOGINFO(lua_State * tolua_S)
 		return 0;
 	}
 
-	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S).c_str(), cLogger::llInfo);
+	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S), cLogger::llInfo);
 	return 0;
 }
 
@@ -445,7 +445,7 @@ static int tolua_LOGWARN(lua_State * tolua_S)
 		return 0;
 	}
 
-	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S).c_str(), cLogger::llWarning);
+	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S), cLogger::llWarning);
 	return 0;
 }
 
@@ -463,7 +463,7 @@ static int tolua_LOGERROR(lua_State * tolua_S)
 		return 0;
 	}
 
-	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S).c_str(), cLogger::llError);
+	cLogger::GetInstance().LogSimple(GetLogMessage(tolua_S), cLogger::llError);
 	return 0;
 }
 

src/Bindings/PluginLua.cpp

@@ -108,7 +108,7 @@ bool cPluginLua::Load(void)
 	std::string PluginPath = GetLocalFolder() + "/";
 
 	// List all Lua files for this plugin. Info.lua has a special handling - make it the last to load:
-	AStringVector Files = cFile::GetFolderContents(PluginPath.c_str());
+	AStringVector Files = cFile::GetFolderContents(PluginPath);
 	AStringVector LuaFiles;
 	bool HasInfoLua = false;
 	for (AStringVector::const_iterator itr = Files.begin(), end = Files.end(); itr != end; ++itr)

src/Bindings/PluginManager.cpp

@@ -57,7 +57,7 @@ void cPluginManager::RefreshPluginList(void)
 {
 	// Get a list of currently available folders:
 	AString PluginsPath = GetPluginsPath() + "/";
-	AStringVector Contents = cFile::GetFolderContents(PluginsPath.c_str());
+	AStringVector Contents = cFile::GetFolderContents(PluginsPath);
 	AStringVector Folders;
 	for (auto & item: Contents)
 	{
@@ -1401,7 +1401,7 @@ bool cPluginManager::BindCommand(
 
 	auto & reg = m_Commands[a_Command];
 	reg.m_Plugin     = a_Plugin;
-	reg.m_Handler    = a_Handler;
+	reg.m_Handler    = std::move(a_Handler);
 	reg.m_Permission = a_Permission;
 	reg.m_HelpString = a_HelpString;
 	return true;
@@ -1508,7 +1508,7 @@ bool cPluginManager::BindConsoleCommand(
 
 	auto & reg = m_ConsoleCommands[a_Command];
 	reg.m_Plugin     = a_Plugin;
-	reg.m_Handler    = a_Handler;
+	reg.m_Handler    = std::move(a_Handler);
 	reg.m_Permission = "";
 	reg.m_HelpString = a_HelpString;
 	return true;
@@ -1739,7 +1739,7 @@ AStringVector cPluginManager::GetFoldersToLoad(cSettingsRepositoryInterface & a_
 	// Get the old format plugin list, and migrate it.
 	// Upgrade path added on 2020-03-27
 	auto OldValues = a_Settings.GetValues("Plugins");
-	for (auto NameValue : OldValues)
+	for (const auto & NameValue : OldValues)
 	{
 		AString ValueName = NameValue.first;
 		if (ValueName.compare("Plugin") == 0)
@@ -1759,7 +1759,7 @@ AStringVector cPluginManager::GetFoldersToLoad(cSettingsRepositoryInterface & a_
 
 	// Get the list of plugins to load:
 	auto Values = a_Settings.GetValues("Plugins");
-	for (auto NameValue : Values)
+	for (const auto & NameValue : Values)
 	{
 		AString Enabled = NameValue.second;
 		if (Enabled == "1")

src/BlockEntities/DropSpenserEntity.cpp

@@ -51,7 +51,6 @@ void cDropSpenserEntity::AddDropSpenserDir(Vector3i & a_RelCoord, NIBBLETYPE a_D
 		case E_META_DROPSPENSER_FACING_XP: a_RelCoord.x++; return;
 	}
 	LOGWARNING("%s: Unhandled direction: %d", __FUNCTION__, a_Direction);
-	return;
 }
 
 

src/BlockEntities/HopperEntity.cpp

@@ -575,11 +575,7 @@ bool cHopperEntity::MoveItemsToChest(cChunk & a_Chunk, Vector3i a_Coords)
 			FLOGWARNING("{0}: A chest entity was not found where expected, at {1} ({2}, {3}})", __FUNCTION__, a_Coords + ofs, ofs.x, ofs.z);
 			continue;
 		}
-		if (MoveItemsToGrid(*chest))
-		{
-			return true;
-		}
-		return false;
+		return MoveItemsToGrid(*chest);
 	}
 
 	// The chest was single and nothing could be moved

src/BlockEntities/MobSpawnerEntity.cpp

@@ -76,14 +76,7 @@ bool cMobSpawnerEntity::UsedBy(cPlayer * a_Player)
 
 void cMobSpawnerEntity::UpdateActiveState(void)
 {
-	if (GetNearbyPlayersNum() > 0)
-	{
-		m_IsActive = true;
-	}
-	else
-	{
-		m_IsActive = false;
-	}
+	m_IsActive = (GetNearbyPlayersNum() > 0);
 }
 
 

src/BlockTypeRegistry.cpp

@@ -17,7 +17,7 @@ BlockInfo::BlockInfo(
 ):
 	m_PluginName(aPluginName),
 	m_BlockTypeName(aBlockTypeName),
-	m_Handler(aHandler),
+	m_Handler(std::move(aHandler)),
 	m_Hints(aHints),
 	m_HintCallbacks(aHintCallbacks)
 {
@@ -94,7 +94,9 @@ void BlockTypeRegistry::registerBlockType(
 	const std::map<AString, BlockInfo::HintCallback> & aHintCallbacks
 )
 {
-	auto blockInfo = std::make_shared<BlockInfo>(aPluginName, aBlockTypeName, aHandler, aHints, aHintCallbacks);
+	auto blockInfo = std::make_shared<BlockInfo>(
+		aPluginName, aBlockTypeName, std::move(aHandler), aHints, aHintCallbacks
+	);
 
 	// Check previous registrations:
 	cCSLock lock(m_CSRegistry);
@@ -191,8 +193,8 @@ void BlockTypeRegistry::removeBlockTypeHint(
 // BlockTypeRegistry::AlreadyRegisteredException:
 
 BlockTypeRegistry::AlreadyRegisteredException::AlreadyRegisteredException(
-	std::shared_ptr<BlockInfo> aPreviousRegistration,
-	std::shared_ptr<BlockInfo> aNewRegistration
+	const std::shared_ptr<BlockInfo> & aPreviousRegistration,
+	const std::shared_ptr<BlockInfo> & aNewRegistration
 ) :
 	Super(message(aPreviousRegistration, aNewRegistration)),
 	m_PreviousRegistration(aPreviousRegistration),
@@ -205,8 +207,8 @@ BlockTypeRegistry::AlreadyRegisteredException::AlreadyRegisteredException(
 
 
 AString BlockTypeRegistry::AlreadyRegisteredException::message(
-	std::shared_ptr<BlockInfo> aPreviousRegistration,
-	std::shared_ptr<BlockInfo> aNewRegistration
+	const std::shared_ptr<BlockInfo> & aPreviousRegistration,
+	const std::shared_ptr<BlockInfo> & aNewRegistration
 )
 {
 	return Printf("Attempting to register BlockTypeName %s from plugin %s, while it is already registered in plugin %s",

src/BlockTypeRegistry.h

@@ -164,8 +164,8 @@ public:
 	/** Creates a new instance of the exception that provides info on both the original registration and the newly attempted
 	registration that caused the failure. */
 	AlreadyRegisteredException(
-		std::shared_ptr<BlockInfo> aPreviousRegistration,
-		std::shared_ptr<BlockInfo> aNewRegistration
+		const std::shared_ptr<BlockInfo> & aPreviousRegistration,
+		const std::shared_ptr<BlockInfo> & aNewRegistration
 	);
 
 	// Simple getters:
@@ -182,8 +182,8 @@ private:
 	/** Returns the general exception message formatted by the two registrations.
 	The output is used when logging. */
 	static AString message(
-		std::shared_ptr<BlockInfo> aPreviousRegistration,
-		std::shared_ptr<BlockInfo> aNewRegistration
+		const std::shared_ptr<BlockInfo> & aPreviousRegistration,
+		const std::shared_ptr<BlockInfo> & aNewRegistration
 	);
 };
 

src/Blocks/BlockBed.cpp

@@ -128,11 +128,7 @@ bool cBlockBedHandler::OnUse(
 	// Fast-forward the time if all players in the world are in their beds:
 	auto TimeFastForwardTester = [](cPlayer & a_OtherPlayer)
 	{
-		if (!a_OtherPlayer.IsInBed())
-		{
-			return true;
-		}
-		return false;
+		return !a_OtherPlayer.IsInBed();
 	};
 	if (a_WorldInterface.ForEachPlayer(TimeFastForwardTester))
 	{

src/Blocks/BlockFire.h

@@ -167,8 +167,6 @@ public:
 				}
 			}
 		}
-
-		return;
 	}
 
 	/** Evaluates if coordinates are a portal going XP / XM; returns true if so, and writes boundaries to variable

src/BoundingBox.cpp

@@ -285,11 +285,7 @@ bool cBoundingBox::Intersect(const cBoundingBox & a_Other, cBoundingBox & a_Inte
 	}
 	a_Intersection.m_Min.z = std::max(m_Min.z, a_Other.m_Min.z);
 	a_Intersection.m_Max.z = std::min(m_Max.z, a_Other.m_Max.z);
-	if (a_Intersection.m_Min.z >= a_Intersection.m_Max.z)
-	{
-		return false;
-	}
-	return true;
+	return (a_Intersection.m_Min.z < a_Intersection.m_Max.z);
 }
 
 

src/BrewingRecipes.cpp

@@ -75,7 +75,7 @@ void cBrewingRecipes::AddRecipeFromLine(AString a_Line, unsigned int a_LineNum)
 		return;
 	}
 
-	const AStringVector & IngredientAndOutput = StringSplit(InputAndIngredient[1].c_str(), "=");
+	const AStringVector & IngredientAndOutput = StringSplit(InputAndIngredient[1], "=");
 	if (IngredientAndOutput.size() != 2)
 	{
 		LOGWARNING("brewing.txt: line %d: A line with '=' was expected", a_LineNum);

src/Chunk.cpp

@@ -1749,8 +1749,6 @@ void cChunk::RemoveClient(cClientHandle * a_Client)
 			a_Client->SendDestroyEntity(*Entity);
 		}
 	}
-
-	return;
 }
 
 

src/ClientHandle.cpp

@@ -213,7 +213,10 @@ void cClientHandle::GenerateOfflineUUID(void)
 
 
 
-AString cClientHandle::FormatChatPrefix(bool ShouldAppendChatPrefixes, AString a_ChatPrefixS, AString m_Color1, AString m_Color2)
+AString cClientHandle::FormatChatPrefix(
+	bool ShouldAppendChatPrefixes, const AString & a_ChatPrefixS,
+	const AString & m_Color1, const AString & m_Color2
+)
 {
 	if (ShouldAppendChatPrefixes)
 	{
@@ -2075,12 +2078,12 @@ bool cClientHandle::CheckBlockInteractionsRate(void)
 	ASSERT(m_Player != nullptr);
 	ASSERT(m_Player->GetWorld() != nullptr);
 
-	if ((cRoot::Get()->GetServer()->ShouldLimitPlayerBlockChanges()) && (m_NumBlockChangeInteractionsThisTick > MAX_BLOCK_CHANGE_INTERACTIONS))
+	if (!cRoot::Get()->GetServer()->ShouldLimitPlayerBlockChanges())
 	{
-		return false;
+		return true;
 	}
 
-	return true;
+	return (m_NumBlockChangeInteractionsThisTick <= MAX_BLOCK_CHANGE_INTERACTIONS);
 }
 
 
@@ -3310,7 +3313,7 @@ void cClientHandle::SocketClosed(void)
 void cClientHandle::SetSelf(cClientHandlePtr a_Self)
 {
 	ASSERT(m_Self == nullptr);
-	m_Self = a_Self;
+	m_Self = std::move(a_Self);
 }
 
 

src/ClientHandle.h

@@ -107,7 +107,10 @@ public:  // tolua_export
 	/** Formats the type of message with the proper color and prefix for sending to the client. */
 	static AString FormatMessageType(bool ShouldAppendChatPrefixes, eMessageType a_ChatPrefix, const AString & a_AdditionalData);
 
-	static AString FormatChatPrefix(bool ShouldAppendChatPrefixes, AString a_ChatPrefixS, AString m_Color1, AString m_Color2);
+	static AString FormatChatPrefix(
+		bool ShouldAppendChatPrefixes, const AString & a_ChatPrefixS,
+		const AString & m_Color1, const AString & m_Color2
+	);
 
 	void Kick(const AString & a_Reason);  // tolua_export
 

src/CraftingRecipes.cpp

@@ -400,7 +400,7 @@ void cCraftingRecipes::AddRecipeLine(int a_LineNum, const AString & a_RecipeLine
 	}
 	if (ResultSplit.size() > 1)
 	{
-		if (!StringToInteger<char>(ResultSplit[1].c_str(), Recipe->m_Result.m_ItemCount))
+		if (!StringToInteger<char>(ResultSplit[1], Recipe->m_Result.m_ItemCount))
 		{
 			LOGWARNING("crafting.txt: line %d: Cannot parse result count, ignoring the recipe.", a_LineNum);
 			LOGINFO("Offending line: \"%s\"", a_RecipeLine.c_str());
@@ -452,7 +452,7 @@ bool cCraftingRecipes::ParseItem(const AString & a_String, cItem & a_Item)
 	if (Split.size() > 1)
 	{
 		AString Damage = TrimString(Split[1]);
-		if (!StringToInteger<short>(Damage.c_str(), a_Item.m_ItemDamage))
+		if (!StringToInteger<short>(Damage, a_Item.m_ItemDamage))
 		{
 			// Parsing the number failed
 			return false;
@@ -893,7 +893,7 @@ void cCraftingRecipes::HandleDyedLeather(const cItem * a_CraftingGrid, cCrafting
 			for (int y = 0; y < a_GridHeight; ++y)
 			{
 				int GridIdx = x + a_GridStride * y;
-				if ((a_CraftingGrid[GridIdx].m_ItemType == result_type) && (found == false))
+				if ((a_CraftingGrid[GridIdx].m_ItemType == result_type) && !found)
 				{
 					found = true;
 					temp = a_CraftingGrid[GridIdx].CopyOne();

src/Defines.cpp

@@ -160,17 +160,10 @@ AString BlockFaceToString(eBlockFace a_BlockFace)
 
 bool IsValidBlock(int a_BlockType)
 {
-	if (
-		(
-		(a_BlockType > -1) &&
-		(a_BlockType <= E_BLOCK_MAX_TYPE_ID)
-		) ||
+	return (
+		((a_BlockType > -1) && (a_BlockType <= E_BLOCK_MAX_TYPE_ID)) ||
 		(a_BlockType == 255)  // the blocks 253-254 don't exist yet -> https://minecraft.gamepedia.com/Data_values#Block_IDs
-	)
-	{
-		return true;
-	}
-	return false;
+	);
 }
 
 

src/Enchantments.cpp

@@ -304,7 +304,7 @@ bool cEnchantments::CanAddEnchantment(int a_EnchantmentID) const
 		// {enchInfinity, enchMending}
 	};
 
-	for (auto excl: IncompatibleEnchantments)
+	for (const auto & excl: IncompatibleEnchantments)
 	{
 		if (excl.count(a_EnchantmentID) != 0)
 		{
@@ -1112,7 +1112,9 @@ void cEnchantments::RemoveEnchantmentWeightFromVector(cWeightedEnchantments & a_
 
 
 
-void cEnchantments::CheckEnchantmentConflictsFromVector(cWeightedEnchantments & a_Enchantments, cEnchantments a_FirstEnchantment)
+void cEnchantments::CheckEnchantmentConflictsFromVector(
+	cWeightedEnchantments & a_Enchantments, const cEnchantments & a_FirstEnchantment
+)
 {
 	if (a_FirstEnchantment.GetLevel(cEnchantments::enchProtection) > 0)
 	{

src/Enchantments.h

@@ -137,7 +137,7 @@ public:
 	static void RemoveEnchantmentWeightFromVector(cWeightedEnchantments & a_Enchantments, const cEnchantments & a_Enchantment);
 
 	/** Check enchantment conflicts from enchantments from the vector */
-	static void CheckEnchantmentConflictsFromVector(cWeightedEnchantments & a_Enchantments, cEnchantments a_FirstEnchantment);
+	static void CheckEnchantmentConflictsFromVector(cWeightedEnchantments & a_Enchantments, const cEnchantments & a_FirstEnchantment);
 
 	/** Gets random enchantment from Vector and returns it */
 	static cEnchantments GetRandomEnchantmentFromVector(cWeightedEnchantments & a_Enchantments);

src/Entities/Entity.cpp

@@ -617,7 +617,6 @@ int cEntity::GetRawDamageAgainst(const cEntity & a_Receiver)
 void cEntity::ApplyArmorDamage(int DamageBlocked)
 {
 	// cEntities don't necessarily have armor to damage.
-	return;
 }
 
 
@@ -2041,11 +2040,10 @@ bool cEntity::IsA(const char * a_ClassName) const
 
 bool cEntity::IsAttachedTo(const cEntity * a_Entity) const
 {
-	if ((m_AttachedTo != nullptr) && (a_Entity->GetUniqueID() == m_AttachedTo->GetUniqueID()))
-	{
-		return true;
-	}
-	return false;
+	return (
+		(m_AttachedTo != nullptr) &&
+		(a_Entity->GetUniqueID() == m_AttachedTo->GetUniqueID())
+	);
 }
 
 

src/Entities/LeashKnot.cpp

@@ -85,7 +85,6 @@ void cLeashKnot::KilledBy(TakeDamageInfo & a_TDI)
 	Super::KilledBy(a_TDI);
 	m_World->BroadcastSoundEffect("entity.leashknot.break", GetPosition(), 1, 1);
 	Destroy();
-	return;
 }
 
 

src/Entities/Player.cpp

@@ -86,7 +86,7 @@ const int cPlayer::EATING_TICKS = 30;
 
 
 
-cPlayer::cPlayer(cClientHandlePtr a_Client, const AString & a_PlayerName) :
+cPlayer::cPlayer(const cClientHandlePtr & a_Client, const AString & a_PlayerName) :
 	Super(etPlayer, 0.6, 1.8),
 	m_bVisible(true),
 	m_FoodLevel(MAX_FOOD_LEVEL),
@@ -1869,14 +1869,8 @@ bool cPlayer::PermissionMatches(const AStringVector & a_Permission, const AStrin
 	}
 
 	// So far all the sub-items have matched
-	// If the sub-item count is the same, then the permission matches:
-	if (lenP == lenT)
-	{
-		return true;
-	}
-
-	// There are more sub-items in either the permission or the template, not a match:
-	return false;
+	// If the sub-item count is the same, then the permission matches
+	return (lenP == lenT);
 }
 
 

src/Entities/Player.h

@@ -48,7 +48,7 @@ public:
 	CLASS_PROTODEF(cPlayer)
 
 
-	cPlayer(cClientHandlePtr a_Client, const AString & a_PlayerName);
+	cPlayer(const cClientHandlePtr & a_Client, const AString & a_PlayerName);
 
 	virtual bool Initialize(OwnedEntity a_Self, cWorld & a_World) override;
 

src/FurnaceRecipe.cpp

@@ -233,7 +233,7 @@ bool cFurnaceRecipe::ParseItem(const AString & a_String, cItem & a_Item)
 
 	if (SplitAmount.size() > 1)
 	{
-		if (!StringToInteger<char>(SplitAmount[1].c_str(), a_Item.m_ItemCount))
+		if (!StringToInteger<char>(SplitAmount[1], a_Item.m_ItemCount))
 		{
 			return false;
 		}
@@ -241,7 +241,7 @@ bool cFurnaceRecipe::ParseItem(const AString & a_String, cItem & a_Item)
 
 	if (SplitMeta.size() > 1)
 	{
-		if (!StringToInteger<short>(SplitMeta[1].c_str(), a_Item.m_ItemDamage))
+		if (!StringToInteger<short>(SplitMeta[1], a_Item.m_ItemDamage))
 		{
 			return false;
 		}

src/Generating/BioGen.cpp

@@ -58,7 +58,7 @@ void cBioGenConstant::InitializeBiomeGen(cIniFile & a_IniFile)
 // cBioGenCache:
 
 cBioGenCache::cBioGenCache(cBiomeGenPtr a_BioGenToCache, size_t a_CacheSize) :
-	m_BioGenToCache(a_BioGenToCache),
+	m_BioGenToCache(std::move(a_BioGenToCache)),
 	m_CacheSize(a_CacheSize),
 	m_NumHits(0),
 	m_NumMisses(0),
@@ -139,7 +139,7 @@ void cBioGenCache::InitializeBiomeGen(cIniFile & a_IniFile)
 ////////////////////////////////////////////////////////////////////////////////
 // cBioGenMulticache:
 
-cBioGenMulticache::cBioGenMulticache(cBiomeGenPtr a_BioGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches) :
+cBioGenMulticache::cBioGenMulticache(const cBiomeGenPtr & a_BioGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches) :
 	m_NumSubCaches(a_NumSubCaches)
 {
 	m_Caches.reserve(a_NumSubCaches);
@@ -167,7 +167,7 @@ void cBioGenMulticache::GenBiomes(cChunkCoords a_ChunkCoords, cChunkDef::BiomeMa
 
 void cBioGenMulticache::InitializeBiomeGen(cIniFile & a_IniFile)
 {
-	for (auto itr : m_Caches)
+	for (const auto & itr : m_Caches)
 	{
 		itr->InitializeBiomeGen(a_IniFile);
 	}

src/Generating/BioGen.h

@@ -97,7 +97,7 @@ public:
 	This allows us to use shorter cache depths with faster lookups for more covered area. (#381)
 	a_SubCacheSize defines the size of each sub-cache
 	a_NumSubCaches defines how many sub-caches are used for the multicache. */
-	cBioGenMulticache(cBiomeGenPtr a_BioGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches);
+	cBioGenMulticache(const cBiomeGenPtr & a_BioGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches);
 
 protected:
 	typedef std::vector<cBiomeGenPtr> cBiomeGenPtrs;

src/Generating/CompoGen.cpp

@@ -330,7 +330,7 @@ void cCompoGenNether::InitializeCompoGen(cIniFile & a_IniFile)
 // cCompoGenCache:
 
 cCompoGenCache::cCompoGenCache(cTerrainCompositionGenPtr a_Underlying, int a_CacheSize) :
-	m_Underlying(a_Underlying),
+	m_Underlying(std::move(a_Underlying)),
 	m_CacheSize(a_CacheSize),
 	m_CacheOrder(new int[ToUnsigned(a_CacheSize)]),
 	m_CacheData(new sCacheData[ToUnsigned(a_CacheSize)]),

src/Generating/ComposableGenerator.cpp

@@ -39,8 +39,8 @@
 
 cTerrainCompositionGenPtr cTerrainCompositionGen::CreateCompositionGen(
 	cIniFile & a_IniFile,
-	cBiomeGenPtr a_BiomeGen,
-	cTerrainShapeGenPtr a_ShapeGen,
+	const cBiomeGenPtr & a_BiomeGen,
+	const cTerrainShapeGenPtr & a_ShapeGen,
 	int a_Seed
 )
 {

src/Generating/ComposableGenerator.h

@@ -98,7 +98,7 @@ public:
 	*/
 	static cTerrainShapeGenPtr CreateShapeGen(
 		cIniFile & a_IniFile,
-		cBiomeGenPtr a_BiomeGen,
+		const cBiomeGenPtr & a_BiomeGen,
 		int a_Seed,
 		bool & a_CacheOffByDefault
 	);
@@ -137,7 +137,7 @@ public:
 	/** Creates a cTerrainHeightGen descendant based on the INI file settings. */
 	static cTerrainHeightGenPtr CreateHeightGen(
 		cIniFile & a_IniFile,
-		cBiomeGenPtr a_BiomeGen,
+		const cBiomeGenPtr & a_BiomeGen,
 		int a_Seed,
 		bool & a_CacheOffByDefault
 	);
@@ -169,8 +169,8 @@ public:
 	a_ShapeGen is the underlying shape generator, some composition generators may depend on it providing additional shape around the chunk. */
 	static cTerrainCompositionGenPtr CreateCompositionGen(
 		cIniFile & a_IniFile,
-		cBiomeGenPtr a_BiomeGen,
-		cTerrainShapeGenPtr a_ShapeGen,
+		const cBiomeGenPtr & a_BiomeGen,
+		const cTerrainShapeGenPtr & a_ShapeGen,
 		int a_Seed
 	);
 } ;

src/Generating/CompositedHeiGen.h

@@ -21,9 +21,9 @@ class cCompositedHeiGen:
 {
 public:
 	cCompositedHeiGen(cBiomeGenPtr a_BiomeGen, cTerrainShapeGenPtr a_ShapeGen, cTerrainCompositionGenPtr a_CompositionGen):
-		m_BiomeGen(a_BiomeGen),
-		m_ShapeGen(a_ShapeGen),
-		m_CompositionGen(a_CompositionGen)
+		m_BiomeGen(std::move(a_BiomeGen)),
+		m_ShapeGen(std::move(a_ShapeGen)),
+		m_CompositionGen(std::move(a_CompositionGen))
 	{
 	}
 

src/Generating/DistortedHeightmap.cpp

@@ -118,7 +118,7 @@ const cDistortedHeightmap::sGenParam cDistortedHeightmap::m_GenParam[256] =
 
 
 
-cDistortedHeightmap::cDistortedHeightmap(int a_Seed, cBiomeGenPtr a_BiomeGen) :
+cDistortedHeightmap::cDistortedHeightmap(int a_Seed, const cBiomeGenPtr & a_BiomeGen) :
 	m_NoiseDistortX(a_Seed + 1000),
 	m_NoiseDistortZ(a_Seed + 2000),
 	m_CurChunkCoords(0x7fffffff, 0x7fffffff),  // Set impossible coords for the chunk so that it's always considered stale

src/Generating/DistortedHeightmap.h

@@ -26,7 +26,7 @@ class cDistortedHeightmap :
 	public cTerrainShapeGen
 {
 public:
-	cDistortedHeightmap(int a_Seed, cBiomeGenPtr a_BiomeGen);
+	cDistortedHeightmap(int a_Seed, const cBiomeGenPtr & a_BiomeGen);
 
 protected:
 	typedef cChunkDef::BiomeMap BiomeNeighbors[3][3];

src/Generating/DungeonRoomsFinisher.cpp

@@ -289,7 +289,7 @@ protected:
 
 cDungeonRoomsFinisher::cDungeonRoomsFinisher(cTerrainShapeGenPtr a_ShapeGen, int a_Seed, int a_GridSize, int a_MaxSize, int a_MinSize, const AString & a_HeightDistrib) :
 	Super(a_Seed + 100, a_GridSize, a_GridSize, a_GridSize, a_GridSize, a_MaxSize, a_MaxSize, 1024),
-	m_ShapeGen(a_ShapeGen),
+	m_ShapeGen(std::move(a_ShapeGen)),
 	m_MaxHalfSize((a_MaxSize + 1) / 2),
 	m_MinHalfSize((a_MinSize + 1) / 2),
 	m_HeightProbability(cChunkDef::Height)

src/Generating/FinishGen.cpp

@@ -275,7 +275,7 @@ void cFinishGenClumpTopBlock::TryPlaceFoliageClump(cChunkDesc & a_ChunkDesc, int
 
 
 
-void cFinishGenClumpTopBlock::ParseConfigurationString(AString a_RawClumpInfo, std::vector<BiomeInfo> & a_Output)
+void cFinishGenClumpTopBlock::ParseConfigurationString(const AString & a_RawClumpInfo, std::vector<BiomeInfo> & a_Output)
 {
 	// Initialize the vector for all biomes.
 	for (int i = static_cast<int>(a_Output.size()); i < static_cast<int>(biMaxVariantBiome); i++)

src/Generating/FinishGen.h

@@ -104,7 +104,7 @@ public:
 		BiomeInfo(int a_MinNumClumpsPerChunk, int a_MaxNumClumpsPerChunk, std::vector<FoliageInfo> a_Blocks) :
 			m_MinNumClumpsPerChunk(a_MinNumClumpsPerChunk),
 			m_MaxNumClumpsPerChunk(a_MaxNumClumpsPerChunk),
-			m_Blocks(a_Blocks)
+			m_Blocks(std::move(a_Blocks))
 		{}
 	};
 
@@ -118,7 +118,7 @@ public:
 
 	/** Parses a string and puts a vector with a length of biMaxVariantBiome in a_Output.
 	The format of the string is "<Biomes separated with a comma>;<Blocks separated with a comma>". This can also be repeated with a | */
-	static void ParseConfigurationString(AString a_String, std::vector<BiomeInfo> & a_Output);
+	static void ParseConfigurationString(const AString & a_String, std::vector<BiomeInfo> & a_Output);
 
 	/** Parses an inifile in search for all clumps */
 	static std::vector<BiomeInfo> ParseIniFile(cIniFile & a_IniFile, AString a_ClumpPrefix);

src/Generating/HeiGen.cpp

@@ -107,7 +107,7 @@ void cHeiGenFlat::InitializeHeightGen(cIniFile & a_IniFile)
 // cHeiGenCache:
 
 cHeiGenCache::cHeiGenCache(cTerrainHeightGenPtr a_HeiGenToCache, size_t a_CacheSize) :
-	m_HeiGenToCache(a_HeiGenToCache),
+	m_HeiGenToCache(std::move(a_HeiGenToCache)),
 	m_CacheSize(a_CacheSize),
 	m_NumHits(0),
 	m_NumMisses(0),
@@ -219,7 +219,7 @@ bool cHeiGenCache::GetHeightAt(int a_ChunkX, int a_ChunkZ, int a_RelX, int a_Rel
 ////////////////////////////////////////////////////////////////////////////////
 // cHeiGenMultiCache:
 
-cHeiGenMultiCache::cHeiGenMultiCache(cTerrainHeightGenPtr a_HeiGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches):
+cHeiGenMultiCache::cHeiGenMultiCache(const cTerrainHeightGenPtr & a_HeiGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches):
 	m_NumSubCaches(a_NumSubCaches)
 {
 	// Create the individual sub-caches:
@@ -638,7 +638,7 @@ public:
 
 	cHeiGenMinMax(int a_Seed, cBiomeGenPtr a_BiomeGen):
 		m_Noise(a_Seed),
-		m_BiomeGen(a_BiomeGen),
+		m_BiomeGen(std::move(a_BiomeGen)),
 		m_TotalWeight(0)
 	{
 		// Initialize the weights:
@@ -831,7 +831,7 @@ protected:
 ////////////////////////////////////////////////////////////////////////////////
 // cTerrainHeightGen:
 
-cTerrainHeightGenPtr cTerrainHeightGen::CreateHeightGen(cIniFile & a_IniFile, cBiomeGenPtr a_BiomeGen, int a_Seed, bool & a_CacheOffByDefault)
+cTerrainHeightGenPtr cTerrainHeightGen::CreateHeightGen(cIniFile & a_IniFile, const cBiomeGenPtr & a_BiomeGen, int a_Seed, bool & a_CacheOffByDefault)
 {
 	AString HeightGenName = a_IniFile.GetValueSet("Generator", "HeightGen", "");
 	if (HeightGenName.empty())

src/Generating/HeiGen.h

@@ -73,7 +73,7 @@ class cHeiGenMultiCache:
 	public cTerrainHeightGen
 {
 public:
-	cHeiGenMultiCache(cTerrainHeightGenPtr a_HeightGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches);
+	cHeiGenMultiCache(const cTerrainHeightGenPtr & a_HeightGenToCache, size_t a_SubCacheSize, size_t a_NumSubCaches);
 
 	// cTerrainHeightGen overrides:
 	virtual void GenHeightMap(cChunkCoords a_ChunkCoords, cChunkDef::HeightMap & a_HeightMap) override;
@@ -176,7 +176,7 @@ public:
 
 	cHeiGenBiomal(int a_Seed, cBiomeGenPtr a_BiomeGen):
 		m_Noise(a_Seed),
-		m_BiomeGen(a_BiomeGen)
+		m_BiomeGen(std::move(a_BiomeGen))
 	{
 	}
 

src/Generating/Noise3DGenerator.cpp

@@ -511,7 +511,7 @@ cBiomalNoise3DComposable::cBiomalNoise3DComposable(int a_Seed, cBiomeGenPtr a_Bi
 	m_DensityNoiseA(a_Seed + 1),
 	m_DensityNoiseB(a_Seed + 2),
 	m_BaseNoise(a_Seed + 3),
-	m_BiomeGen(a_BiomeGen),
+	m_BiomeGen(std::move(a_BiomeGen)),
 	m_LastChunkCoords(0x7fffffff, 0x7fffffff)  // Set impossible coords for the chunk so that it's always considered stale
 {
 	// Generate the weight distribution for summing up neighboring biomes:

src/Generating/PiecePool.h

@@ -183,7 +183,7 @@ public:
 
 	void SetVerticalStrategy(cVerticalStrategyPtr a_VerticalStrategy)
 	{
-		m_VerticalStrategy = a_VerticalStrategy;
+		m_VerticalStrategy = std::move(a_VerticalStrategy);
 	}
 
 	cVerticalStrategyPtr GetVerticalStrategy(void) const

src/Generating/PieceStructuresGen.cpp

@@ -23,8 +23,8 @@ public:
 
 	cGen(int a_Seed, cBiomeGenPtr a_BiomeGen, cTerrainHeightGenPtr a_HeightGen, int a_SeaLevel, const AString & a_Name):
 		Super(a_Seed),
-		m_BiomeGen(a_BiomeGen),
-		m_HeightGen(a_HeightGen),
+		m_BiomeGen(std::move(a_BiomeGen)),
+		m_HeightGen(std::move(a_HeightGen)),
 		m_SeaLevel(a_SeaLevel),
 		m_Name(a_Name),
 		m_MaxDepth(5)
@@ -131,7 +131,7 @@ cPieceStructuresGen::cPieceStructuresGen(int a_Seed):
 
 
 
-bool cPieceStructuresGen::Initialize(const AString & a_Prefabs, int a_SeaLevel, cBiomeGenPtr a_BiomeGen, cTerrainHeightGenPtr a_HeightGen)
+bool cPieceStructuresGen::Initialize(const AString & a_Prefabs, int a_SeaLevel, const cBiomeGenPtr & a_BiomeGen, const cTerrainHeightGenPtr & a_HeightGen)
 {
 	// Load each piecepool:
 	auto structures = StringSplitAndTrim(a_Prefabs, "|");

src/Generating/PieceStructuresGen.h

@@ -34,7 +34,7 @@ public:
 	a_Prefabs contains the list of prefab sets that should be activated, "|"-separated.
 	All problems are logged to the console and the generator skips over them.
 	Returns true if at least one prefab set is valid (the generator should be kept). */
-	bool Initialize(const AString & a_Prefabs, int a_SeaLevel, cBiomeGenPtr a_BiomeGen, cTerrainHeightGenPtr a_HeightGen);
+	bool Initialize(const AString & a_Prefabs, int a_SeaLevel, const cBiomeGenPtr & a_BiomeGen, const cTerrainHeightGenPtr & a_HeightGen);
 
 	// cFinishGen override:
 	virtual void GenFinish(cChunkDesc & a_ChunkDesc) override;

src/Generating/PrefabStructure.cpp

@@ -19,7 +19,7 @@ cPrefabStructure::cPrefabStructure(
 ):
 	Super(a_GridX, a_GridZ, a_OriginX, a_OriginZ),
 	m_Pieces(std::move(a_Pieces)),
-	m_HeightGen(a_HeightGen)
+	m_HeightGen(std::move(a_HeightGen))
 {
 }
 

src/Generating/ProtIntGen.h

@@ -198,7 +198,7 @@ public:
 
 	cProtIntGenZoom(int a_Seed, Underlying a_UnderlyingGen):
 		Super(a_Seed),
-		m_UnderlyingGen(a_UnderlyingGen)
+		m_UnderlyingGen(std::move(a_UnderlyingGen))
 	{
 	}
 
@@ -269,7 +269,7 @@ public:
 
 	cProtIntGenSmooth(int a_Seed, Underlying a_Underlying):
 		Super(a_Seed),
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -342,7 +342,7 @@ class cProtIntGenAvgValues:
 public:
 
 	cProtIntGenAvgValues(Underlying a_Underlying):
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -387,7 +387,7 @@ class cProtIntGenAvg4Values:
 public:
 
 	cProtIntGenAvg4Values(Underlying a_Underlying):
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -438,7 +438,7 @@ class cProtIntGenWeightAvg:
 public:
 
 	cProtIntGenWeightAvg(Underlying a_Underlying):
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -490,7 +490,7 @@ public:
 		m_ChancePct(a_ChancePct),
 		m_Min(a_Min),
 		m_Range(a_Range),
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -537,7 +537,7 @@ public:
 		Super(a_Seed),
 		m_Range(a_HalfRange * 2 + 1),
 		m_HalfRange(a_HalfRange),
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -580,7 +580,7 @@ public:
 	cProtIntGenRndAvg(int a_Seed, int a_AvgChancePct, Underlying a_Underlying):
 		Super(a_Seed),
 		m_AvgChancePct(a_AvgChancePct),
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -638,7 +638,7 @@ public:
 	cProtIntGenRndBetween(int a_Seed, int a_AvgChancePct, Underlying a_Underlying):
 		Super(a_Seed),
 		m_AvgChancePct(a_AvgChancePct),
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -693,7 +693,7 @@ class cProtIntGenBeaches:
 public:
 
 	cProtIntGenBeaches(Underlying a_Underlying):
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -798,7 +798,7 @@ public:
 	cProtIntGenAddIslands(int a_Seed, int a_Chance, Underlying a_Underlying):
 		Super(a_Seed),
 		m_Chance(a_Chance),
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -842,7 +842,7 @@ class cProtIntGenBiomeGroupEdges:
 public:
 
 	cProtIntGenBiomeGroupEdges(Underlying a_Underlying):
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -941,7 +941,7 @@ public:
 
 	cProtIntGenBiomes(int a_Seed, Underlying a_Underlying):
 		Super(a_Seed),
-		m_Underlying(a_Underlying)
+		m_Underlying(std::move(a_Underlying))
 	{
 	}
 
@@ -1072,7 +1072,7 @@ public:
 		m_From(a_From),