Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes float parsing and serialization culture-invariant in several core math and serialization utilities, and cleans up a few encoding/typographical issues in comments and third-party headers.
Changes:
- Standardizes string and float conversions in
Sandbox.Systemto useCultureInfo.InvariantCultureorFormattableString.Invariant, covering translation utilities, serialized properties, andRangedFloat. - Updates vector/rotation/angles
Parsemethods to use invariant culture to match other math types and parsing utilities. - Fixes mis-encoded characters and punctuation in comments and copyright headers, and clarifies simplification method summaries.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| engine/Sandbox.Test.Unit/UI/Styles/StyleSetting.cs | Fixes an apostrophe encoding issue in a CSS transition unit test comment. |
| engine/Sandbox.System/Translation.cs | Makes string conversions and float parsing invariant-culture, aligning translation/conversion behavior with culture-independent expectations. |
| engine/Sandbox.System/SerializedObject/SerializedProperty.cs | Uses invariant culture when converting arbitrary values to strings in serialized properties. |
| engine/Sandbox.System/Math/Vector4.cs | Updates Vector4.Parse/TryParse default path to use invariant culture, matching other math types’ parsing behavior. |
| engine/Sandbox.System/Math/Rotation.cs | Routes default Rotation.Parse/TryParse calls through invariant-culture parsing for consistent numeric handling. |
| engine/Sandbox.System/Math/RangedFloat.cs | Uses invariant culture for parsing optional floats and for ToString, ensuring round-trippable, culture-independent range serialization. |
| engine/Sandbox.System/Math/Angles.cs | Updates Angles.Parse/TryParse default behavior to use invariant culture, consistent with other math parsing helpers and tests. |
| engine/Sandbox.System/Html/TextNode.cs | Fixes a mis-encoded copyright symbol in the Html Agility Pack–derived header comment. |
| engine/Sandbox.System/Html/ParseErrorCode.cs | Same header copyright encoding fix for the Html Agility Pack–derived enum file. |
| engine/Sandbox.System/Html/ParseError.cs | Same header copyright encoding fix for the Html Agility Pack–derived parse error class. |
| engine/Sandbox.System/Html/NodeType.cs | Same header copyright encoding fix for the Html Agility Pack–derived node type enum. |
| engine/Sandbox.System/Html/Node.cs | Same header copyright encoding fix for the Html Agility Pack–derived node class. |
| engine/Sandbox.System/Html/Document.cs | Same header copyright encoding fix for the Html Agility Pack–derived document class. |
| engine/Sandbox.Engine/Resources/Model/Model.PhysicsBodyBuilder.cs | Replaces mis-encoded dashes in SimplifyMethod XML doc comments with standard hyphens for readability and consistency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( targetType == typeof( float ) ) | ||
| { | ||
| if ( float.TryParse( from.ToString(), out var f ) ) | ||
| if ( float.TryParse( from.ToString(), CultureInfo.InvariantCulture, out var f ) ) |
There was a problem hiding this comment.
Here we’re using the float.TryParse(string, IFormatProvider, out float) overload, whereas the rest of the codebase generally uses the explicit NumberStyles.Float + provider overload for culture‑invariant float parsing (for example, Math/Length.cs and Utility/Parse.cs). For consistency with existing parsing behavior and to avoid any subtle differences in accepted formats, consider switching this to the float.TryParse(string, NumberStyles.Float, CultureInfo.InvariantCulture, out var f) pattern used elsewhere.
| if ( float.TryParse( from.ToString(), CultureInfo.InvariantCulture, out var f ) ) | |
| if ( float.TryParse( from.ToString(), NumberStyles.Float, CultureInfo.InvariantCulture, out var f ) ) |
handsomematt
left a comment
There was a problem hiding this comment.
These are good changes, but you've touched and committed so many irrelevent files.
|
Looks like |
7301fda to
6f4b25f
Compare
Pull Request
Thanks for contributing to s&box ❤️
Please fill out the sections below to help us review your change efficiently.
Summary
Fixes float parsing being dependant on culture/locale. The partial handling of this was already present, but Facepunch forgot some of these.
Motivation & Context
Fixes: N/A
Implementation Details
N/A
Screenshots / Videos (if applicable)
Checklist