Add Array.pick_random() as a shorthand for Array[randi() % Array.size()]

[noidexe]

Describe the project you are working on

In most my games I sometimes have to pick a random element from an array (of cards, types of enemies, etc.).

Describe the problem or limitation you are having in your project

To get a random element I have to do array[ randi() % array.size() ] while also checking that it is not empty, which impacts readability and can be error prone, especially if the array has a long name.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add Array.pick_random() (or any method name you think makes the most sense) as syntactic sugar for Array[ randi() % Array.size() ]

It makes the code easier to read, reduces errors and makes it more accesible to beginners since they have an easy way to get random elements without having to understand that randi() give any possible int value and how modulo is use to restrict those values.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

A new pick_random() method will be added to arrays which does the Array[randi() % Array.size()] in C++. It could throw an error or return null for empty arrays. I'd like your input on which would be the best alternative.

I can take care of implementing it since it's a simple enough change.

If this enhancement will not be used often, can it be worked around with a few lines of script?

I can, but it is something that will be used often.

Is there a reason why this should be core and not an add-on in the asset library?

It's a method addition to Array so it has to be core.

[rainlizard]

This is a very useful one. I've had it in a singleton for a long time, I named mine after the GameMaker function, choose().

var rng = RandomNumberGenerator.new()

func choose(array): # Usage: Random.choose([thing,thing,thing])
	return array[randi() % array.size()]

func randi_range(from,to):
	return rng.randi_range(from,to)

randi_range has already been added to global functions for 4.0 (it's just not in 3.3), so adding the other one I use all the time would be a nice addition too, then I could dump this unnecessary singleton. What makes it so useful is that it's an array so you can put anything inside of it, not just randomly picking between specific numbers, but also randomly picking between scene files and any other resource.

[noidexe]

@rainlizard Yeah I had something similar which also worked with dictionaries, but I think it'd be better if it is a method so that the user will come across it when searching the class reference for the data type.

Assuming the proposal gets traction I have some extra discussion topics:

I mentioned Array, does it makes sense to add pick_random() to all container types?

Pool/Packed*Array: C++ equivalent of array[ randi() % array.size() ]
Dictionary: C++ equivalent of dictionary[ dictionary.keys()[randi() % dictionary.size() ]]
String: C++ equivalent of string[ randi() % string.length() ] (Not sure if this one is particularly useful)

How should we handle the method being called on an empty container?

Option A: Return an error.

The user will have to check if the container is not empty before calling pick_random()

Option B: Return null.

The user won't get an error when calling pick_random() but might get an error later when trying to use the returned value. Also, null or any other value used to indicate an error could be actually one of the valid values inside the container leading to ambiguity.

Option C: Return a user defined default.

In this case the method signature would be pick_random( default : Variant = null ) in which the user can optionally supply a default value to return.

Option D: Impement both A and C

This can be similar to how get_node() and get_node_or_null() work.

[sairam4123]

I feel like Option C would be really good.

[Xrayez]

This is implemented in Goost as Random.choice() method (name inspired by Python). It currently handles array sequences (Array, Pool*Array) and String. I think Dictionary could also be supported if we treat keys equivalently to integer indices as in Array. Yet the same can be achieved with Random.choice(Dictionary.values()), and Random.choice(Dictionary.keys()) is also possible, so there's no ambiguity.

Random and Random2D are singletons which provide other useful and convenient randomization methods such as this. 🙂

---

Option C: Return a user defined default.

In this case the method signature would be pick_random( default : Variant = null ) in which the user can optionally supply a default value to return.

Sounds like a good idea to me at first thought. However, by picking a random element, there's an assumption that container is not empty. Returning null by default without printing an error could potentially lead to difficult to debug problems.

For reference, Python raises IndexError: https://docs.python.org/3/library/random.html#random.choice, so I'd go for Option A.

[fire-forge]

It would be a good idea to add an optional parameter to this method that takes a RandomNumberGenerator. If a value is passed to it, then it would use the RandomNumberGenerator.randi() method, otherwise it would use the global randi() method.

I mainly want this for things like procedural generation, where the result needs to be deterministic and the same random number needs to be chosen every time given the same seed.

Code example:

var rng: = RandomNumberGenerator.new()
var array: = ["a", "b", "c", "d"]

array.pick_random(rng) # Uses the RandomNumberGenerator's randi() function
array.pick_random() # Uses the global randi() function

[Xrayez]

I mainly want this for things like procedural generation, where the result needs to be deterministic and the same random number needs to be chosen every time given the same seed.

This is one of the reasons why I went for implementing and exposing Random singleton/class in Goost, so you can also control the seed prior to using all randomization methods. For instance, in Godot you cannot rely on Array.shuffle() to produce expected random sequence mainly because something else in the engine could interfere with internal random state. In Goost, shuffle method is implemented in RNG derived class, see Random.shuffle().

Also, it's too easy for randomization methods to creep into core data structures. If Godot goes this route, I think it would make sense to implement such methods in RandomNumberGenerator class, and possibly exposing those methods globally in some way, see for instance #1741.

But if Godot prefers to have lean-and-mean API, then technically even what's proposed here could be achieved with Array.shuffle() already:

# Assuming array is not empty:
array.shuffle()
var random_pick = array[0]

Of course, this is at the cost of performance, because elements need to be actually rearranged in the array.

[Xrayez]

I mentioned Array, does it makes sense to add pick_random() to all container types?

I've added support for Dictionary in goostengine/goost#140 at Goost, works alright and implementation-wise should be hopefully efficient enough using public API.

[gustavi]

Idea: a pop_random() method could be useful and implemented easily in the previous PR. It may be a dedicated proposal anyway.

[Xrayez]

Idea: a pop_random() method could be useful and implemented easily in the previous PR. It may be a dedicated proposal anyway.

I see this useful for picking elements that won't repeat (and don't need to be reused). But this is probably Goost material anyways. Added to goostengine/goost#7.

Note that popping elements from the array may not be fast, since this leads to reallocation of all successor elements in the array. But this is good enough for small arrays, should be documented.

But this could also be optimized by moving the back of array to the index of the element to be popped, and then pop back the array. However, this will lead to reordering elements in the original array, which may not be desired.

[fire-forge]

I think this should be added to the RandomNumberGenerator class instead of Array so that it can be used to get consistent results with a seed. This is especially important for procedural generation.

[akien-mga]

Implemented by godotengine/godot#67444.