tmedwards / sugarcube-2

SugarCube is a free (gratis and libre) story format for Twine/Twee.
https://www.motoslave.net/sugarcube/2/
BSD 2-Clause "Simplified" License
185 stars 42 forks source link

Bug either, array.prototype.random, and pseudo-random- either does not use pseudo-random nor does array #85

Closed Rizean closed 4 years ago

Rizean commented 4 years ago

either uses array.prototype.random which uses Array.random is deprecated. This effectively make either not useable when you use State.prng.init() as the random generated called is the native Math.random.

::StoryInit
<<run Config.saves.autosave = true>>
<<run State.prng.init()>>
<<run window.__random = random>>
<<run window.__either = either>>
<<run window.__arrayRandom = Array.from([1,3,4]).random>>

Good - __random references the expected random function

'' + __random
"function random(/* [min ,] max */) {
        let min;
        let max;

        switch (arguments.length) {
        case 0:
            throw new Error('random called with insufficient parameters');
        case 1:
            min = 0;
            max = Math.trunc(arguments[0]);
            break;
        default:
            min = Math.trunc(arguments[0]);
            max = Math.trunc(arguments[1]);
            break;
        }

        if (!Number.isInteger(min)) {
            throw new Error('random min parameter must be an integer');
        }
        if (!Number.isInteger(max)) {
            throw new Error('random max parameter must be an integer');
        }

        if (min > max) {
            [min, max] = [max, min];
        }

        return Math.floor(State.random() * (max - min + 1)) + min;
    }"

Bad - __arrayRandom references the deprecated random function

'' + __arrayRandom
"value(/* DEPRECATED: [min ,] max */) {
            if (this == null) { // lazy equality for null
                throw new TypeError('Array.prototype.random called on null or undefined');
            }

            const length = this.length >>> 0;

            if (length === 0) {
                return;
            }

            const index = arguments.length === 0
                ? _random(0, length - 1)
                : _randomIndex(length, [...arguments]);

            return this[index];
        }"

Bad - __either references the deprecated Array.random function

'' + __either
"function either(/* variadic */) {
        if (arguments.length === 0) {
            return;
        }

        return Array.prototype.concat.apply([], arguments).random();
    }"
tmedwards commented 4 years ago

There's no bug here. The State.prng.init() API doc notes exactly which APIs it affects.

If you want to replace Math.random() with the seedable PRNG instance created by State.prng.init(), then you may do so. It does not happen by default for various reasons, not the least of which that 3rd-party libraries that want randomness generally don't expect/want it to deterministic.