From a908f39dde1dc8866bda1a85b8443ae4cdaaa2b4 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sat, 21 Jun 2014 20:33:23 +0100 Subject: [PATCH 1/6] Rewrote furnace recipe parser * Fixes #110 --- src/FurnaceRecipe.cpp | 260 ++++++++++++++++++++++++++---------------- src/FurnaceRecipe.h | 22 ++++ 2 files changed, 185 insertions(+), 97 deletions(-) diff --git a/src/FurnaceRecipe.cpp b/src/FurnaceRecipe.cpp index bd7fd8079..b4b3d34d9 100644 --- a/src/FurnaceRecipe.cpp +++ b/src/FurnaceRecipe.cpp @@ -5,7 +5,8 @@ #include "Item.h" #include -#include + +#define FURNACE_RECIPE_FILE "furnace.txt" @@ -53,128 +54,193 @@ void cFurnaceRecipe::ReloadRecipes(void) ClearRecipes(); LOGD("Loading furnace recipes..."); - std::ifstream f; - char a_File[] = "furnace.txt"; - f.open(a_File, std::ios::in); - - if (!f.good()) + std::ifstream F(FURNACE_RECIPE_FILE, std::ios::in); + if (!F.good()) { - f.close(); - LOG("Could not open the furnace recipes file \"%s\"", a_File); + F.close(); + LOG("Could not open the furnace recipes file \"%s\"", FURNACE_RECIPE_FILE); return; } + + AString SyntaxError; + unsigned int Line = 0; + AString ParsingLine; - // TODO: Replace this messy parse with a high-level-structured one (ReadLine / ProcessLine) - bool bSyntaxError = false; - while (f.good()) + ASSERT(ParsingLine.empty()); + + while (std::getline(F, ParsingLine)) { - char c; - - ////////////////////////////////////////////////////////////////////////// - // comments - f >> c; - f.unget(); - if( c == '#' ) + Line++; + ParsingLine.erase(std::remove_if(ParsingLine.begin(), ParsingLine.end(), isspace), ParsingLine.end()); // Remove whitespace + if (ParsingLine.empty()) { - while( f.good() && c != '\n' ) - { - f.get( c ); - } continue; } - - ////////////////////////////////////////////////////////////////////////// - // Line breaks - f.get( c ); - while( f.good() && ( c == '\n' || c == '\r' ) ) { f.get( c ); } - if (f.eof()) + // Comments + if (ParsingLine[0] == '#') { - break; + continue; } - f.unget(); - ////////////////////////////////////////////////////////////////////////// - // Check for fuel - f >> c; - if( c == '!' ) // It's fuel :) + if (ParsingLine[0] == '!') // Fuels start with a bang :) { - // Read item - int IItemID = 0, IItemCount = 0, IItemHealth = 0; - f >> IItemID; - f >> c; if( c != ':' ) { bSyntaxError = true; break; } - f >> IItemCount; + int IItemID = 0, IItemCount = 0, IItemHealth = 0, IBurnTime = 0; + AString::size_type BeginPos = 1; // Begin at one after exclamation mark (bang) - // Optional health - f >> c; - if( c != ':' ) - f.unget(); - else + if ( + !ReadMandatoryNumber(BeginPos, ":", ParsingLine, SyntaxError, Line, IItemID) || // Read item ID + !ReadOptionalNumbers(BeginPos, ":", "=", ParsingLine, SyntaxError, Line, IItemCount, IItemHealth) || // Read item count (and optionally health) + !ReadMandatoryNumber(BeginPos, "0123456789", ParsingLine, SyntaxError, Line, IBurnTime, true) // Read item burn time - last value + ) { - f >> IItemHealth; + break; } - // Burn time - int BurnTime; - f >> c; if( c != '=' ) { bSyntaxError = true; break; } - f >> BurnTime; - // Add to fuel list Fuel F; - F.In = new cItem( (ENUM_ITEM_ID) IItemID, (char)IItemCount, (short)IItemHealth ); - F.BurnTime = BurnTime; - m_pState->Fuel.push_back( F ); + F.In = new cItem((ENUM_ITEM_ID)IItemID, (char)IItemCount, (short)IItemHealth); + F.BurnTime = IBurnTime; + m_pState->Fuel.push_back(F); + printf("%i %i %i %i\n", IItemID, IItemCount, IItemHealth, IBurnTime); continue; } - f.unget(); - - ////////////////////////////////////////////////////////////////////////// - // Read items - int IItemID = 0, IItemCount = 0, IItemHealth = 0; - f >> IItemID; - f >> c; if( c != ':' ) { bSyntaxError = true; break; } - f >> IItemCount; - - // Optional health - f >> c; - if( c != ':' ) - f.unget(); - else + else if (isdigit(ParsingLine[0])) // Recipes start with a numeral :) { - f >> IItemHealth; + int IItemID = 0, IItemCount = 0, IItemHealth = 0, IBurnTime = 0; + int OItemID = 0, OItemCount = 0, OItemHealth = 0; + AString::size_type BeginPos = 0; // Begin at start of line + + if ( + !ReadMandatoryNumber(BeginPos, ":", ParsingLine, SyntaxError, Line, IItemID) || // Read item ID + !ReadOptionalNumbers(BeginPos, ":", "@", ParsingLine, SyntaxError, Line, IItemCount, IItemHealth) || // Read item count (and optionally health) + !ReadMandatoryNumber(BeginPos, "=", ParsingLine, SyntaxError, Line, IBurnTime) || // Read item burn time + !ReadMandatoryNumber(BeginPos, ":", ParsingLine, SyntaxError, Line, OItemID) || // Read result ID + !ReadOptionalNumbers(BeginPos, ":", "012456789", ParsingLine, SyntaxError, Line, OItemCount, OItemHealth, true) // Read result count (and optionally health) - last value + ) + { + break; + } + + // Add to recipe list + Recipe R; + R.In = new cItem((ENUM_ITEM_ID)IItemID, (char)IItemCount, (short)IItemHealth); + R.Out = new cItem((ENUM_ITEM_ID)OItemID, (char)OItemCount, (short)OItemHealth); + R.CookTime = IBurnTime; + m_pState->Recipes.push_back(R); } - - int CookTime; - f >> c; if( c != '@' ) { bSyntaxError = true; break; } - f >> CookTime; - - int OItemID = 0, OItemCount = 0, OItemHealth = 0; - f >> c; if( c != '=' ) { bSyntaxError = true; break; } - f >> OItemID; - f >> c; if( c != ':' ) { bSyntaxError = true; break; } - f >> OItemCount; - - // Optional health - f >> c; - if( c != ':' ) - f.unget(); - else - { - f >> OItemHealth; - } - - // Add to recipe list - Recipe R; - R.In = new cItem( (ENUM_ITEM_ID)IItemID, (char)IItemCount, (short)IItemHealth ); - R.Out = new cItem( (ENUM_ITEM_ID)OItemID, (char)OItemCount, (short)OItemHealth ); - R.CookTime = CookTime; - m_pState->Recipes.push_back( R ); } - if (bSyntaxError) + F.close(); + + if (!SyntaxError.empty()) { - LOGERROR("ERROR: FurnaceRecipe, syntax error" ); + LOGWARN("Error parsing furnace recipes at %s", SyntaxError.c_str()); } - LOG("Loaded " SIZE_T_FMT " furnace recipes and " SIZE_T_FMT " fuels", m_pState->Recipes.size(), m_pState->Fuel.size()); + else + { + LOG("Loaded " SIZE_T_FMT " furnace recipes and " SIZE_T_FMT " fuels", m_pState->Recipes.size(), m_pState->Fuel.size()); + } +} + + + + + +void cFurnaceRecipe::SetParseError(AString & a_ErrorString, unsigned int a_Line, size_t a_Position, const AString & a_CharactersMissing) +{ + Printf(a_ErrorString, "line %i pos %i: missing '%s'", a_Line, a_Position, a_CharactersMissing.c_str()); +} + + + + + +bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const AString & a_Delimiter, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_Value, bool a_IsLastValue) +{ + AString::size_type End; + if (a_IsLastValue) + { + End = a_Text.find_first_not_of(a_Delimiter, a_Begin); + } + else + { + End = a_Text.find_first_of(a_Delimiter, a_Begin); + if (End == AString::npos) + { + SetParseError(a_ErrorString, a_Line, a_Begin, a_Delimiter); + return false; + } + } + + // stoi won't throw an exception if the string is alphanumeric, we should check for this + if (!DoesStringContainOnlyNumbers(a_Text.substr(a_Begin, End - a_Begin))) + { + SetParseError(a_ErrorString, a_Line, a_Begin, "number"); + return false; + } + a_Value = std::stoi(a_Text.substr(a_Begin, End - a_Begin)); + + a_Begin = End + 1; // Jump over delimiter + return true; +} + + + + + +bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue) +{ + unsigned int End, Begin = a_Begin; + + End = a_Text.find_first_of(a_DelimiterOne, Begin); + if (End != AString::npos) + { + if (DoesStringContainOnlyNumbers(a_Text.substr(Begin, End - Begin))) + { + a_ValueOne = std::stoi(a_Text.substr(Begin, End - Begin)); + Begin = End + 1; + + if (a_IsLastValue) + { + End = a_Text.find_first_not_of(a_DelimiterTwo, Begin); + } + else + { + End = a_Text.find_first_of(a_DelimiterTwo, Begin); + if (End == AString::npos) + { + SetParseError(a_ErrorString, a_Line, Begin, a_DelimiterTwo); + return false; + } + } + + // stoi won't throw an exception if the string is alphanumeric, we should check for this + if (!DoesStringContainOnlyNumbers(a_Text.substr(Begin, End - Begin))) + { + SetParseError(a_ErrorString, a_Line, Begin, "number"); + return false; + } + a_ValueTwo = std::stoi(a_Text.substr(Begin, End - Begin)); + + a_Begin = End + 1; // Jump over delimiter + return true; + } + else + { + return ReadMandatoryNumber(a_Begin, a_DelimiterTwo, a_Text, a_ErrorString, a_Line, a_ValueOne, a_IsLastValue); + } + } + + return ReadMandatoryNumber(a_Begin, a_DelimiterTwo, a_Text, a_ErrorString, a_Line, a_ValueOne, a_IsLastValue); +} + + + + + +bool cFurnaceRecipe::DoesStringContainOnlyNumbers(const AString & a_String) +{ + return std::all_of(a_String.begin(), a_String.end(), isdigit); } diff --git a/src/FurnaceRecipe.h b/src/FurnaceRecipe.h index 2f91e9bcb..e0222f2fb 100644 --- a/src/FurnaceRecipe.h +++ b/src/FurnaceRecipe.h @@ -41,6 +41,28 @@ public: private: void ClearRecipes(void); + /** PrintFs the line, position, and error into an error string */ + inline static void SetParseError(AString & a_ErrorString, unsigned int a_Line, size_t a_Position, const AString & a_CharactersMissing); + + /** Reads a number from a string given, starting at a given position and ending at a delimiter given + Updates beginning position to the delimiter found + 1, and updates the value to the one read + If it encounters a substring that is not fully numeric, it will call SetParseError() and return false; the caller should abort processing + Otherwise, the function will return true + */ + static bool ReadMandatoryNumber(AString::size_type & a_Begin, const AString & a_Delimiter, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_Value, bool a_IsLastValue = false); + + /** Reads two numbers from a string given, starting at a given position and ending at the first delimiter given, then again (with an updated position) until the second delimiter given + Updates beginning position to the second delimiter found + 1, and updates the values to the ones read + If it encounters a substring that is not fully numeric whilst reading the second value, it will call SetParseError() and return false; the caller should abort processing + If this happens whilst reading the first value, it will call ReadMandatoryNumber() with the appropriate position, as this may legitimately occur with the optional value and AString::find_first_of finding the incorrect delimiter. It will return the result of ReadMandatoryNumber() + True will be returned definitively for an optional value that is valid + */ + static bool ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue = false); + + /** Uses std::all_of to determine if a string contains only digits */ + inline static bool DoesStringContainOnlyNumbers(const AString & a_String); + + struct sFurnaceRecipeState; sFurnaceRecipeState * m_pState; }; From 537467fe25386a67ef5cb9a33b9806ac2db0c894 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sat, 21 Jun 2014 20:35:28 +0100 Subject: [PATCH 2/6] Removed debugging code --- src/FurnaceRecipe.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/FurnaceRecipe.cpp b/src/FurnaceRecipe.cpp index b4b3d34d9..ce9233bcc 100644 --- a/src/FurnaceRecipe.cpp +++ b/src/FurnaceRecipe.cpp @@ -102,7 +102,6 @@ void cFurnaceRecipe::ReloadRecipes(void) F.In = new cItem((ENUM_ITEM_ID)IItemID, (char)IItemCount, (short)IItemHealth); F.BurnTime = IBurnTime; m_pState->Fuel.push_back(F); - printf("%i %i %i %i\n", IItemID, IItemCount, IItemHealth, IBurnTime); continue; } else if (isdigit(ParsingLine[0])) // Recipes start with a numeral :) From 4a01fba3aafef53ac8015bd5f6028fc677d6d245 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sun, 22 Jun 2014 00:06:58 +0100 Subject: [PATCH 3/6] Suggestions --- src/FurnaceRecipe.cpp | 62 ++++++++++++++++++------------------------- src/FurnaceRecipe.h | 8 +++--- 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/FurnaceRecipe.cpp b/src/FurnaceRecipe.cpp index ce9233bcc..448a4bfa8 100644 --- a/src/FurnaceRecipe.cpp +++ b/src/FurnaceRecipe.cpp @@ -54,21 +54,18 @@ void cFurnaceRecipe::ReloadRecipes(void) ClearRecipes(); LOGD("Loading furnace recipes..."); - std::ifstream F(FURNACE_RECIPE_FILE, std::ios::in); - if (!F.good()) + std::ifstream f(FURNACE_RECIPE_FILE, std::ios::in); + if (!f.good()) { - F.close(); + f.close(); LOG("Could not open the furnace recipes file \"%s\"", FURNACE_RECIPE_FILE); return; } - AString SyntaxError; unsigned int Line = 0; AString ParsingLine; - ASSERT(ParsingLine.empty()); - - while (std::getline(F, ParsingLine)) + while (std::getline(f, ParsingLine)) { Line++; ParsingLine.erase(std::remove_if(ParsingLine.begin(), ParsingLine.end(), isspace), ParsingLine.end()); // Remove whitespace @@ -89,12 +86,12 @@ void cFurnaceRecipe::ReloadRecipes(void) AString::size_type BeginPos = 1; // Begin at one after exclamation mark (bang) if ( - !ReadMandatoryNumber(BeginPos, ":", ParsingLine, SyntaxError, Line, IItemID) || // Read item ID - !ReadOptionalNumbers(BeginPos, ":", "=", ParsingLine, SyntaxError, Line, IItemCount, IItemHealth) || // Read item count (and optionally health) - !ReadMandatoryNumber(BeginPos, "0123456789", ParsingLine, SyntaxError, Line, IBurnTime, true) // Read item burn time - last value + !ReadMandatoryNumber(BeginPos, ":", ParsingLine, Line, IItemID) || // Read item ID + !ReadOptionalNumbers(BeginPos, ":", "=", ParsingLine, Line, IItemCount, IItemHealth) || // Read item count (and optionally health) + !ReadMandatoryNumber(BeginPos, "0123456789", ParsingLine, Line, IBurnTime, true) // Read item burn time - last value ) { - break; + return; } // Add to fuel list @@ -111,14 +108,14 @@ void cFurnaceRecipe::ReloadRecipes(void) AString::size_type BeginPos = 0; // Begin at start of line if ( - !ReadMandatoryNumber(BeginPos, ":", ParsingLine, SyntaxError, Line, IItemID) || // Read item ID - !ReadOptionalNumbers(BeginPos, ":", "@", ParsingLine, SyntaxError, Line, IItemCount, IItemHealth) || // Read item count (and optionally health) - !ReadMandatoryNumber(BeginPos, "=", ParsingLine, SyntaxError, Line, IBurnTime) || // Read item burn time - !ReadMandatoryNumber(BeginPos, ":", ParsingLine, SyntaxError, Line, OItemID) || // Read result ID - !ReadOptionalNumbers(BeginPos, ":", "012456789", ParsingLine, SyntaxError, Line, OItemCount, OItemHealth, true) // Read result count (and optionally health) - last value + !ReadMandatoryNumber(BeginPos, ":", ParsingLine, Line, IItemID) || // Read item ID + !ReadOptionalNumbers(BeginPos, ":", "@", ParsingLine, Line, IItemCount, IItemHealth) || // Read item count (and optionally health) + !ReadMandatoryNumber(BeginPos, "=", ParsingLine, Line, IBurnTime) || // Read item burn time + !ReadMandatoryNumber(BeginPos, ":", ParsingLine, Line, OItemID) || // Read result ID + !ReadOptionalNumbers(BeginPos, ":", "012456789", ParsingLine, Line, OItemCount, OItemHealth, true) // Read result count (and optionally health) - last value ) { - break; + return; } // Add to recipe list @@ -129,32 +126,25 @@ void cFurnaceRecipe::ReloadRecipes(void) m_pState->Recipes.push_back(R); } } - F.close(); + f.close(); - if (!SyntaxError.empty()) - { - LOGWARN("Error parsing furnace recipes at %s", SyntaxError.c_str()); - } - else - { - LOG("Loaded " SIZE_T_FMT " furnace recipes and " SIZE_T_FMT " fuels", m_pState->Recipes.size(), m_pState->Fuel.size()); - } + LOG("Loaded " SIZE_T_FMT " furnace recipes and " SIZE_T_FMT " fuels", m_pState->Recipes.size(), m_pState->Fuel.size()); } -void cFurnaceRecipe::SetParseError(AString & a_ErrorString, unsigned int a_Line, size_t a_Position, const AString & a_CharactersMissing) +void cFurnaceRecipe::PrintParseError(unsigned int a_Line, size_t a_Position, const AString & a_CharactersMissing) { - Printf(a_ErrorString, "line %i pos %i: missing '%s'", a_Line, a_Position, a_CharactersMissing.c_str()); + LOGWARN("Error parsing furnace recipes at line %i pos %i: missing '%s'", a_Line, a_Position, a_CharactersMissing.c_str()); } -bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const AString & a_Delimiter, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_Value, bool a_IsLastValue) +bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const AString & a_Delimiter, const AString & a_Text, unsigned int a_Line, int & a_Value, bool a_IsLastValue) { AString::size_type End; if (a_IsLastValue) @@ -166,7 +156,7 @@ bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const ASt End = a_Text.find_first_of(a_Delimiter, a_Begin); if (End == AString::npos) { - SetParseError(a_ErrorString, a_Line, a_Begin, a_Delimiter); + PrintParseError(a_Line, a_Begin, a_Delimiter); return false; } } @@ -174,7 +164,7 @@ bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const ASt // stoi won't throw an exception if the string is alphanumeric, we should check for this if (!DoesStringContainOnlyNumbers(a_Text.substr(a_Begin, End - a_Begin))) { - SetParseError(a_ErrorString, a_Line, a_Begin, "number"); + PrintParseError(a_Line, a_Begin, "number"); return false; } a_Value = std::stoi(a_Text.substr(a_Begin, End - a_Begin)); @@ -187,7 +177,7 @@ bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const ASt -bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue) +bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue) { unsigned int End, Begin = a_Begin; @@ -208,7 +198,7 @@ bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const ASt End = a_Text.find_first_of(a_DelimiterTwo, Begin); if (End == AString::npos) { - SetParseError(a_ErrorString, a_Line, Begin, a_DelimiterTwo); + PrintParseError(a_Line, Begin, a_DelimiterTwo); return false; } } @@ -216,7 +206,7 @@ bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const ASt // stoi won't throw an exception if the string is alphanumeric, we should check for this if (!DoesStringContainOnlyNumbers(a_Text.substr(Begin, End - Begin))) { - SetParseError(a_ErrorString, a_Line, Begin, "number"); + PrintParseError(a_Line, Begin, "number"); return false; } a_ValueTwo = std::stoi(a_Text.substr(Begin, End - Begin)); @@ -226,11 +216,11 @@ bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const ASt } else { - return ReadMandatoryNumber(a_Begin, a_DelimiterTwo, a_Text, a_ErrorString, a_Line, a_ValueOne, a_IsLastValue); + return ReadMandatoryNumber(a_Begin, a_DelimiterTwo, a_Text, a_Line, a_ValueOne, a_IsLastValue); } } - return ReadMandatoryNumber(a_Begin, a_DelimiterTwo, a_Text, a_ErrorString, a_Line, a_ValueOne, a_IsLastValue); + return ReadMandatoryNumber(a_Begin, a_DelimiterTwo, a_Text, a_Line, a_ValueOne, a_IsLastValue); } diff --git a/src/FurnaceRecipe.h b/src/FurnaceRecipe.h index e0222f2fb..cddcc215f 100644 --- a/src/FurnaceRecipe.h +++ b/src/FurnaceRecipe.h @@ -41,15 +41,15 @@ public: private: void ClearRecipes(void); - /** PrintFs the line, position, and error into an error string */ - inline static void SetParseError(AString & a_ErrorString, unsigned int a_Line, size_t a_Position, const AString & a_CharactersMissing); + /** Calls LOGWARN with the line, position, and error */ + inline static void PrintParseError(unsigned int a_Line, size_t a_Position, const AString & a_CharactersMissing); /** Reads a number from a string given, starting at a given position and ending at a delimiter given Updates beginning position to the delimiter found + 1, and updates the value to the one read If it encounters a substring that is not fully numeric, it will call SetParseError() and return false; the caller should abort processing Otherwise, the function will return true */ - static bool ReadMandatoryNumber(AString::size_type & a_Begin, const AString & a_Delimiter, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_Value, bool a_IsLastValue = false); + static bool ReadMandatoryNumber(AString::size_type & a_Begin, const AString & a_Delimiter, const AString & a_Text, unsigned int a_Line, int & a_Value, bool a_IsLastValue = false); /** Reads two numbers from a string given, starting at a given position and ending at the first delimiter given, then again (with an updated position) until the second delimiter given Updates beginning position to the second delimiter found + 1, and updates the values to the ones read @@ -57,7 +57,7 @@ private: If this happens whilst reading the first value, it will call ReadMandatoryNumber() with the appropriate position, as this may legitimately occur with the optional value and AString::find_first_of finding the incorrect delimiter. It will return the result of ReadMandatoryNumber() True will be returned definitively for an optional value that is valid */ - static bool ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, AString & a_ErrorString, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue = false); + static bool ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue = false); /** Uses std::all_of to determine if a string contains only digits */ inline static bool DoesStringContainOnlyNumbers(const AString & a_String); From 63ce2e8b37444f96d7892d555cb296947b3afbe5 Mon Sep 17 00:00:00 2001 From: worktycho Date: Sun, 22 Jun 2014 12:30:37 +0100 Subject: [PATCH 4/6] Fixed compile errors --- src/FurnaceRecipe.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FurnaceRecipe.cpp b/src/FurnaceRecipe.cpp index 448a4bfa8..8040552cc 100644 --- a/src/FurnaceRecipe.cpp +++ b/src/FurnaceRecipe.cpp @@ -137,7 +137,7 @@ void cFurnaceRecipe::ReloadRecipes(void) void cFurnaceRecipe::PrintParseError(unsigned int a_Line, size_t a_Position, const AString & a_CharactersMissing) { - LOGWARN("Error parsing furnace recipes at line %i pos %i: missing '%s'", a_Line, a_Position, a_CharactersMissing.c_str()); + LOGWARN("Error parsing furnace recipes at line %i pos " SIZE_T_FMT ": missing '%s'", a_Line, a_Position, a_CharactersMissing.c_str()); } @@ -179,7 +179,7 @@ bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const ASt bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue) { - unsigned int End, Begin = a_Begin; + AString::size_type End, Begin = a_Begin; End = a_Text.find_first_of(a_DelimiterOne, Begin); if (End != AString::npos) From c476fc3cf5a2bbdb27b61fcad290c84029721462 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sun, 22 Jun 2014 21:49:37 +0100 Subject: [PATCH 5/6] Suggestions --- src/FurnaceRecipe.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/FurnaceRecipe.cpp b/src/FurnaceRecipe.cpp index 448a4bfa8..90b478285 100644 --- a/src/FurnaceRecipe.cpp +++ b/src/FurnaceRecipe.cpp @@ -57,7 +57,6 @@ void cFurnaceRecipe::ReloadRecipes(void) std::ifstream f(FURNACE_RECIPE_FILE, std::ios::in); if (!f.good()) { - f.close(); LOG("Could not open the furnace recipes file \"%s\"", FURNACE_RECIPE_FILE); return; } @@ -126,7 +125,6 @@ void cFurnaceRecipe::ReloadRecipes(void) m_pState->Recipes.push_back(R); } } - f.close(); LOG("Loaded " SIZE_T_FMT " furnace recipes and " SIZE_T_FMT " fuels", m_pState->Recipes.size(), m_pState->Fuel.size()); } @@ -146,6 +144,7 @@ void cFurnaceRecipe::PrintParseError(unsigned int a_Line, size_t a_Position, con bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const AString & a_Delimiter, const AString & a_Text, unsigned int a_Line, int & a_Value, bool a_IsLastValue) { + // TODO: replace atoi with std::stoi AString::size_type End; if (a_IsLastValue) { @@ -167,7 +166,7 @@ bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const ASt PrintParseError(a_Line, a_Begin, "number"); return false; } - a_Value = std::stoi(a_Text.substr(a_Begin, End - a_Begin)); + a_Value = atoi(a_Text.substr(a_Begin, End - a_Begin).c_str()); a_Begin = End + 1; // Jump over delimiter return true; @@ -179,6 +178,7 @@ bool cFurnaceRecipe::ReadMandatoryNumber(AString::size_type & a_Begin, const ASt bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const AString & a_DelimiterOne, const AString & a_DelimiterTwo, const AString & a_Text, unsigned int a_Line, int & a_ValueOne, int & a_ValueTwo, bool a_IsLastValue) { + // TODO: replace atoi with std::stoi unsigned int End, Begin = a_Begin; End = a_Text.find_first_of(a_DelimiterOne, Begin); @@ -186,7 +186,7 @@ bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const ASt { if (DoesStringContainOnlyNumbers(a_Text.substr(Begin, End - Begin))) { - a_ValueOne = std::stoi(a_Text.substr(Begin, End - Begin)); + a_ValueOne = std::atoi(a_Text.substr(Begin, End - Begin).c_str()); Begin = End + 1; if (a_IsLastValue) @@ -209,7 +209,7 @@ bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const ASt PrintParseError(a_Line, Begin, "number"); return false; } - a_ValueTwo = std::stoi(a_Text.substr(Begin, End - Begin)); + a_ValueTwo = atoi(a_Text.substr(Begin, End - Begin).c_str()); a_Begin = End + 1; // Jump over delimiter return true; @@ -229,7 +229,8 @@ bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const ASt bool cFurnaceRecipe::DoesStringContainOnlyNumbers(const AString & a_String) { - return std::all_of(a_String.begin(), a_String.end(), isdigit); + // TODO: replace this with std::all_of(a_String.begin(), a_String.end(), isdigit) + return a_String.find_first_not_of("0123456789") == AString::npos; } From 7a236921319c44de38246ff73d5343e174dd560f Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Mon, 23 Jun 2014 17:40:51 +0100 Subject: [PATCH 6/6] Parenthesised comparison --- src/FurnaceRecipe.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FurnaceRecipe.cpp b/src/FurnaceRecipe.cpp index d53854950..36dd2d5e9 100644 --- a/src/FurnaceRecipe.cpp +++ b/src/FurnaceRecipe.cpp @@ -230,7 +230,7 @@ bool cFurnaceRecipe::ReadOptionalNumbers(AString::size_type & a_Begin, const ASt bool cFurnaceRecipe::DoesStringContainOnlyNumbers(const AString & a_String) { // TODO: replace this with std::all_of(a_String.begin(), a_String.end(), isdigit) - return a_String.find_first_not_of("0123456789") == AString::npos; + return (a_String.find_first_not_of("0123456789") == AString::npos); }