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

Implement isSlimeChunk #894

Merged
merged 4 commits into from Apr 6, 2018

Conversation

Projects
None yet
5 participants
@fionera
Contributor

fionera commented Mar 31, 2018

isSlimeChunk was alwas false. This implements the algorithm for it.

import java.util.List;
import java.util.Set;
import java.util.*;

This comment has been minimized.

@kashike

kashike Mar 31, 2018

Simply adding an import would work, and would be better.

This comment has been minimized.

@fionera

fionera Mar 31, 2018

Contributor

Will fix it. IntelliJs automatic import optimize was on :/

+ (long) (this.x * 0x5ac0db)
+ (long) (this.z * this.z) * 0x4307a7L
+ (long) (this.z * 0x5f24f) ^ 0x3ad8025f).nextInt(10) == 0;

This comment has been minimized.

@Pr0methean

Pr0methean Mar 31, 2018

Contributor

Could you please inline java.util.Random's formula? This method may end up being called more frequently than we can afford to create objects!

This comment has been minimized.

@fionera

fionera Mar 31, 2018

Contributor

Thats why i added the cache or what do you mean?

This comment has been minimized.

@Pr0methean

Pr0methean Mar 31, 2018

Contributor

What I mean is to just calculate whether a new Random created with the given seed would output 0 on the first call to nextInt(10), rather than actually calling the constructor of Random. That way, you're not creating an object that has to be garbage-collected later, even if the JVM's runtime compiler fails to optimize that creation and garbage collection out. (It should optimize them out, but compiler optimizations don't always do what they should.)

This comment has been minimized.

@Pr0methean

Pr0methean Mar 31, 2018

Contributor

On second thought, if you can't do this, it's okay; we'll optimize it later.

This comment has been minimized.

@fionera

fionera Mar 31, 2018

Contributor

Maybe give the Object in Constructor? That way you only clone it and set the seed per chunk.

This comment has been minimized.

@Pr0methean

Pr0methean Mar 31, 2018

Contributor

My concern was about the allocation of memory and the process of garbage collection, both of which would happen just the same whether the object's constructor ran or not.

This comment has been minimized.

@fionera

fionera Mar 31, 2018

Contributor

Ah ok. Now its saved in the Regionfiles

@@ -170,7 +177,17 @@ public GlowBlock getBlock(int x, int y, int z) {
@Override
public boolean isSlimeChunk() {
return false; // TODO: implement slime chunks
if (isSlimeChunk == -1) {
boolean isSlimeChunk = new Random(this.world.getSeed()

This comment has been minimized.

@momothereal

momothereal Mar 31, 2018

Member

Can you add a link or source for this formula (e.g. Taken from the Minecraft Wiki)

fionera added some commits Mar 31, 2018

@mastercoms mastercoms merged commit 14d07b7 into GlowstoneMC:dev Apr 6, 2018

2 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@fionera fionera deleted the fionera:implement-isSlimeChunk branch Apr 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment