Add ParseFixed6 equivalent of ParseInt

Source/data/file.cpp

@@ -47,24 +47,24 @@ void DataFile::reportFatalError(Error code, std::string_view fileName)
 	}
 }
 
-void DataFile::reportFatalFieldError(std::errc code, std::string_view fileName, std::string_view fieldName, const DataFileField &field)
+void DataFile::reportFatalFieldError(DataFileField::Error code, std::string_view fileName, std::string_view fieldName, const DataFileField &field)
 {
 	switch (code) {
-	case std::errc::invalid_argument:
+	case DataFileField::Error::NotANumber:
 		app_fatal(fmt::format(fmt::runtime(_(
 		                          /* TRANSLATORS: Error message when parsing a data file and a text value is encountered when a number is expected. Arguments are {found value}, {column heading}, {file name}, {row/record number}, {column/field number} */
 		                          "Non-numeric value {0} for {1} in {2} at row {3} and column {4}")),
 		    field.currentValue(), fieldName, fileName, field.row(), field.column()));
-	case std::errc::result_out_of_range:
+	case DataFileField::Error::OutOfRange:
 		app_fatal(fmt::format(fmt::runtime(_(
 		                          /* TRANSLATORS: Error message when parsing a data file and we find a number larger than expected. Arguments are {found value}, {column heading}, {file name}, {row/record number}, {column/field number} */
 		                          "Out of range value {0} for {1} in {2} at row {3} and column {4}")),
 		    field.currentValue(), fieldName, fileName, field.row(), field.column()));
-	default:
+	case DataFileField::Error::InvalidValue:
 		app_fatal(fmt::format(fmt::runtime(_(
-		                          /* TRANSLATORS: Fallback error message when an error occurs while parsing a data file and we can't determine the cause. Arguments are {file name}, {row/record number}, {column/field number} */
-		                          "Unexpected error while reading {0} at row {1} and column {2}")),
-		    fileName, field.row(), field.column()));
+		                          /* TRANSLATORS: Error message when we find an unrecognised value in a key column. Arguments are {found value}, {column heading}, {file name}, {row/record number}, {column/field number} */
+		                          "Invalid value {0} for {1} in {2} at row {3} and column {4}")),
+		    field.currentValue(), fieldName, fileName, field.row(), field.column()));
 	}
 }
 

Source/data/file.hpp

@@ -75,7 +75,7 @@ class DataFile {
 	static tl::expected<DataFile, Error> load(std::string_view path);
 
 	static void reportFatalError(Error code, std::string_view fileName);
-	static void reportFatalFieldError(std::errc code, std::string_view fileName, std::string_view fieldName, const DataFileField &field);
+	static void reportFatalFieldError(DataFileField::Error code, std::string_view fileName, std::string_view fieldName, const DataFileField &field);
 
 	void resetHeader()
 	{

Source/data/iterators.hpp

@@ -6,6 +6,7 @@
 #include <expected.hpp>
 
 #include "parser.hpp"
+#include "utils/parse_int.hpp"
 
 namespace devilution {
 
@@ -16,6 +17,38 @@ class DataFileField {
 	unsigned column_;
 
 public:
+	enum class Error {
+		NotANumber,
+		OutOfRange,
+		InvalidValue
+	};
+
+	static tl::expected<void, Error> mapError(std::errc ec)
+	{
+		switch (ec) {
+		case std::errc():
+			return {};
+		case std::errc::result_out_of_range:
+			return tl::unexpected { Error::OutOfRange };
+		case std::errc::invalid_argument:
+			return tl::unexpected { Error::NotANumber };
+		default:
+			return tl::unexpected { Error::InvalidValue };
+		}
+	}
+
+	static tl::expected<void, Error> mapError(ParseIntError ec)
+	{
+		switch (ec) {
+		case ParseIntError::OutOfRange:
+			return tl::unexpected { Error::OutOfRange };
+		case ParseIntError::ParseError:
+			return tl::unexpected { Error::NotANumber };
+		default:
+			return tl::unexpected { Error::InvalidValue };
+		}
+	}
+
 	DataFileField(GetFieldResult *state, const char *end, unsigned row, unsigned column)
 	    : state_(state)
 	    , end_(end)
@@ -56,10 +89,10 @@ class DataFileField {
 	 * use with operator* or repeated calls to parseInt (even with different types).
 	 * @tparam T an Integral type supported by std::from_chars
 	 * @param destination value to store the result of successful parsing
-	 * @return the error code from std::from_chars
+	 * @return an error code corresponding to the from_chars result if parsing failed
 	 */
 	template <typename T>
-	[[nodiscard]] std::errc parseInt(T &destination)
+	[[nodiscard]] tl::expected<void, Error> parseInt(T &destination)
 	{
 		std::from_chars_result result {};
 		if (state_->status == GetFieldResult::Status::ReadyToRead) {
@@ -74,18 +107,56 @@ class DataFileField {
 		} else {
 			result = std::from_chars(state_->value.data(), end_, destination);
 		}
-		return result.ec;
+
+		return mapError(result.ec);
 	}
 
 	template <typename T>
-	[[nodiscard]] tl::expected<T, std::errc> asInt()
+	[[nodiscard]] tl::expected<T, Error> asInt()
 	{
 		T value = 0;
-		auto parseIntResult = parseInt(value);
-		if (parseIntResult == std::errc()) {
-			return value;
+		return parseInt(value).map([value]() { return value; });
+	}
+
+	/**
+	 * @brief Attempts to parse the current field as a fixed point value with 6 bits for the fraction
+	 *
+	 * You can freely interleave this method with calls to operator*. If this is the first value
+	 * access since the last advance this will scan the current field and store it for later
+	 * use with operator* or repeated calls to parseInt/Fixed6 (even with different types).
+	 * @tparam T an Integral type supported by std::from_chars
+	 * @param destination value to store the result of successful parsing
+	 * @return an error code equivalent to what you'd get from from_chars if parsing failed
+	 */
+	template <typename T>
+	[[nodiscard]] tl::expected<void, Error> parseFixed6(T &destination)
+	{
+		ParseIntResult<T> parseResult;
+		if (state_->status == GetFieldResult::Status::ReadyToRead) {
+			const char *begin = state_->next;
+			// first read, consume digits
+			parseResult = ParseFixed6<T>({ begin, static_cast<size_t>(end_ - begin) }, &state_->next);
+			// then read the remainder of the field
+			*state_ = GetNextField(state_->next, end_);
+			// and prepend what was already parsed
+			state_->value = { begin, (state_->value.data() - begin) + state_->value.size() };
+		} else {
+			parseResult = ParseFixed6<T>(state_->value);
 		}
-		return tl::unexpected { parseIntResult };
+
+		if (parseResult.has_value()) {
+			destination = parseResult.value();
+			return {};
+		} else {
+			return mapError(parseResult.error());
+		}
+	}
+
+	template <typename T>
+	[[nodiscard]] tl::expected<T, Error> asFixed6()
+	{
+		T value = 0;
+		return parseFixed6(value).map([value]() { return value; });
 	}
 
 	/**

Source/playerdat.cpp

@@ -120,20 +120,20 @@ void ReloadExperienceData()
 			case ExperienceColumn::Level: {
 				auto parseIntResult = field.parseInt(level);
 
-				if (parseIntResult != std::errc()) {
+				if (!parseIntResult.has_value()) {
 					if (*field == "MaxLevel") {
 						skipRecord = true;
 					} else {
-						DataFile::reportFatalFieldError(parseIntResult, filename, "Level", field);
+						DataFile::reportFatalFieldError(parseIntResult.error(), filename, "Level", field);
 					}
 				}
 			} break;
 
 			case ExperienceColumn::Experience: {
 				auto parseIntResult = field.parseInt(experience);
 
-				if (parseIntResult != std::errc()) {
-					DataFile::reportFatalFieldError(parseIntResult, filename, "Experience", field);
+				if (!parseIntResult.has_value()) {
+					DataFile::reportFatalFieldError(parseIntResult.error(), filename, "Experience", field);
 				}
 			} break;
 

Source/utils/parse_int.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <algorithm>
 #include <charconv>
 #include <string_view>
 #include <system_error>
@@ -19,10 +20,13 @@ using ParseIntResult = tl::expected<IntT, ParseIntError>;
 template <typename IntT>
 ParseIntResult<IntT> ParseInt(
     std::string_view str, IntT min = std::numeric_limits<IntT>::min(),
-    IntT max = std::numeric_limits<IntT>::max())
+    IntT max = std::numeric_limits<IntT>::max(), const char **endOfParse = nullptr)
 {
 	IntT value;
 	const std::from_chars_result result = std::from_chars(str.data(), str.data() + str.size(), value);
+	if (endOfParse != nullptr) {
+		*endOfParse = result.ptr;
+	}
 	if (result.ec == std::errc::invalid_argument)
 		return tl::unexpected(ParseIntError::ParseError);
 	if (result.ec == std::errc::result_out_of_range || value < min || value > max)
@@ -32,4 +36,99 @@ ParseIntResult<IntT> ParseInt(
 	return value;
 }
 
+inline uint8_t ParseFixed6Fraction(std::string_view str, const char **endOfParse = nullptr)
+{
+	unsigned numDigits = 0;
+	uint32_t decimalFraction = 0;
+
+	// Read at most 7 digits, at that threshold we're able to determine an exact rounding for 6 bit fixed point numbers
+	while (!str.empty() && numDigits < 7) {
+		if (str[0] < '0' || str[0] > '9') {
+			break;
+		}
+		decimalFraction = decimalFraction * 10 + str[0] - '0';
+		++numDigits;
+		str.remove_prefix(1);
+	}
+	if (endOfParse != nullptr) {
+		// to mimic the behaviour of std::from_chars consume all remaining digits in case the value was overly precise.
+		*endOfParse = std::find_if_not(str.data(), str.data() + str.size(), [](char character) { return character >= '0' && character <= '9'; });
+	}
+	// to ensure rounding to nearest we normalise all values to 7 decimal places
+	while (numDigits < 7) {
+		decimalFraction *= 10;
+		++numDigits;
+	}
+	// we add half the step between representable values to use integer truncation as a substitute for rounding to nearest.
+	return (decimalFraction + 78125) / 156250;
+}
+
+template <typename IntT>
+ParseIntResult<IntT> ParseFixed6(std::string_view str, const char **endOfParse = nullptr)
+{
+	if (endOfParse != nullptr) {
+		// To allow for early returns we set the end pointer to the start of the string, which is the common case for errors.
+		*endOfParse = str.data();
+	}
+
+	if (str.empty()) {
+		return tl::unexpected { ParseIntError::ParseError };
+	}
+
+	constexpr IntT minIntegerValue = std::numeric_limits<IntT>::min() >> 6;
+	constexpr IntT maxIntegerValue = std::numeric_limits<IntT>::max() >> 6;
+
+	const char *currentChar; // will be set by the call to parseInt
+	ParseIntResult<IntT> integerParseResult = ParseInt(str, minIntegerValue, maxIntegerValue, &currentChar);
+
+	bool isNegative = std::is_signed_v<IntT> && str[0] == '-';
+	bool haveDigits = integerParseResult.has_value() || integerParseResult.error() == ParseIntError::OutOfRange;
+	if (haveDigits) {
+		str.remove_prefix(static_cast<size_t>(std::distance(str.data(), currentChar)));
+	} else if (isNegative) {
+		str.remove_prefix(1);
+	}
+
+	// if the string has no leading digits we still need to try parse the fraction part
+	uint8_t fractionPart = 0;
+	if (!str.empty() && str[0] == '.') {
+		// got a fractional part to read too
+		str.remove_prefix(1); // skip past the decimal point
+
+		fractionPart = ParseFixed6Fraction(str, &currentChar);
+		haveDigits = haveDigits || str.data() != currentChar;
+	}
+
+	if (!haveDigits) {
+		// early return in case we got a string like "-.abc", don't want to set the end pointer in this case
+		return tl::unexpected { ParseIntError::ParseError };
+	}
+
+	if (endOfParse != nullptr) {
+		*endOfParse = currentChar;
+	}
+
+	if (!integerParseResult.has_value() && integerParseResult.error() == ParseIntError::OutOfRange) {
+		// if the integer parsing gave us an out of range value then we've done a bit of unnecessary
+		//  work parsing the fraction part, but it saves duplicating code.
+		return integerParseResult;
+	}
+	// integerParseResult could be a ParseError at this point because of a string like ".123" or "-.1"
+	//  so we need to default to 0 (and use the result of the minus sign check when it's relevant)
+	IntT integerPart = integerParseResult.value_or(0);
+
+	// rounding could give us a value of 64 for the fraction part (e.g. 0.993 rounds to 1.0) so we need to ensure this doesn't overflow
+	if (fractionPart >= 64 && (integerPart >= maxIntegerValue || (std::is_signed_v<IntT> && integerPart <= minIntegerValue))) {
+		return tl::unexpected { ParseIntError::OutOfRange };
+	} else {
+		IntT fixedValue = integerPart << 6;
+		if (isNegative) {
+			fixedValue -= fractionPart;
+		} else {
+			fixedValue += fractionPart;
+		}
+		return fixedValue;
+	}
+}
+
 } // namespace devilution

test/CMakeLists.txt

@@ -32,6 +32,7 @@ set(tests
   missiles_test
   pack_test
   path_test
+  parse_int_test
   player_test
   quests_test
   random_test