-
Notifications
You must be signed in to change notification settings - Fork 85
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
EnumerateSpeedLimits in OnToolGUI .... #1149
EnumerateSpeedLimits in OnToolGUI .... #1149
Conversation
- Removed SpeedUnit.CurrentlyConfigured because that is not a speed unit. - Added GlobalConfig.Main.GetDisplaySpeedUnit() method to get SpeedUnit from user config directly. - Added SpeedValue.Unlimited() convenience method for better expressing intent. - Removed EnumerateSpeedLimits from OnToolGUI in SpeedLimitsTool. The values should be generated once and reused since they don't change during runtime.
@@ -68,6 +68,8 @@ public static SpeedValue FromMph(ushort mph) | |||
public static SpeedValue FromMph(MphValue mph) | |||
=> new SpeedValue(mph.Mph / ApiConstants.SPEED_TO_MPH); | |||
|
|||
public static SpeedValue Unlimited() => new SpeedValue(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we use speed value 20f
as Unlimited (1000 km/h). 0f
has some different meaning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check GetPrevious and GetNext and
SpeedLimitsTool:636 before change
allSpeedLimits.Add(new SpeedValue(0)); // add last item: no limit
Unlimited() is basically a replacement for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 is used to display 🚫 in the speed palette
this is not the same as 20f used as unlimited. And i would prefer unlimited constant to be 20f.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so "no limit" is not the same as "unlimited"?
isn't 🚫 the sign for unlimited speed?
I am confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 is texture index for 🚫
20f is used to set the speed limit to Unlimited
you gave name Unlimited to 0f which is wrong
the value 0f in speed palette should be converted to 20f somewhere in the code, that's the confusing moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so functions like ToKmphPreciseString are wrong too?
can you show me where this 20f is? can't find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SpeedLimitManager::MAX_SPEED = 10f * 2f
SpeedLimitManager::ToGameSpeedLimit()
and ::IsValidRange()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can work with this. Merge away!
The values should be generated once and reused since they don't change during runtime.