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
177 stars 41 forks source link

Update seedrandom external and use new state #166

Open JimmySilverstein opened 2 years ago

JimmySilverstein commented 2 years ago

Is your feature request related to a problem? When playing a SugarCube-based game for an extended period of time; the pull count being exceedingly high will induce a staggering performance drain during PRNGWrapper.unmarshal().

For example, if my pull count was 7 million, to quote an extreme case, during the marshalling for that PRNG object, the time taken can easily exceed 1 second. Potentially, far more for less performant machine and devices. At least what I can tell from profiling the issue.

Describe the solution you'd like. I would propose to update the seedrandom external library, which includes a suitable state export-import feature. This new mechanism provided by the library can instead supplant the code in the marshalling functions, where you recreate the PRNG object with the state object provided, instead of the pull count. Eliminating the need to manually return the state from the initial seed.

A potential issue with this is, you would be saving that state object every moment. This state object is a collection of two integers, with a 256 integer array. Substantial enough, when you consider how SugarCube may have to store this state object every moment, from my understanding.

Additional context. Link to my repo with a prototype Do note that this repo also has minor changes not directly relating to this problem, but should be clear enough to work from and inspect to see if this is an appropriate avenue to use. Link to specific commit for all of the changes to PRNG and state

Reference to seedrandom's state restoration usage. image

tmedwards commented 2 years ago

I am aware of the state feature in newer versions of Seedrandom.

 

For example, if my pull count was 7 million, to quote an extreme case, during the marshalling for that PRNG object, the time taken can easily exceed 1 second. Potentially, far more for less performant machine and devices. At least what I can tell from profiling the issue.

At an average of 7 seeded pulls per moment, which is a lot, that's an average of 1 million played moments.

If a player racks up a pull count that's even a significant fraction of 7 million, then the author/developer is doing something unwise.

I get that you're playing devil's advocate here, but you're not being remotely realistic at all.

 

A potential issue with this is, you would be saving that state object every moment. This state object is a collection of two integers, with a 256 integer array. Substantial enough, when you consider how SugarCube may have to store this state object every moment, from my understanding.

This is one of the reasons I haven't done something similar in v2. The potential save bloat makes this a non-starter.

Storage space is at a premium in the browser, with the providers available to v2, and stuffing the PRNG state into each moment is not really an option.

The storage space issue is also why v2 attempts to reduce save size by deduplicating the moments within and applying LZstring compression.

 

That said. I could switch to one of the faster algorithms—e.g., Alea, which is ~2x as fast—but that would require extensive testing to ensure that existing saves using the default RC4-based algorithm are compatible with the newer version, because breaking existing saves is not an option.

tmedwards commented 2 years ago

Reopening this. While the proposed method has issues making it unsuitable for v2, the basic idea of improving the revival speed of the seedable PRNG is fine.