Skip to content

Implement reproducible random#307

Merged
ForNeVeR merged 6 commits intoForNeVeR:mainfrom
evgTSV:ran-gen
Nov 27, 2025
Merged

Implement reproducible random#307
ForNeVeR merged 6 commits intoForNeVeR:mainfrom
evgTSV:ran-gen

Conversation

@evgTSV
Copy link
Contributor

@evgTSV evgTSV commented Oct 24, 2025

Implement reproducible random (xoshiro256++), similar algorithm is used in dotnet.

Issue #276

@ForNeVeR ForNeVeR self-requested a review November 16, 2025 23:03
@ForNeVeR ForNeVeR self-assigned this Nov 16, 2025
Copy link
Owner

@ForNeVeR ForNeVeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment on lines 31 to 41
member _.NextFishId() =
idGenerator.GetFishId()
idGenerator.NextIdWithPrefix(FishId.prefix) |> FishId

member _.NextBombId() =
idGenerator.GetBombId()
idGenerator.NextIdWithPrefix(BombId.prefix) |> BombId

member _.NextBonusId() =
idGenerator.GetBonusId()
idGenerator.NextIdWithPrefix(BonusId.prefix) |> BonusId
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I don't understand why these methods were introduced in the ReproducibleRandom class to begin with; they are not supposed to be random at all.

Let's not touch these and SequentialIdGenerator in this PR? I'll open a separate issue to move them out of this type.

The SequentialIdGenerator was supposed to be sequential, not random. Moreover, random by its very definition is not unique, and uniqueness is what we need from the identifiers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's move it another PR

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#319.

Copy link
Owner

@ForNeVeR ForNeVeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ForNeVeR ForNeVeR merged commit c79e7fc into ForNeVeR:main Nov 27, 2025
12 of 13 checks passed
@evgTSV evgTSV deleted the ran-gen branch November 28, 2025 18:31
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