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

Random Number Generation class #7199

Closed
chanon opened this issue Nov 27, 2016 · 30 comments
Closed

Random Number Generation class #7199

chanon opened this issue Nov 27, 2016 · 30 comments

Comments

@chanon
Copy link
Contributor

chanon commented Nov 27, 2016

I find Godot's random number generation methods to be somewhat lacking.
I think for a game engine, there should be more random number generation methods built-in.

It would be nice if Godot implemented something like this:
https://docs.python.org/2/library/random.html

I've never written Python but since GDScript is based on it, I looked up what rng features Python had, and it looks a lot better than what GDScript has.

What is needed is a class like that that can encapsulate random number generation from a specified seed. Right now what we have are methods that use a shared global seed - but in game dev, you need separate RNGs with separate seeds for things such as an RNG for randomly generating procedural worlds with a 'world seed', another RNG for particles and non-important stuff - etc.

I have tried using rand_seed ... but there is no documentation on what type it returns (it looks like an int) and no documentation on the possible range.

So again, it would be great if Godot could provide an easy to use and optimized RNG class in GDScript encapsulating a seed and exposing methods such as

_init(seed = -1) - allows initializing the seed upon construction, default value means random seed
rand() - returns float 0-1
randi(max)
randf(max)
randf_range(from, to)
randi_range(from, to)

and even other methods like Python provides such as

(using Python's names)
choice(array) - returns a random element in the array
sample(n, array) - returns new array with n elements randomed from input array
shuffle(array) - shuffle the array in place

etc.

I was about to build a class like this built on rand_seed, but as noted, I don't even know what range the values can be. Also I'm not sure what algorithm is being used for it and how good it is.

@puppetmaster-
Copy link

What I'm really missing is this:
var myRandom = Random.new()

@chanon
Copy link
Contributor Author

chanon commented Nov 27, 2016

What I'm really missing is this:
var myRandom = Random.new()

Yes, something like that is needed and it should be able to accept an optional seed:
var worldRNG = Random.new(worldSeed)

@bojidar-bg
Copy link
Contributor

Related to #5852, (maybe) #6235 and (maybe) #1268.

(to sum up the first two of those issues, plus the rest of the thread, plus the python docs), we might have the following functions in the Random class:

  • get_int(from = 0, to = <something big>) or get_int(max = <something big>) + get_int_range(from, to)
  • get_float(from = 0, to = 1) or get_float(max = 1) + get_float_range(from, to)
  • get_normal_float(mean/mu, sigma), get_exp_float(mean/r), get_poisson_float(mean/lambda), etc.
  • pick(array), shuffle(array), pick_multiple(array, amount) ...
  • Random([seed]), Random(Random.ALGO_*, [seed])

@lukaskotik
Copy link

Poisson is not continuous distribution, so I would omit _float from the name. And what about to change get_ to rand_? If I understand it correctly get_float, get_int should generate random number from an uniform distribution, so something like _unifcont, _unifdics (from continuous, resp. discrete uniform distribution) maybe more appropriate name. Finally I would also suggest to add binomial distribution. It can be easily derived from uniform distribution on interval [0, 1] but such straightforward approach is not efficient.

@bojidar-bg
Copy link
Contributor

@lukaskotik I think we should design the class not for people well versed in the terminology of statistics, but for normal people that are just getting into gamedev with godot.
Therefore, making the function named rand_unifcont would completely throw off those people, to which it might read as random unicorn 😉
Though, if you insist, the functions might be called get_uniform_float/rand_uniform_float.

Still, I agree that naming the functions rand_float and rand_int instead of get_* might be nicer. Anyway, here is an example of how each would look in GDScript:

var world_random
func _ready():
  world_random = Random.new()
  world_random.set_algorithm(Random.ALGO_MT) # for example
  world_random.set_seed(123)
  generate_world(4, 200)

func generate_world(level, size):
  # rand_*
  for i in range(world_random.rand_int_range(100 + 10 * level, 150 + 20 * level)):
    var obstacle = generate_obstacle()
    obstacle.set_pos(Vector2(
      world_random.rand_float_range(-size, +size),
      world_random.rand_float_range(-size, +size)
    ))
    if world_random.rand_float() < 0.1 + 0.01 * level:
      obstacle.set_type(OBSTACLE_LAVA)
    add_child(obstacle)
  # get_*
  for i in range(world_random.get_int_range(3 + 6 * level, 4 + 7 * level)):
    var enemy = generate_enemy()
    enemy.set_pos(Vector2(
      world_random.get_float_range(-size, +size),
      world_random.get_float_range(-size, +size)
    ))
    if world_random.get_float() < 0.01 * (level-3):
      enemy.set_boss(true)
    add_child(enemy)

@lukaskotik
Copy link

@bojidar-bg You are right. It can be misleading for a gamedev.
It was just a proposal. get_... is OK for me. It is true that GDscript uses get_ terminology, so most likely everyone would expect functions beginning with get and not with rand. Finally, I prefer get_uniform_float and get_uniform_int since it clearly says what it does and the naming logic corresponds to get_normal_float etc.
Anyway, random_unicorn is also great name for a function ;)

@bojidar-bg
Copy link
Contributor

Then... should it be get_normal_float_range or something else?

@lukaskotik
Copy link

lukaskotik commented Nov 29, 2016

Uniform distribution is defined on an interval, so I assume that the term _range reminds the user that the bounds of the interval have to specified. Normal distribution is defined on whole R (real numbers, float in programming terminology) so I would not add _range. I would not even include _range to exponential distribution that is defined on [0, +infinity).
But maybe I would omit the _range from the name and just made one function with parameters min, max (or from, to or lower_bound, upper_bound - what does sound you better?) set to 0, 1 by default: get_uniform_float(float min = 0, float max = 1). So calling get_uniform_float() returns a random number between 0, 1, generated from uniform distribution - it is something, I believe, majority of game devs are used to. And calling get_uniform_float(10.5, 666.6) returns a random number from the interval [10.5, 666.6].

What about the following?

Continuous distributions:

  • get_uniform_float(float min = 0, float max = 1) (or get_unif_float(float min = 0, float max = 1)?) to generate a random number from uniform distribution on [min, max]
  • get_exp_float(float rate), rate > 0, to generate a random number from exponential distribution with parameter rate (mean = 1 / rate). Or use lambda instead of rate? - if you open wiki you will see lambda. So it can be easier for an user to fast understand what this parameters means if he is used to find information on wiki.
  • get_normal_float(float mu, float sigma) to generate a random number from normal distribution with mean = mu and standard deviation = sigma > 0.

Discrete distributions:

  • get_uniform_int(int min = 0, int max) (or get_unif_int(int min = 0, int max)?) that generates a random number (integer) from discrete uniform distribution on {min, min+1, ..., max}
  • get_poisson_int(float lambda), lambda > 0 to generate a random number (integer) from Poisson distribution with mean lambda
  • get_binom_int(int n, float p), n > 0, 0<p<1 to generate a random number from binomial distribution for number of trials n and probability of trial success p

And maybe

  • get_geometric_int(float p), 0<p<1, to generate a random number (0,1,2,...) from geometric distribution (number of failures before the first success) with probability of success p

It is a question whether _float and _int are necessary? The advantage of it is that it clearly shows to the user what kind of distribution it is (continuous or discrete) and what is the output.

All these distributions are quite common and quite natural (have nice interpretations) so having them in Godot would be nice! I don't know if someone could feel desire to have some other probabilistic distributions (Pareto, gamma, hypergeometric, ...?)

@bojidar-bg
Copy link
Contributor

@lukaskotik I think that's a nice selection of probability functions! (my terminology is off probably)

About the _int and _float, I think they are necessary for being way easier on newcomers, since cont and disc are too unicornish 😃

About the parameter names, I don't know if we should keep the "official" names, or would it be better to call most of them just mean, for better DX (Developer Experience).

Finally, about the range, I wondered really, really much about it, but, consider the following case:

var my_0_1_float = random.get_uniform_float()
# Yay, works
var my_0_100_float = random.get_uniform_float(100)
# Uh, why does it error here? OR Why does it never return numbers between 0 and 1?
var my_0_100_float = random.get_unform_float() * 100
# Eureka, that works!
var my_2_100_float = random.get_unform_float(2, 100)
# Nice, this also works!

I.e. some people might/would/won't mistake the first parameter for max, in case the second isn't given.

But, on the opposite side of the coin, get_uniform_float_range sounds like it would return an array of floats uniformly spaced between min and max. So, I guess, no _range variation shall be better 👍 💯

@lukaskotik
Copy link

lukaskotik commented Nov 29, 2016

I was thinking about completely omit _int, _float (or _disc, _cont ;) ) since the fact whether the distribution is continuous or discrete is "included" in the definition (hence name) of distribution (except uniform distribution that can be both continuous or discrete). But I totally agree that _int and _float are much easier for almost everyone who wants to play with random numbers and does not have experience with probabilistic distributions.

About the names of parameters. Maybe we can use the same naming conventions as Python.numpy.random?
And what about to also have a size parameter to generate an array of random numbers of a given size? I think it can be quite useful and efficient for games that need high level of randomness.

I see your point with the _range. And I agree that it can produce many Q&A and Facebook questions :). I personally prefer the variation without _range but I don't know if it is better.
What is the opinion of the others?

@puppetmaster-
Copy link

puppetmaster- commented Nov 29, 2016

SUMMARY

Initializing

  • Random.new()
  • Random.new(seed) as @bojidar-bg commented

Setter

  • random.set_algorithm(Random.ALGO_MT)
  • random.set_seed(seed)
  • random.set_size()

Getter (return depending on algorithm/seed/size set)

  • random.get_bool()
  • random.get_int()
  • random.get_float()

Range (same arguments like in for iterating)

  • random.get_int_range() as @bojidar-bg commented
  • random.get_float_range() as @bojidar-bg commented
  • random.get_int_in_range()
  • random.get_float_in_range()

Do we need to randomize()? I don't like this function, it should do it automatically after initializing.

@chanon
Copy link
Contributor Author

chanon commented Nov 30, 2016

Great ideas/discussion.

I'm fine with any naming convention .. just make sure the class/methods have good documentation

@bojidar-bg
Copy link
Contributor

@puppetmaster- Ok, but... there is a slight problem -- we cannot pass arguments to constructors from GDScript, due to the nature of the GDNativeClass class (since its .new function isn't vararg, nor it can be bound for each class seperatelly) (and yeah, that's the reason we don't have any static methods accessible to GDScript...).
Second, we can't have _range functions, since it sounds like it would return a range of integers, not an integer in range.

@puppetmaster-
Copy link

puppetmaster- commented Nov 30, 2016

I updated my comment, what do you think?
My question about randomize() function is still unanswered.

@bojidar-bg
Copy link
Contributor

I think it would be nice to have randomize, as it helps prototype games faster. It would be just a shortcut for set_seed(<get_time>) anyway.

@lmbarros
Copy link
Contributor

lmbarros commented Mar 2, 2017

Does anyone know if this feature would be well received by the Godot devs? I actually started to sketch an implementation, but I think I will not spend much more time in it unless it has a good chance of being merged.

If you are curious, the code is in the random_number_generators branch of my Godot fork. There are some points I'd like to change, and, of course, I need to add more random number generators and distributions (which would be basically a port from code I previously wrote in other languages.

@chanon
Copy link
Contributor Author

chanon commented Mar 2, 2017

I have no idea, but just wanted to add that this random number generator might be interesting:
http://www.pcg-random.org

@akien-mga
Copy link
Member

I have no idea, but just wanted to add that this random number generator might be interesting: http://www.pcg-random.org

That's already what Godot 3.0 uses, implemented by @tagcup.

@ghost
Copy link

ghost commented Mar 2, 2017

I'm totally for this.

The only PRNG had only one variable as the state (called seed), but the current minimal PCG has two: seed and inc. Due to retrofitting to the old API, we're currently using the default inc. A class can encapsulate both variables and would be future-proof in that regard.

@ghost
Copy link

ghost commented Mar 2, 2017

@lmbarros Our current PRNG is already a very good one (see there also for comparisons with LCG, which has poor statistical quality). All core the implementation is already there, I'd say we just need a wrapper for GDScript for existing functions for basic things. Others (normal etc) can be built on top of PCG.

@lmbarros
Copy link
Contributor

lmbarros commented Mar 2, 2017 via email

@ghost
Copy link

ghost commented Mar 2, 2017

  1. Add a few more distributions beyond uniform (this would be the most important point for me).

These can be built on top of a basic PRNG class, so I'd suggest leaving those out from the PRNG class, and make additional functions/classes that accept the basic PRNG as parameters. This also solves the 2nd point you made.

  1. Allow users to be sure that they can create reproducible pseudorandom sequences even between Godot releases.

This isn't up to me, but I don't think Godot makes such promises in general. If you want that level of guarantees, then I guess you should be using your own (basic) PRNG module.

So overall, I'd recommend splitting this into two parts: base PRNG class which can be called "source", and a generic API for stuff like returning numbers in intervals, or from different probability distributions, which makes use of source. Something similar to Go's rand API would be nice.

@lmbarros
Copy link
Contributor

lmbarros commented Mar 2, 2017 via email

@ghost
Copy link

ghost commented Mar 2, 2017

Sure, sound good.
BTW, since the default integer type in GDScript is 64-bits now, I guess we should define a source to be a class with a method rand() (or something else, just an example) returning uint64_t. Seed and other parameters can go into its constructor.
I guess this also means we should switch to PCG64 (current implementation is PCG32).

About such big decisions (like compatibility promises across versions), you should talk to @reduz or @akien-mga.

@lmbarros
Copy link
Contributor

lmbarros commented Mar 9, 2017

So, I have added a pull request with the RNG stuff I wrote so far. I don't think it really really ready to be merged, but maybe we can discuss the merits and flaws of my design and implementation in the pull request itself.

@akien-mga akien-mga changed the title GDScript feature request: Random Number Generation class Random Number Generation class Jun 29, 2017
@JarLowrey
Copy link

JarLowrey commented Jul 5, 2018

+1 for a sample or shuffle function.

Edit: Looks like shuffle is in there already

@Calinou
Copy link
Member

Calinou commented Jul 6, 2018

+1 for a sample or shuffle function.

Please refrain from bumping issues without contributing new information; use the 👍 reaction button on the first post instead.

Zirak added a commit to Zirak/godot that referenced this issue Jul 7, 2018
Begin to address godotengine#7199 and godotengine#7989, among others. Add a Random class implementing
the existing interface for random number generation, acting as a thin wrapper
around the current functionality.

The random functions in the Math namespace operate on a global instance of this
class, keeping backwards-compatibility and simple use cases.

The code had to be split into two classes, RandomBase and Random, as it seems
like reference.h relies on math_funcs.h, and thus math_funcs cannot itself
instance a reference.
@Zireael07
Copy link
Contributor

We have randi() and randf() already, why is this not closed?

@lukaskotik
Copy link

@Zireael07
Since there exist much more useful distributions of random variables then only the uniform distribution.

@akien-mga
Copy link
Member

Fixed by #22314.

@akien-mga akien-mga added this to the 3.1 milestone Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants