RealityStop / Bolt.Addons.Community

A community-driven project for extending Unity Bolt
MIT License
251 stars 34 forks source link

Due to Random.Range behaves different from int to float when using int it should add 1 to max #9

Closed eka closed 5 years ago

eka commented 5 years ago

https://github.com/RealityStop/Bolt.Addons.Community/blob/73c13f27ebfb7ef035fbf3e68953b901b45a2eca/src/Fundamentals/Units/Math/RandomNumbers.cs#L116

In Random.Range for integers the max value is exclusive so "Random Numbers" will have an different behavior when used with floats vs ints

For consistency sake I suggest to add 1 to max when integers are used.

RealityStop commented 5 years ago

Sorry for missing this, somehow my email notifications are off and I've been putting the addons on ice until Bolt 2 gets a bit more stable, so I hadn't manually checked in.

Regarding the post, I'm not sure that deviating from Unity's handling of Random is good stewardship. You're absolutely right that the behavior between integer and floating point will be different, but it matches Unity's own handling of that.

Unity has arguably made that decision so that you can do things like myArray[Random.Range(0, myArray.Count)] and not get an error. Perhaps we could handle the case by having an "inclusive" toggle and better documentation on the unit to make it visually clear to the user what the unit is going to do, and support both use cases.

eka commented 5 years ago

Yeah we discussed this long time ago :D