Skip to content

Preserve shared RNG state when copying Perlin and Simplex noise - #195

Merged
HexDecimal merged 2 commits into
libtcod:mainfrom
antonsoo:fix/noise-snapshot-random-state
Oct 13, 2026
Merged

HexDecimal merged 2 commits into
libtcod:mainfrom
antonsoo:fix/noise-snapshot-random-state

Conversation

@antonsoo

Copy link
Copy Markdown
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 finally. Direct wavelet serialization keeps its existing behavior.

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.

@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.09%. Comparing base (51e4832) to head (408dc1d).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@antonsoo

This comment was marked as resolved.

@HexDecimal

Copy link
Copy Markdown
Collaborator

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 Noise's constructor. This would stop copy and pickle from advancing RNG as a side effect.

@HexDecimal

Copy link
Copy Markdown
Collaborator

Correction: The wavelet seed appears to be based on the previous noise generation, so they would not be the same data.

@HexDecimal

Copy link
Copy Markdown
Collaborator

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.

Comment thread CHANGELOG.md Outdated

@HexDecimal HexDecimal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for 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.

Comment thread tcod/noise.py
Comment on lines +361 to +364
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)

Copy link
Copy Markdown
Collaborator

Choose a reason for 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for 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.

@HexDecimal
HexDecimal merged commit d702f30 into libtcod:main Oct 13, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants