Alter random integer generation for binding consistency

[devinamos24]

I was attempting to follow the Godot 2D game guide and encountered an error when spawning enemies. The error always happens within a few spawns.

Error:
"
USER ERROR: FATAL: Index p_index = -2 is out of bounds (size() = 3).
at: CowData::get (.\core/templates/cowdata.h:155)
"

If I am doing something wrong please let me know. Thank you for your time.

Edit: I forgot to mention that the exact lines of Kotlin code that cause the error are the:

        val mob = mobScene.instantiate() as Mob
        . . .
        addChild(mob)

DodgeCreepsGame.zip

[devinamos24]

Okay, I found the issue, and I feel quite dumb for not reading the documentation better. When calling the GD.randi() function, the unsigned integer is truncated using the Long.toInt() helper function. The result is a random signed integer, which differs in behavior to differ from GDScript and C#. Normally this would not cause any issues as the modulo division operator would drop the negative sign, but of course Kotlin's mod operator does not do floor division by default.

Anyway, I have a recommendation that would help keep the binding consistent with the engine calls. Instead of GD.rng.randi() returning a Long, have it return a UInt. This would make the possible random number more predictable to developers without them having to read the documentation. Then in GD.randi(), instead of truncating the UInt, just use integer division to divide by 2, then convert it to an Int with the helper function. Here is an example:

// NOTE: In order for this to work, the VariantType UINT must be added
fun RandomNumberGenerator.randi(): UInt {
    TransferContext.writeArguments()
    TransferContext.callMethod(rawPtr, ENGINEMETHOD_ENGINECLASS_RANDOMNUMBERGENERATOR_RANDI, UINT)
    return (TransferContext.readReturnValue(UINT, false) as UInt)
}

fun GD.randi(): Int = (rng.randi() / 2u).toInt()

[CedNaru]

We will have to check how the global randi() is truly implemented in the original C++ code. The issue with dividing by 2 the random number means that the same seed won't return the same number depending on whether you use it inside or outside Kotlin, and that's also not consistent.

Also, so far we decided to return unsigned number as signed, meaning that a UInt32 is converted to Long. It's done that way, because there are many other cases in Godot tagged as unsigned that would require manual conversion to signed by the users for simple operations because Kotlin is not designed around unsigned values.
Maybe we should think again about that regarding Uint32 and Uint64.

[devinamos24]

I did some digging and found the implementation of the random number generation. It uses the PCG algorithm.

As far as unsigned integers in Kotlin is concerned, they have been stable since 1.5. The built-in types: UByte, UShort, UInt, and ULong should all have the same functionality as their signed counterparts. However, you cannot perform operations between unsigned and signed values. Because of this detail, I think it would be best to change GD.randi() to this:

internal interface GDRandom {
      . . .
      fun randi(): Long =  rng.randi()
}

Here is an example of what could go wrong, and why this change would be really awesome.

val num1: Long = GD.rng.randi() // We'll say this is 3_000_000_000
val num2: Int = num1.toInt() // This is what GD.randi() would have returned if the above number was returned. This would be -1_294_967_296

// The problem with this behavior can be seen below
val mobTypes = animatedSprite2D.spriteFrames!!.getAnimationNames()
animatedSprite2D.play(mobTypes[GD.randi() % mobTypes.size].asStringName()) // This would cause an error because of the negative number from GD.randi()

// Instead the developer would have to do this
animatedSprite2D.play(mobTypes[(GD.rng.randi() % mobTypes.size).toInt()].asStringName()) // This would not cause an error because it would keep the positive number

// Here is what would happen with the change in mind
val num1: Long = GD.rng.randi() // We'll say this is 3_000_000_000
val num2: Long = GD.randi() // GD.randi() would not convert to int, it would keep it as Long

// Here is the situation above with the changes in place
val mobTypes = animatedSprite2D.spriteFrames!!.getAnimationNames()
animatedSprite2D.play(mobTypes[(GD.randi() % mobTypes.size).toInt()].asStringName()) // This would not cause an error

For now, the simple workaround would be to use GD.rng.randi() instead of GD.randi(). Let me know what you think, thank you for your time!

[CedNaru]

I removed the ambiguity. Now GD.randi() will always return a Long that can nicely contains the range of a unsigned 32 bits integer