Skip to content

fix(ini): Support plus sign character prefix in INI integer parse #2576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xezon wants to merge 3 commits into
base: main
Choose a base branch
from
+10 −2
Open

fix(ini): Support plus sign character prefix in INI integer parse #2576

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
262b1f3
fix(ini): Support plus sign character prefix in INI integer parse
xezon Apr 11, 2026
8f190fb
Fix assert
xezon Apr 11, 2026
a1fc7c8
tweak assert message
xezon Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions Core/GameEngine/Source/Common/INI/INI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1618,8 +1618,16 @@ void INI::initFromINIMulti( void *what, const MultiIniFieldParse& parseTableList
template <typename Type>
Type scanType(std::string_view token)
{
// TheSuperHackers @info std::from_chars cannot parse "-1" as uint32 so the result needs to be int64 for integers.
std::conditional_t<std::is_integral_v<Type>, Int64, Real> result{};
DEBUG_ASSERTCRASH(!token.empty(), ("token is not expected to be empty"));

// Unlike sscanf, std::from_chars cannot parse "+"
Copy link
Copy Markdown

@Skyaero42 Skyaero42 Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #2548, this 'fix' needs a comment on why we want to keep parsing the '+' sign

// TheSuperHackers @info std::from_chars does not support a leading '+' sign. Strip it to match // sscanf and accommodate mod makers who use explicit signed values (e.g. +20 / -20).

Copy link
Copy Markdown
Author

@xezon xezon Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think this level of detail is required for this comment. The comment already clarifies that from_chars behaves different to sscanf, and this difference is mitigated accordingly.

Copy link
Copy Markdown

@Caball009 Caball009 Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add to the first post that this change is not required for the default ini files.

Copy link
Copy Markdown
Author

@xezon xezon Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok did that. It is implicitly mentioned by the "Mod" label as well.

if (token[0] == '+')
Copy link
Copy Markdown

@Caball009 Caball009 Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about "+" for integers? If we only care about "+" for floating point values, we could do something like:

if constexpr (std::floating_point<Type>)
{
	// Unlike sscanf, std::from_chars cannot parse "+"
	if (token[0] == '+')
	{
		token.remove_prefix(1);
	}
}

Copy link
Copy Markdown
Author

@xezon xezon Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In INI file, integer and float are indistinguishable, so a user could add + to both and not be aware about its underlying type.

{
token.remove_prefix(1);
}

// Unlike sscanf, std::from_chars cannot parse "-" as unsigned integer
std::conditional_t<std::is_integral_v<Type>, Int64, Type> result{};
const auto [ptr, ec] = std::from_chars(token.data(), token.data() + token.size(), result);

if (ec != std::errc{})
Expand Down
Loading