Minor cBrewingRecipes cleanup (#3731)

src/Bindings/ManualBindings.cpp

@@ -3087,7 +3087,8 @@ static int tolua_cRoot_GetBrewingRecipe(lua_State * tolua_S)
 	}
 
 	// Push the output item
-	L.Push(Recipe->Output.get());
+	cItem & OutItem = const_cast<cItem &>(Recipe->Output);
+	L.Push(&OutItem);
 	return 1;
 }
 

src/BlockEntities/BrewingstandEntity.cpp

@@ -141,7 +141,7 @@ bool cBrewingstandEntity::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
 			}
 
 			Recipe = m_CurrentBrewingRecipes[i];
-			m_Contents.SetSlot(i, Recipe->Output->CopyOne());
+			m_Contents.SetSlot(i, Recipe->Output.CopyOne());
 		}
 
 		// Brewing process completed
@@ -243,7 +243,7 @@ void cBrewingstandEntity::OnSlotChanged(cItemGrid * a_ItemGrid, int a_SlotNum)
 		if (m_CurrentBrewingRecipes[i] != nullptr)
 		{
 			Recipe = m_CurrentBrewingRecipes[i];
-			if (Recipe->Ingredient->IsEqual(GetSlot(bsIngredient)) && Recipe->Input->IsEqual(GetSlot(i)))
+			if (Recipe->Ingredient.IsEqual(GetSlot(bsIngredient)) && Recipe->Input.IsEqual(GetSlot(i)))
 			{
 				Stop = false;
 				continue;
@@ -255,7 +255,7 @@ void cBrewingstandEntity::OnSlotChanged(cItemGrid * a_ItemGrid, int a_SlotNum)
 		{
 			// Found a brewing recipe for the items
 			m_CurrentBrewingRecipes[i] = Recipe;
-			m_Results[i] = Recipe->Output->CopyOne();
+			m_Results[i] = Recipe->Output.CopyOne();
 			Stop = false;
 		}
 	}
@@ -330,7 +330,7 @@ void cBrewingstandEntity::LoadRecipes(void)
 		if (Recipe != nullptr)
 		{
 			m_CurrentBrewingRecipes[i] = Recipe;
-			m_Results[i] = Recipe->Output->CopyOne();
+			m_Results[i] = Recipe->Output.CopyOne();
 		}
 	}
 }

src/BrewingRecipes.cpp

@@ -2,7 +2,6 @@
 #include "Globals.h"  // NOTE: MSVC stupidness requires this to be the same across all modules
 
 #include "BrewingRecipes.h"
-#include "Item.h"
 
 #include <fstream>
 
@@ -12,23 +11,10 @@
 
 
 
-typedef std::vector<std::unique_ptr<cBrewingRecipes::cRecipe>> RecipeList;
-
-
-
-
-
-struct cBrewingRecipes::sBrewingRecipeState
-{
-	RecipeList Recipes;
-};
-
-
 
 
 
 cBrewingRecipes::cBrewingRecipes()
-	: m_pState(new sBrewingRecipeState)
 {
 	ReloadRecipes();
 }
@@ -37,15 +23,6 @@ cBrewingRecipes::cBrewingRecipes()
 
 
 
-cBrewingRecipes::~cBrewingRecipes()
-{
-	ClearRecipes();
-}
-
-
-
-
-
 void cBrewingRecipes::ReloadRecipes(void)
 {
 	ClearRecipes();
@@ -68,7 +45,7 @@ void cBrewingRecipes::ReloadRecipes(void)
 		size_t FirstCommentSymbol = ParsingLine.find('#');
 		if (FirstCommentSymbol != AString::npos)
 		{
-			ParsingLine.erase(ParsingLine.begin() += static_cast<long>(FirstCommentSymbol), ParsingLine.end());
+			ParsingLine.erase(FirstCommentSymbol);
 		}
 
 		if (ParsingLine.empty())
@@ -78,26 +55,20 @@ void cBrewingRecipes::ReloadRecipes(void)
 		AddRecipeFromLine(ParsingLine, LineNum);
 	}  // while (getline(ParsingLine))
 
-	LOG("Loaded " SIZE_T_FMT " brewing recipes", m_pState->Recipes.size());
+	LOG("Loaded " SIZE_T_FMT " brewing recipes", m_Recipes.size());
 }
 
 
 
 
 
-void cBrewingRecipes::AddRecipeFromLine(const AString & a_Line, unsigned int a_LineNum)
+void cBrewingRecipes::AddRecipeFromLine(AString a_Line, unsigned int a_LineNum)
 {
-	AString Line(a_Line);
+	a_Line.erase(std::remove_if(a_Line.begin(), a_Line.end(), isspace), a_Line.end());
-	Line.erase(std::remove_if(Line.begin(), Line.end(), isspace), Line.end());
 
-	short InputDamage;
+	auto Recipe = cpp14::make_unique<cRecipe>();
-	short OutputDamage;
 
-	std::unique_ptr<cItem> InputItem = cpp14::make_unique<cItem>();
+	const AStringVector & InputAndIngredient = StringSplit(a_Line, "+");
-	std::unique_ptr<cItem> IngredientItem = cpp14::make_unique<cItem>();
-	std::unique_ptr<cItem> OutputItem = cpp14::make_unique<cItem>();
-
-	const AStringVector & InputAndIngredient = StringSplit(Line, "+");
 
 	if (InputAndIngredient.size() != 2)
 	{
@@ -114,39 +85,28 @@ void cBrewingRecipes::AddRecipeFromLine(const AString & a_Line, unsigned int a_L
 		return;
 	}
 
-	if (!ParseItem(IngredientAndOutput[0], *IngredientItem))
+	if (!ParseItem(IngredientAndOutput[0], Recipe->Ingredient))
 	{
 		LOGWARNING("brewing.txt: Parsing of the item didn't worked.");
 		LOGINFO("Offending line: \"%s\"", a_Line.c_str());
 		return;
 	}
 
-	if (!StringToInteger<short>(InputAndIngredient[0], InputDamage))
+	if (!StringToInteger<short>(InputAndIngredient[0], Recipe->Input.m_ItemDamage))
 	{
 		LOGWARNING("brewing.txt: line %d: Cannot parse the damage value for the input item\"%s\".", a_LineNum, InputAndIngredient[0].c_str());
 		LOGINFO("Offending line: \"%s\"", a_Line.c_str());
 		return;
 	}
 
-	if (!StringToInteger<short>(IngredientAndOutput[1], OutputDamage))
+	if (!StringToInteger<short>(IngredientAndOutput[1], Recipe->Output.m_ItemDamage))
 	{
 		LOGWARNING("brewing.txt: line %d: Cannot parse the damage value for the output item\"%s\".", a_LineNum, IngredientAndOutput[1].c_str());
 		LOGINFO("Offending line: \"%s\"", a_Line.c_str());
 		return;
 	}
 
-	// The items has always the same type
+	m_Recipes.push_back(std::move(Recipe));
-	InputItem->m_ItemType = E_ITEM_POTION;
-	InputItem->m_ItemDamage = InputDamage;
-
-	OutputItem->m_ItemType = E_ITEM_POTION;
-	OutputItem->m_ItemDamage = OutputDamage;
-
-	std::unique_ptr<cRecipe> Recipe = cpp14::make_unique<cRecipe>();
-	Recipe->Input = std::move(InputItem);
-	Recipe->Output = std::move(OutputItem);
-	Recipe->Ingredient = std::move(IngredientItem);
-	m_pState->Recipes.push_back(std::move(Recipe));
 }
 
 
@@ -164,7 +124,7 @@ bool cBrewingRecipes::ParseItem(const AString & a_String, cItem & a_Item)
 
 void cBrewingRecipes::ClearRecipes(void)
 {
-	m_pState->Recipes.clear();
+	m_Recipes.clear();
 }
 
 
@@ -173,9 +133,9 @@ void cBrewingRecipes::ClearRecipes(void)
 
 const cBrewingRecipes::cRecipe * cBrewingRecipes::GetRecipeFrom(const cItem & a_Input, const cItem & a_Ingredient) const
 {
-	for (auto & Recipe : m_pState->Recipes)
+	for (const auto & Recipe : m_Recipes)
 	{
-		if ((Recipe->Input->IsEqual(a_Input)) && (Recipe->Ingredient->IsEqual(a_Ingredient)))
+		if ((Recipe->Input.IsEqual(a_Input)) && (Recipe->Ingredient.IsEqual(a_Ingredient)))
 		{
 			return Recipe.get();
 		}
@@ -187,22 +147,15 @@ const cBrewingRecipes::cRecipe * cBrewingRecipes::GetRecipeFrom(const cItem & a_
 		if (a_Input.m_ItemDamage & 0x2000)
 		{
 			// Create new recipe and add it to list
-			std::unique_ptr<cItem> InputItem = cpp14::make_unique<cItem>();
+			auto Recipe = cpp14::make_unique<cRecipe>();
-			std::unique_ptr<cItem> IngredientItem = cpp14::make_unique<cItem>();
-			std::unique_ptr<cItem> OutputItem = cpp14::make_unique<cItem>();
 
-			InputItem->m_ItemType = E_ITEM_POTION;
+			Recipe->Input.m_ItemType = a_Input.m_ItemDamage;
-			InputItem->m_ItemDamage = a_Input.m_ItemDamage;
+			Recipe->Output.m_ItemDamage = a_Input.m_ItemDamage + 8192;
-			OutputItem->m_ItemType = E_ITEM_POTION;
+			Recipe->Ingredient.m_ItemType = E_ITEM_GUNPOWDER;
-			OutputItem->m_ItemDamage = a_Input.m_ItemDamage + 8192;
-			IngredientItem->m_ItemType = E_ITEM_GUNPOWDER;
 
-			std::unique_ptr<cRecipe> Recipe = cpp14::make_unique<cRecipe>();
+			auto RecipePtr = Recipe.get();
-			Recipe->Input = std::move(InputItem);
+			m_Recipes.push_back(std::move(Recipe));
-			Recipe->Output = std::move(OutputItem);
+			return RecipePtr;
-			Recipe->Ingredient = std::move(IngredientItem);
-			m_pState->Recipes.push_back(std::move(Recipe));
-			return Recipe.get();
 		}
 		return nullptr;
 	}
@@ -210,41 +163,32 @@ const cBrewingRecipes::cRecipe * cBrewingRecipes::GetRecipeFrom(const cItem & a_
 	// Check for splash potion
 	if (a_Input.m_ItemDamage & 0x4000)
 	{
-		const std::unique_ptr<cRecipe> * FoundRecipe = nullptr;
 		// Search for the drinkable potion, the ingredients are the same
 		short SplashItemDamage = a_Input.m_ItemDamage - 8192;
 
-		for (auto & Recipe : m_pState->Recipes)
+		auto FoundRecipe = std::find_if(m_Recipes.cbegin(), m_Recipes.cend(), [&](const std::unique_ptr<cRecipe>& a_Recipe)
 		{
-			if ((Recipe->Input->m_ItemDamage == SplashItemDamage) && (Recipe->Ingredient->IsEqual(a_Ingredient)))
+			return (
-			{
+				(a_Recipe->Input.m_ItemDamage == SplashItemDamage) &&
-				FoundRecipe = &Recipe;
+				(a_Recipe->Ingredient.IsEqual(a_Ingredient))
-				break;
+			);
-			}
+		});
-		}
 
-		if (FoundRecipe == nullptr)
+		if (FoundRecipe == m_Recipes.cend())
 		{
 			return nullptr;
 		}
 
 		// Create new recipe and add it to list
-		std::unique_ptr<cItem> InputItem = cpp14::make_unique<cItem>();
+		auto Recipe = cpp14::make_unique<cRecipe>();
-		std::unique_ptr<cItem> IngredientItem = cpp14::make_unique<cItem>();
-		std::unique_ptr<cItem> OutputItem = cpp14::make_unique<cItem>();
 
-		InputItem->m_ItemType = E_ITEM_POTION;
+		Recipe->Input.m_ItemDamage = a_Input.m_ItemDamage;
-		InputItem->m_ItemDamage = a_Input.m_ItemDamage;
+		Recipe->Output.m_ItemDamage = (*FoundRecipe)->Output.m_ItemDamage + 8192;
-		OutputItem->m_ItemType = E_ITEM_POTION;
+		Recipe->Ingredient.m_ItemType = (*FoundRecipe)->Ingredient.m_ItemType;
-		OutputItem->m_ItemDamage = (*FoundRecipe)->Output->m_ItemDamage + 8192;
-		IngredientItem->m_ItemType = (*FoundRecipe)->Ingredient->m_ItemType;
 
-		std::unique_ptr<cRecipe> Recipe = cpp14::make_unique<cRecipe>();
+		auto RecipePtr = Recipe.get();
-		Recipe->Input = std::move(InputItem);
+		m_Recipes.push_back(std::move(Recipe));
-		Recipe->Output = std::move(OutputItem);
+		return RecipePtr;
-		Recipe->Ingredient = std::move(IngredientItem);
-		m_pState->Recipes.push_back(std::move(Recipe));
-		return Recipe.get();
 	}
 	return nullptr;
 }
@@ -262,9 +206,9 @@ bool cBrewingRecipes::IsIngredient(const cItem & a_Ingredient) const
 		return true;
 	}
 
-	for (auto & Recipe : m_pState->Recipes)
+	for (const auto & Recipe : m_Recipes)
 	{
-		if (Recipe->Ingredient->IsEqual(a_Ingredient))
+		if (Recipe->Ingredient.IsEqual(a_Ingredient))
 		{
 			return true;
 		}

src/BrewingRecipes.h

@@ -5,7 +5,7 @@
 
 
 
-class cItem;
+#include "Item.h"
 
 
 
@@ -15,21 +15,21 @@ class cBrewingRecipes
 {
 public:
 	cBrewingRecipes(void);
-	~cBrewingRecipes();
 
 	void ReloadRecipes(void);
 
 	struct cRecipe
 	{
-		cRecipe() {}
+		cRecipe()
-		cRecipe(cRecipe &&) {}
+		{
+			// These items always have the same type
+			Input.m_ItemType = E_ITEM_POTION;
+			Output.m_ItemType = E_ITEM_POTION;
+		}
 
-		cRecipe(const cRecipe&) = delete;
+		cItem Input;
-		cRecipe & operator=(const cRecipe&) = delete;
+		cItem Output;
-
+		cItem Ingredient;
-		std::unique_ptr<cItem> Input;
-		std::unique_ptr<cItem> Output;
-		std::unique_ptr<cItem> Ingredient;
 	};
 
 	/** Returns a recipe for the specified input, nullptr if no recipe found */
@@ -44,15 +44,18 @@ public:
 	/** Returns true if the item is the fuel, false if not. */
 	bool IsFuel(const cItem & a_Item) const;
 private:
+	using cRecipes = std::vector<std::unique_ptr<cBrewingRecipes::cRecipe>>;
+
 	void ClearRecipes(void);
 
 	/** Parses the recipe contained in the line, adds it to m_pState's recipes.
 	Logs a warning to the console on input error. */
-	void AddRecipeFromLine(const AString & a_Line, unsigned int a_LineNum);
+	void AddRecipeFromLine(AString a_Line, unsigned int a_LineNum);
 
 	/** Parses an item string, returns true if successful. */
 	bool ParseItem(const AString & a_String, cItem & a_Item);
 
-	struct sBrewingRecipeState;
+	/** The collection of parsed recipes.
-	std::unique_ptr<sBrewingRecipeState> m_pState;
+	GetRecipeFrom may cache splash variants of recipes here but the observable behaviour is constant, so this should be mutable. */
+	mutable cRecipes m_Recipes;
 };