godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.47k stars 20.26k forks source link

Random Number Generation class #7199

Closed chanon closed 5 years ago

chanon commented 7 years ago

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- commented 7 years ago

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

chanon commented 7 years ago

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 commented 7 years ago

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:

lukaskotik commented 7 years ago

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 commented 7 years ago

@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 :wink: 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 commented 7 years ago

@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 commented 7 years ago

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

lukaskotik commented 7 years ago

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:

Discrete distributions:

And maybe

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 commented 7 years ago

@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 :smiley:

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 :+1: :100:

lukaskotik commented 7 years ago

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- commented 7 years ago

SUMMARY

Initializing

Setter

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

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

chanon commented 7 years ago

Great ideas/discussion.

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

bojidar-bg commented 7 years ago

@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- commented 7 years ago

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

bojidar-bg commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

akien-mga commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

Hi @tagcup,

Yes, PCG is good, sure. I personally have three concerns I would like to solve:

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

2) Let the user create separate instances of the RNG algorithm, in order to generate different independent sequences.

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

Maybe point 3 deserves some explanation. Imagine I implemented a world-generation algorithm in Godot 2.1. My players are happy exchanging nice "world seeds" they have found. Then I upgrade to Godot 3.0, which has a different (arguably better, but different) RNG algorithm. This breaks all the world seeds my players have been sharing among each other. (This gets worse for games like the old Elite, in which the world is generated procedurally generated, but always using the same seed.)

So, my idea to solve point 3 is to be explicit about what algorithm is being used. Do I want to use PCG? Instantaite the RandPCG32 class. Wanna Xoroshiro128+? Instantiate RandXoroshiro128Plus. Different RNGs have different trade offs, so I wanted to leave this choice up to the users.

Yes, we can say that PCG will be our RNG of choice forever. I don't think I'd vote for this, but I'd agree to follow this path if this is the choice of the devs. (I can see merits in having one single RNG -- simplicity, to begin with).

(BTW, I was not suggesting to replace PCG with an LCG, I just used an LCG in my "proof of concept" because it was the simplest one I had at hand. The LCG would be just one of the RNGs available to the user.)

Oh, my, I am verbose :-)

Anyway, my main point at the moment is: I want to help to improve the random number generation in Godot, but I'd like to align the approach to follow with the devs (because I want it merged).

What do you think?

On Thu, Mar 2, 2017 at 1:18 PM, Ferenc Arn notifications@github.com wrote:

@lmbarros https://github.com/lmbarros Our current PCG is already a very good one http://www.pcg-random.org/pdf/toms-oneill-pcg-family-v1.02.pdf (see there also for comparisons with LCG, which has poor statistical quality). All the implementation is already there, I'd say we just need a wrapper for GDScript for existing functions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/7199#issuecomment-283699522, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-DaZWlqO8EsVQIPuS1BcWEB3LWaAeDks5rhuvAgaJpZM4K9Lhd .

ghost commented 7 years ago

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.

3) 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 commented 7 years ago

My design already separates the distributions/utlities from the RNG algorithms themselves, albeit in a slightly different manner.

Well, I guess I'll work a little further on this and I'll ping you again when I have something more complete.

Thanks!

PS: You said "this isn't up to me". This would be up to whom, then? :-) I just started to look at Godot more seriously, so I am still trying to figure out who is who :-)

On Thu, Mar 2, 2017 at 3:33 PM, Ferenc Arn notifications@github.com wrote:

  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 https://golang.org/pkg/math/rand/ would be nice.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/7199#issuecomment-283738780, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-DadAuZBdVRFVm1-DAQ235ff3g6LrHks5rhwuDgaJpZM4K9Lhd .

ghost commented 7 years ago

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 commented 7 years ago

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.

JarLowrey commented 6 years ago

+1 for a sample or shuffle function.

Edit: Looks like shuffle is in there already

Calinou commented 6 years ago

+1 for a sample or shuffle function.

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

Zireael07 commented 5 years ago

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

lukaskotik commented 5 years ago

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

akien-mga commented 5 years ago

Fixed by #22314.