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

#438 Speedlimit refactor #475

Merged
merged 8 commits into from
Aug 2, 2019

Conversation

kvakvs
Copy link
Collaborator

@kvakvs kvakvs commented Aug 1, 2019

Fixes #438 - by Victor

  • Move all constants (The decision should be based on where the constant is used)
    • either to SpeedLimitManager
    • or to a UI related class (SpeedLimitsTool?)
  • Move methods EnumerateSpeedLimits, ToMphPreciseString, ToKmphPreciseString, ToMphRounded, ToMphPrecise, ToKmphPrecise, ToKmphRounded, GetPrevious, GetNext, GetVerticalTextureScale to a UI related class (SpeedLimitsTool?)
  • Move NearlyEqual and IsZero to a (new?) utility class, e.g. FloatUtils
  • Move IsValidRange to SpeedLimitManager
  • Modify VehicleBehaviorManager.FindBestLane() such that the variable optImprovementSpeed stores velocitiy in game units (and not in km/h).
  • Modify member variables MinMinSafeSpeedImprovement and MaxMinSafeSpeedImprovement in DynamicLaneSelection such that they hold velocities in game units (and not in km/h).
  • If required, add two members velocity (float) and unit (SpeedUnit) to the SpeedLimit struct and update UI code to use it properly (as a composite value). If not required, remove the struct entirely.

Introduces new struct SpeedValue which contains game speed float, and can do conversions with Velocity magnitudes (8x the game speed) and Mph / Kmph. It can potentially replace float everywhere in the game logic but it spreads like a plague, once you replace something, the compiler will complain until everything became the new data type. Worth only if there is no performance loss, and i am not sure about Mono 2.0.

@originalfoo originalfoo added this to the 11.0 milestone Aug 1, 2019
@originalfoo originalfoo added code cleanup Refactor code, remove old code, improve maintainability technical Tasks that need to be performed in order to improve quality and maintainability labels Aug 1, 2019
TLM/TLM/Util/FloatUtil.cs Outdated Show resolved Hide resolved
@kvakvs
Copy link
Collaborator Author

kvakvs commented Aug 1, 2019

@dymanoid i hope [Serializable] is not adding any extra data fields to the struct. It is not used now, but may be used later if we replace more floats with this struct.
Also: I'm worried how bad is Mono 2.0 perf penalty on a struct with a float.

@dymanoid
Copy link
Contributor

dymanoid commented Aug 1, 2019

You only need the [Serializable] attribute if you want to use BinaryFormatter for serialization (I suppose that is not the case in TM:PE, but I'm not sure). For all other serialization types (including XML), you don't need this. It won't change the struct footprint though, so there is no harm in having it applied.

A struct with a single float should be as fast as a single float. If you are concerned about the performance in Mono, just benchmark it (using BenchmarkDotNet).

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

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

Tested in game last night and everything seemed to be working fine.

LGTM! 👍

Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kvakvs kvakvs merged commit d9f0f51 into CitiesSkylinesMods:master Aug 2, 2019
@originalfoo originalfoo added the SPEED LIMITS Feature: Speed limits label Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Refactor code, remove old code, improve maintainability SPEED LIMITS Feature: Speed limits technical Tasks that need to be performed in order to improve quality and maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor SpeedLimit struct
4 participants