Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move from JKISS PRNG to Xorshift128+ #17

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ingwarsw
Copy link
Contributor

Should make things faster and more "random".

root and others added 3 commits September 17, 2024 19:48
After a few months of playing we found that some things are not so random as we wanted..
And I found the curlpit.. poor seed initialisation for seeded version.

For example `random(8, {alopst_any_seed});` would return `0`
@cotillion
Copy link
Owner

I'm curious, were you experiencing issues with the previous implementation?
I'm not opposed to changing to Xorshift but it might be a good idea to have select generator via config.h

In the last switch there were some issues with code which expected a certain seed to always return the same random values.

@bithir
Copy link
Contributor

bithir commented Nov 18, 2024

I'm fairly sure that Kingdoms still have scenarios where this is the case (i.e. code expects a certain seed to always produce the same random value).

I do not oppose this change, but I am curious why this change is required?

As for compatibility, I would have to dig in to see what consequences it has (or, rather likely, upgrade and see what breaks) in our Mud.

@ingwarsw
Copy link
Contributor Author

Its hard to tell exactly, but we had few parts of the mud that seemed less random (there was a cases when random was going into some direction and stayed there for a while).
Its just a feeling nothing really to show "hard metrics" for.

And yes, I just found and fixed the issues with same values when using the seed.
It took me a while to encounter this..

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.

3 participants