Skip to content

Changed World Generation to use random seed numbers if not specified #116

Closed
TheShermanTanker wants to merge 12 commits into
CloudburstMC:bleedingfrom
TheShermanTanker:bleeding
Closed

Changed World Generation to use random seed numbers if not specified #116
TheShermanTanker wants to merge 12 commits into
CloudburstMC:bleedingfrom
TheShermanTanker:bleeding

Conversation

@TheShermanTanker

@TheShermanTanker TheShermanTanker commented Apr 10, 2021

Copy link
Copy Markdown
Collaborator

Currently, Cloudburst uses the world name as the seed if it is not specified in the world config, requiring users to manually put a seed in every time a new world is loaded if they do not want to get the same world over and over again. This Pull Request cleans up some method names but most importantly adds the functionality of Vanilla-like random seed generation to replace the placeholder of using world names as seeds. (I'm not expecting this PR to be accepted, but rather to raise an issue in hopes that the Cloudburst maintainers might rectify this in the future, because this isn't a very complicated thing to implement but would be extremely frustrating for users if forgotten about when Cloudburst goes into production)

EDIT: If I may ask, what is planned for the default world loading? Does Cloudburst plan to have the default world options in Cloudburst yml? I was initially going to change the default and nether worlds to not depend on the yml file alongside the changes for seed generation but didn't do so in the end

Comment thread src/main/java/org/cloudburstmc/server/Server.java Outdated
Comment thread src/main/java/org/cloudburstmc/server/Server.java Outdated
@TheShermanTanker

Copy link
Copy Markdown
Collaborator Author

Above issues rectified

@TheShermanTanker

TheShermanTanker commented Apr 14, 2021

Copy link
Copy Markdown
Collaborator Author

Bump

EDIT: Nevermind my dumbass didn't realize github doesn't work this way

Object seedObj = config.getSeed();
long seed;
if(seedObj == null) { //Auto generate seed if one was not specified
seedObj = ThreadLocalRandom.current().nextLong();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i haven't actually looked at this code in-depth for a while, but won't this be run every time the server starts, thus causing the seed to be different between restarts? the seed isn't written to config anywhere, and i'm pretty sure the seed stored in level.dat isn't used...

@TheShermanTanker TheShermanTanker Apr 15, 2021

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Whoops, looks like I forgot about that, thanks for pointing out the oversight. I would go for level.dat personally, but does Cloudburst prefer config instead?

@TheShermanTanker

Copy link
Copy Markdown
Collaborator Author

@DaMatrix I've gotten back from checking the source and it does indeed seem that the seed stored inside level.dat is used if present- In fact, the seed inside level.dat will actually overwrite the seed from config and/or the randomly generated seed if the server can find it:
https://github.com/CloudburstMC/Server/blob/b000e760aebf294af426c616a8a53853a4708add/src/main/java/org/cloudburstmc/server/level/provider/leveldb/serializer/LevelDBDataSerializer.java#L145

@DaMatrix

Copy link
Copy Markdown
Member

alright, in that case i see no problems

@DaMatrix DaMatrix self-requested a review April 16, 2021 15:03
@DaMatrix DaMatrix requested a review from SupremeMortal April 16, 2021 15:04
Comment thread src/main/java/org/cloudburstmc/server/Server.java Outdated
Comment thread src/main/java/org/cloudburstmc/server/Server.java Outdated
@TheShermanTanker

Copy link
Copy Markdown
Collaborator Author

@SupremeMortal I felt like worlds was a better name than "levels", as it is slightly more descriptive, but I'll change it back if required

@TheShermanTanker

Copy link
Copy Markdown
Collaborator Author

Changed the names back, I do still prefer the ring "worlds" has as opposed to "levels" though, I changed them because the config class is named World and the variable is worldConfigs, and I was confused why they're called 2 different things

@TheShermanTanker

Copy link
Copy Markdown
Collaborator Author

Hi, just to check will the conflicts be automatically resolved on merge?

@TheShermanTanker

Copy link
Copy Markdown
Collaborator Author

Computer crashed and took the entire repo with it, starting new commit :/

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