Further abstraction of the dependency system for non-cube dependents and bugfixing of the dependency system #18

Cyclonit · 2016-06-06T20:06:17Z

The 2d visualization code you posted does not show any unloading of cubes. However, I did make the ServerCubeCache log whenever it unloaded a cube (and loaded) and it did appear to act properly. I couldn't find any cubes not being unloaded after moving 1000 blocks to the north. One issue remaining is the world not showing up on the first generation. Saving the game and reloading solves the problem.

…being set correctly.

Barteks2x · 2016-06-06T20:24:04Z

src/main/java/cubicchunks/server/ServerCubeCache.java

@@ -24,6 +24,8 @@
 package cubicchunks.server;

 import com.google.common.collect.Maps;
+
+import cubicchunks.CubeProviderDebug;


Make sure to remove this before commit, it breaks the build

Barteks2x · 2016-06-06T20:45:05Z

Which world type did you test it with? I think I've seen some columns being left loaded as I'm moving, see #11.

Also, this issue needs fixing: #14

Cyclonit · 2016-06-06T20:46:25Z

I tested using Custom Cubic.

Barteks2x · 2016-06-06T20:50:20Z

Just squash all of these small commits... this is getting a bit too messy. And undo formatting changes in CubeIO, at some point my IDE will undo your changes, and it will cause just a lot of pointless changes. (this may be useful to read if you don't know what it means)

Cyclonit · 2016-06-06T20:53:23Z

I'll clean it up asap. I managed to confuse origin/upstream and master/dependency, leading to a jambled mess... Sorry

Barteks2x · 2016-06-06T21:03:34Z

src/main/java/cubicchunks/server/CubeIO.java

@@ -95,7 +95,7 @@ private static DB initializeDBConnection(final File saveFile, final WorldProvide

 		file.getParentFile().mkdirs();

-		DB db = DBMaker.fileDB(file).transactionEnable().make();
+		DB db = DBMaker.fileDB(file).make();


transactions should be enabled

- Trying to fix git synchronization issues... sorry - Trying to fix git synchronization issues... sorry - Fixed cubes not appearing on first world load (by reverting change to ServerCubeCache) - Removed unnecessary imports.

…nto dependency

Barteks2x · 2016-06-06T21:44:53Z

I would also really suggest switching to IntelliJ IDEA, at least for this project. This way you will avois almost all formatting changes.

- Extracted interface Dependent and renamed former class to DependentCube in preparation for allowing non-cube dependents.

… DependentCube in preparation for permitting generic dependent entities. - Reverted changes to RegionDependency to use a singleton instead. - Removed explicit type declarations.

Cyclonit · 2016-06-07T17:19:25Z

I have changed to IDEA, but I cannot guarantee an immediate improvement with regards to formatting as it is driving me nuts D:

Barteks2x · 2016-06-07T18:14:28Z

Just import my formatter settings. This way you will have exactly the same settings = no difference.

Barteks2x · 2016-06-07T18:19:15Z

src/main/java/cubicchunks/worldgen/dependency/RegionDependency.java

 	}

-	public RegionDependency(Cube cube, GeneratorStage stage, int xLow, int xHigh, int yLow, int yHigh, int zLow, int zHigh) {
+	public RegionDependency(GeneratorStage targetStage, int xLow, int xHigh, int yLow, int yHigh, int zLow, int zHigh) {


I would suggest using Vec3is instead of ints here, argument order isn't obvious from code.

Do you have any naming in mind? "lowerFrontLeft" and "upperBackRight" sound kinda wrong...

startPosition and endPosition? You can sort them in constructor.

…stead of individual integers. - Added todos to several classes.

Barteks2x · 2016-06-14T07:45:40Z

src/main/java/cubicchunks/lighting/FirstLightProcessor.java

+					CubeCoords coords = new CubeCoords(cube.getX() + x, cube.getY() + y, cube.getZ() + z);
+					if (!cache.cubeExists(coords) || cache.getCube(coords).getCurrentStage().precedes(generatorStage)) {
+						CubicChunks.LOGGER.error("Missing cube ({})! {}/{}", coords, cache.getCube(coords).getCurrentStage().getName(), generatorStage.getName());
+						System.exit(1);


Never use System.exit(), or Forge will show a big warning when loading it that it has direct reference to it, and that it will redirect it. Use FMLCommonHandler.exitJava(). Or throw new Error().

You can simply create new FastCubeBlockAccess(cubeCache, centerCube, radius) and modify it to also check stage (and it should check the stage, I simply forgot about that) - it will throw exception if required cube isn't loaded and it will dump information about which cubes are loaded (you can also modify that to include stage if it doesn't do that now). And maybe you could pass that FastCubeBlockAccess to some of these methods and use it there.

Or leave it as it's now and let me fix that later.

I want to do as few things as possible in FirstLightProcessor because it needs to be fast. Currently it's close to vanilla initial lighting preformance. And most of the time is spent creating FastCubeBlockAccess objects.

I just quickly commited my current code so you can have a go at it if you want to. The check in FirstLightProcessor can go as it never encounters a missing block. If the DependencySystem were failing at delivering the required cubes properly, that loop would exit the program and it does not.

Ok, just keep in mind that before merging it, System.exit() must be removed. Or possibly remove this check entirely. There is a check if world.checkLightFor() returns true (if it returns false it means that world.isAreaLoaded(thePosition, 17, false) returned false).

Of course, I'll clean it up once I found this f*** bug...

Barteks2x · 2016-06-14T09:09:53Z

I'm not sure what the issue is now. I'm flying around for several minutes and din't experience any bug (I didn't test cube unloading yet). Lighting works as I expect it to work and it shows no rogue cubes.

Update: well, I did find some bug, but nothing is wrong in the log.

Cyclonit · 2016-06-14T09:19:36Z

Try logging the DependentCubeManager.getDependentCubeCount and DependencyManager.getRequiredCubeCount. Those should approach zero, but sometimes they don't.

Barteks2x · 2016-06-14T09:23:14Z

As for these cubes that are not sent to player: they seem to have wrong target stage set. After trying to see why PlayerCubeMap doesn't send it to me it turned out that target stage is set to lighting. And I was in that cube.

Barteks2x · 2016-06-14T10:53:51Z

Also, as of some recent changes FeatureProcessor has no dependencies as I merged surface and terrain stages. I could technically even merge terrain and features (as in vanilla). And (at least in my case) debugging showed that it's this stage that has some issues (still, it would be better to fix it properly if it works with Features as independent stage)

Cyclonit · 2016-06-14T11:55:36Z

I'm somewhat confident now that everything works as it was supposed to do last week...

Thanks for pointing out the wrong targetStage. Apparently some reorganization of loadCube caused it not being set in all cases when loadCube was invoked with a targetStage as parameter.

Barteks2x · 2016-06-14T13:13:51Z

src/main/java/cubicchunks/worldgen/GeneratorPipeline.java

@@ -122,7 +122,7 @@ public void generate(Cube cube) {
 	public void generate(Cube cube, GeneratorStage targetStage) {

 		// Make sure the proper target stage is set.
-		if (cube.getTargetStage().precedes(targetStage)) {
+		if (cube.getTargetStage() == null || cube.getTargetStage().precedes(targetStage)) {


Can target stage be null?

Currently yes, because it is no longer set in all paths of ServerCubeCache.loadCube, but I'll change that to get rid of that additional check. Cleaner without it.

- Fixed Cube.targetStage not being set properly in ServerCubeCache.loadCube. - Fixed casing in comments. - Added missing comments. - Removed debug code from DependencyManager. - Removed debug code from DependentCubeManager. - Added missing annotations in RegionDependency. - Changed ServerCubeCache.loadCube such that Cube.targetStage is defined in all paths and removed a null check from GeneratorPipeline.

Barteks2x · 2016-06-15T01:59:06Z

Chunk unloading still needs to be fixed, this PR breaks it:

Cyclonit · 2016-06-15T08:44:46Z

The dependency system makes it difficult for cubes to be unloaded during generation. Usually they end up being finished and unloaded afterwards, meaning it may take some time for them to disappear.

Barteks2x · 2016-06-15T08:51:08Z

The way it works is a bit weird. It works fine when moving vertically, but when moving in x/z - cubes are not getting unloaded as in screenshot above. Except that they eventually do end up unloaded because vanilla unloads all cubes that are not loaded by any players every time it saves all chunks. But this is definitely not something that you should rely on. Maybe unload a cube when there is nothing left that depends on it?

Barteks2x · 2016-06-15T08:52:18Z

Another approach would be to keep them in unload queue and just skip them from unloading if there is something that depends on them. This way they will be automatically unloaded as soon as possible (potentially slow for high view distance/a lot of players)

Cyclonit · 2016-06-15T09:00:36Z

That's what is happening right now. In unloadCube and unloadQueuedCubes the ServerCubeCache skips cubes that are currently being required for other cubes. Cubes which are not required can be unloaded (and all cubes blocked by them will be unloaded thereafter), but since the lighting stage requires the other cubes to be in the exact same stage, this doesn't change much.

So to break it down: Dependents must be unloaded first, but since lots of dependents are required themselves, cubes end up not being unloaded early on.

Barteks2x · 2016-06-15T09:08:06Z

It's not skipping them. It's removing them from unload queue and skipping.

Cyclonit · 2016-06-15T09:15:21Z

Does PlayerCubeMap not put them in the queeue again regardless?

Barteks2x · 2016-06-15T09:18:03Z

Nope. It unloads once. And then it assumes that it's unloaded.

Barteks2x · 2016-06-15T09:19:53Z

Also, why don't you just read the code?

Cyclonit · 2016-06-15T09:23:14Z

I apologize, I'm on the phone right now and commented faster than I was thinking.

…oUnload. - Changed ServerCubeCache to not remove skipped cubes in unloadQueuedChunks.

Cyclonit · 2016-06-15T17:30:52Z

The ServerCubeCache no longer removes required cubes from the queue. But to be honest, I don't see a significant change.

Barteks2x · 2016-06-15T21:38:59Z

src/main/java/cubicchunks/server/ServerCubeCache.java

-			long cubeAddress = this.cubesToUnload.poll();
+		Iterator<CubeCoords> iter = this.cubesToUnload.iterator();
+		while (iter.hasNext()) {
+			CubeCoords coords = iter.next();


you never limit the amount of unloaded cubes per tick. unloadQueuedChunks should be really called tick.

You're right. But I assume limiting it would be a good idea. If you agree I'll add a simple counter and exit once the maximum number is reached. But I'll have to postpone doing so to tomorrow.

This is how it was done before, and this is how vanilla does it.

- Fixed CustomCubicChunksWorldType to use IndependentGeneratorStage for feature stage. - Fixed ServerCubeCache.unloadQueuedChunks to not process all queued cubes at once.

Barteks2x · 2016-06-17T05:01:57Z

I know that unloadding still dowan't work correctly, but fixing that rewuored rewriting some parts of ServerCubeCache (changes from the branch I told you not to merge).

- Fixed infinite chains of cubes being loaded due to targetStage not …

0af4b86

…being set correctly.

Barteks2x reviewed Jun 6, 2016
View reviewed changes

Cyclonit force-pushed the dependency branch 2 times, most recently from 5fb2a97 to f03fcf1 Compare June 6, 2016 21:21

Cyclonit added 4 commits June 6, 2016 23:21

- Reverted formatting shenanigans

f03fcf1

- Trying to fix git synchronization issues... sorry - Trying to fix git synchronization issues... sorry - Fixed cubes not appearing on first world load (by reverting change to ServerCubeCache) - Removed unnecessary imports.

- Reverted formatting shenanigans

b59e9d3

- Trying to fix git synchronization issues... sorry - Trying to fix git synchronization issues... sorry - Fixed cubes not appearing on first world load (by reverting change to ServerCubeCache) - Removed unnecessary imports.

Merge branch 'dependency' of http://github.com/Cyclonit/CubicChunks i…

696e6ce

…nto dependency

- Revert of CubeIO

fe8a95e

Cyclonit added 3 commits June 7, 2016 18:29

- Applied formatting using IDEA settings.

024f23d

- Extracted interface Dependent and renamed former class to DependentCube in preparation for allowing non-cube dependents.

- Extracted interface Dependent and renamed former class Dependent to…

a70e3a2

… DependentCube in preparation for permitting generic dependent entities. - Reverted changes to RegionDependency to use a singleton instead. - Removed explicit type declarations.

- Extracted interface Dependent and renamed former class Dependent to…

567fb02

… DependentCube in preparation for permitting generic dependent entities. - Reverted changes to RegionDependency to use a singleton instead. - Removed explicit type declarations.

Cyclonit force-pushed the dependency branch from a70e3a2 to 567fb02 Compare June 7, 2016 17:17

Barteks2x reviewed Jun 7, 2016
View reviewed changes

- Changed RegionDependency to use Vec3i in one of its constructors in…

730bea8

…stead of individual integers. - Added todos to several classes.

Barteks2x reviewed Jun 14, 2016
View reviewed changes

Cyclonit force-pushed the dependency branch from b41f95b to b3e1d38 Compare June 14, 2016 11:58

Barteks2x reviewed Jun 14, 2016
View reviewed changes

Cyclonit force-pushed the dependency branch from b3e1d38 to 871adbe Compare June 14, 2016 15:22

- Fixed cubes not being generated up to the latest required stage.

87709ce

- Changed ServerCubeCache to use CubeCoords instead of Long in cubesT…

5286946

…oUnload. - Changed ServerCubeCache to not remove skipped cubes in unloadQueuedChunks.

Merge remote-tracking branch 'remotes/upstream/master' into dependency

9d4727a

Barteks2x reviewed Jun 15, 2016
View reviewed changes

Merge remote-tracking branch 'remotes/upstream/master' into dependency

777d3dc

- Fixed CustomCubicChunksWorldType to use IndependentGeneratorStage for feature stage. - Fixed ServerCubeCache.unloadQueuedChunks to not process all queued cubes at once.

Barteks2x merged commit dd14d25 into OpenCubicChunks:master Jun 17, 2016

Cyclonit deleted the dependency branch June 17, 2016 05:52

Further abstraction of the dependency system for non-cube dependents and bugfixing of the dependency system #18

Further abstraction of the dependency system for non-cube dependents and bugfixing of the dependency system #18

Conversation

Cyclonit commented Jun 6, 2016

Choose a reason for hiding this comment

Barteks2x commented Jun 6, 2016

Cyclonit commented Jun 6, 2016

Barteks2x commented Jun 6, 2016 • edited Loading

Cyclonit commented Jun 6, 2016

Choose a reason for hiding this comment

Barteks2x commented Jun 6, 2016

Cyclonit commented Jun 7, 2016

Barteks2x commented Jun 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x Jun 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x Jun 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x commented Jun 14, 2016 • edited Loading

Cyclonit commented Jun 14, 2016

Barteks2x commented Jun 14, 2016

Barteks2x commented Jun 14, 2016 • edited Loading

Cyclonit commented Jun 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x commented Jun 15, 2016

Cyclonit commented Jun 15, 2016

Barteks2x commented Jun 15, 2016

Barteks2x commented Jun 15, 2016 • edited Loading

Cyclonit commented Jun 15, 2016 • edited Loading

Barteks2x commented Jun 15, 2016

Cyclonit commented Jun 15, 2016

Barteks2x commented Jun 15, 2016

Barteks2x commented Jun 15, 2016

Cyclonit commented Jun 15, 2016

Cyclonit commented Jun 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x commented Jun 17, 2016

Barteks2x commented Jun 6, 2016 •

edited

Loading

Barteks2x commented Jun 7, 2016 •

edited

Loading

Barteks2x Jun 14, 2016 •

edited

Loading

Barteks2x Jun 14, 2016 •

edited

Loading

Barteks2x commented Jun 14, 2016 •

edited

Loading

Barteks2x commented Jun 14, 2016 •

edited

Loading

Cyclonit commented Jun 14, 2016 •

edited

Loading

Barteks2x commented Jun 15, 2016 •

edited

Loading

Cyclonit commented Jun 15, 2016 •

edited

Loading