Skip to content
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.

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

StringUtil: Make TryParse of floats handle comma and dot decimal separators. #8541

Merged
merged 1 commit into from
Jan 5, 2020

Conversation

jordan-woyak
Copy link
Member

This should fix: https://bugs.dolphin-emu.org/issues/11899
(floats fail to parse on locales with non-dot decimal separators)

I also noticed signed integer and enum types were being handled by the overload intended for floats so I generalized the integer version of TryParse.

@AdmiralCurtiss
Copy link
Contributor

This instead causes problems if someone has thousand-separators. Shouldn't we just force the C locale for config files?

@jordan-woyak
Copy link
Member Author

@AdmiralCurtiss It looks like our "ToString" functions do not use thousand-separator format strings so this shouldn't be a problem.
I agree we should save in the "C" locale but we still need to handle loading of existing non-"C"-locale configs.

@JosJuice
Copy link
Member

Yeah, I can't recall Dolphin ever writing thousands separators to config files.

@CookiePLMonster
Copy link
Contributor

Decimal separators are rather rare I think and I'd be surprised if they were written in configs - for what it's worth, I think it's worth going as far as breaking some configs if it means achieving perfect consistency and making configs fully locale agnostic moving forward. It's a bit surprising they were following locales in the first place...

@JosJuice
Copy link
Member

JosJuice commented Jan 3, 2020

Decimal separators are common in controller config INIs.

@CookiePLMonster
Copy link
Contributor

Ugh my bad, of course I meant thousands separators.

Source/Core/Common/StringUtil.h Outdated Show resolved Hide resolved
Source/Core/Common/StringUtil.h Outdated Show resolved Hide resolved
Source/Core/Common/StringUtil.h Show resolved Hide resolved
@leoetlino leoetlino merged commit f35f4f2 into dolphin-emu:master Jan 5, 2020
@jordan-woyak jordan-woyak deleted the float-parse-fix branch December 30, 2020 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants