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

SanitizeDefine fix for different cultures. #144

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hilminamli
Copy link

Used en-GB culture info in ToUpper method.
Lets say we want to convert "Android" to upper case. Without a cultureinfo, ToUpper method uses local computer's culture info. For example for Turkish Android is converted to ANDROİD. This was causing in scripting defines in player settings.

Before Fix:
before
After Fix:
after

Used en-GB culture info in ToUpper method.
Lets say we want to convert "Android" to upper case. Without a cultureinfo, ToUpper method uses local computer's culture info. For example for Turkish Android is convertoed to ANDROİD.
This was causing BUILD_ARCH_ANDRO_D scripting defines in player settings.
@robinnorth
Copy link
Collaborator

Thanks @hilminamli, that's a good catch!

String.ToUpperInvariant would be a simpler fix, would you like to update your PR to use this instead? There's also a few other instances where String.ToUpper is used that would also need this fix, so a find and replace for all of them would be appropriate.

@robinnorth robinnorth added this to the v8.0.0 milestone Jul 1, 2024
Used String.ToUpperInvariant() to avoid the varying uppercase conversion rules of different cultures.

After this changes runned build for 3 platform at same time
Android IL2CPP
PC IL2CPP
MacOS Mono
@hilminamli
Copy link
Author

Hey @robinnorth,

Just updated the PR to replace all ToUpper methods with String.ToUpperInvariant.

Super usefull tool, especially with pre and post build actions. Keep it up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants