GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — Preserve shared RNG state when copying Perlin and Simplex noise by antonsoo · Pull Request #195 · libtcod/python-tcod Conversation
Contributor
Copying or pickling Perlin/Simplex noise advances a caller-supplied random generator, even though neither algorithm needs new random draws to sample its existing field. Serialization initializes an unused wavelet tile through that same generator. A later dungeon generation step can therefore change just because a checkpoint was saved. import copy import pickle import tcod rng = tcod.random.Random(seed=42) noise = tcod.noise.Noise(2, seed=rng) reference = copy.deepcopy(rng) pickle.dumps(noise) print([rng.randint(0, 100) for _ in range(5)]) print([reference.randint(0, 100) for _ in range(5)]) # Before: [57, 45, 80, 84, 37] vs [7, 20, 41, 34, 13] # After: both [7, 20, 41, 34, 13] Prepare the unused wavelet tile with a copy of the native generator for Perlin/Simplex snapshots. Retain tile synchronization if a copied instance later switches to wavelet noise, and restore the original algorithm/RNG pointer in Twelve regressions cover both native RNG algorithms and shallow copy, deep copy and pickle; all fail before and pass after, including the subsequent wavelet switch. Across 96 native terrain/BSP continuations, all 32 formerly changed dungeon maps now match an uninterrupted run. The 64 already-working records remain exactly unchanged, including terrain values, random draws and partitions. Full native no-window suite: 321 passed, 95 rendering skips, with SDL dummy drivers and the pinned libtcod fixtures. Mypy and applicable project hooks pass. Linux/Python 3.12; graphical windows and other platforms were not tested.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
There was an error while loading. Please reload this page.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## main #195 +/- ## ========================================== + Coverage 86.08% 86.09% +0.01% ========================================== Files 29 29 Lines 7590 7597 +7 ========================================== + Hits 6534 6541 +7 Misses 1056 1056 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
There's a more critical issue. This code was already a hack due to libtcod's lazy initialization of the wavelet data. By fixing this hack to not advance the RNG a new issue comes up, consider the following example: rng = tcod.random.Random(seed=42) noise = tcod.noise.Noise(2, seed=rng) noise2 = tcod.noise.Noise(2, seed=rng) pickle.dumps([noise, noise2]) Because RNG is preserved, both of these noise objects will be initialized to the same wavelet data when they're meant to have different results. Fixing this issue properly would involve removing the hack, which might require changes upstream in libtcod. Alternately, the hack to force initialization of the wavelet data could be moved to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Correction: The wavelet seed appears to be based on the previous noise generation, so they would not be the same data. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Without changing libtcod, reducing the size of the hack by moving it to the constructor would be nice. But your solution seems to affect backwards compatibility the least. So I'll continue with your idea. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This is more graceful than the other options I've considered. I can fix the changelog later.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| wavelet_random = saved_random | ||
| if saved_algo != tcod.constants.NOISE_WAVELET: | ||
| # Preparing an unused algorithm must not advance a shared RNG. | ||
| wavelet_random = ffi.gc(lib.TCOD_random_save(saved_random), lib.TCOD_random_delete) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I do need this part explained to me, why this branch conditional? I would've expected the RNG to be copied for all permutations.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The condition preserves the existing behavior when wavelet is the selected algorithm. Its first sample initializes the tile and advances the shared RNG; serialization currently performs that same lazy initialization. Copying the RNG there would also change the subsequent random sequence for existing wavelet users.
For Perlin/Simplex, sampling the existing field does not advance the RNG. The serialization workaround initializes a wavelet tile that those algorithms do not use, so I copy the generator only for that case. The original and restored objects still share the prepared tile if they later switch to wavelet.
I checked first sampling and pickling against native RNG draws for both generator algorithms. Wavelet retains the same advanced sequence before and after; Perlin/Simplex pickling now retains the sequence of ordinary sampling. The branch is intended to keep this backward-compatibility boundary, and could be removed if changing direct-wavelet serialization is preferred.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.