Refactor tree and flora generation for improved extensibility #1562

msteiger · 2015-01-27T17:45:37Z

This PR fixes a few things such as cut-off trees and different generation probabilites for biomes.

Its main focus is the refactoring of tree and flora generation so that modules can extend it. The new SurfaceObjectProvider is the base class of DefaultFloraProvider and DefaultTreeProvider.

Modules can write similar classes to map growth probabilites to biome (or other variables) and add filters such as flatness or UI-based config.

Tree generators are moved to a static class to enable re-use.
Plants have been renamed to "Flora" to be consistent with content and split into 3 groups: Grass, Flowers and Mushrooms. This allows for putting more mushrooms in the forest and more grass on plains.

Also, a unit test (requires #1536 and #1542) estimates maximum tree extents to correctly adjust facet borders. Commit 38b85e9 removes the workaround and requires the verticalChunkMeshSegments variable to be removed from ChunkImpl.

The filter depends on the order of chunk generation. It cannot filter trees in a deterministic manner.

Josharias · 2015-01-27T18:33:43Z

It is great to see this get an overhaul with more strategy. Looking forward to giving this a closer look!

skaldarnar · 2015-01-27T20:46:32Z

modules/Core/src/test/java/org/terasology/world/generator/TreeTests.java

+        Vector3i pos = new Vector3i(ChunkConstants.SIZE_X / 2, 0, ChunkConstants.SIZE_Z / 2);
+
+        Vector3i min = new Vector3i(pos);
+        Vector3i max = new Vector3i(pos);


Should these two be final since they are used in an inner class?

Yup this gets me a compile error :-)

Works fine when final is added.

skaldarnar · 2015-01-27T21:03:14Z

I've commented on some lines for on-spot review. Besides these minor points all looks good, trees are generated as expected, and I couldn't find any cut-off trees.

I suggest to change the tree probabilities to have birches in plains and pines in snowy biomes. Furthermore, I'd like to see birches be added to the forest biome as well.

Cervator · 2015-01-27T21:31:43Z

Not home yet to check it out but am curious if this also fixes tree gen in TTA? Guess I'll know soon :-)

Josharias · 2015-01-28T05:07:10Z

modules/Core/src/main/java/org/terasology/core/world/generator/rasterizers/FloraRasterizer.java

+        FloraFacet facet = chunkRegion.getFacet(FloraFacet.class);
+        Block air = BlockManager.getAir();
+
+        Random rng = new FastRandom(chunk.getPosition().hashCode());


This will cause the flora to be not deterministic anymore if the size of the facet changes. I don't think it really matters in this case. But just saying it out loud.

Josharias · 2015-01-28T05:13:16Z

Nice abstraction over these classes, msteiger. All generated well for me.

Cervator · 2015-01-28T06:11:28Z

Tested out here as well, looks good (after the quick final fix) :-)

Doesn't fix trees in TTA so I take it something extra has to be applied to world gen there

#1536 is listed as a requirement in one of the commits, and @emanuele3d has partially addressed it, but is more stuff needed?

engine-tests ran 100% success, but the 5 tests in TreeTests failed in both IntelliJ and via plain Gradle :-(

01:08:15,716 |-INFO in ch.qos.logback.classic.gaffer.ConfigurationDelegate@384f27a1 - Added status listener of type [ch.qos.logback.core.status.OnConsoleStatusListener]
01:08:15,755 |-INFO in ch.qos.logback.classic.gaffer.ConfigurationDelegate@384f27a1 - About to instantiate appender of type [ch.qos.logback.core.ConsoleAppender]
01:08:15,757 |-INFO in ch.qos.logback.classic.gaffer.ConfigurationDelegate@384f27a1 - Naming appender as [CONSOLE]
01:08:15,947 |-INFO in ch.qos.logback.classic.gaffer.ConfigurationDelegate@384f27a1 - Setting level of logger [ROOT] to DEBUG
01:08:15,958 |-INFO in ch.qos.logback.classic.gaffer.ConfigurationDelegate@384f27a1 - Attaching appender named [CONSOLE] to Logger[ROOT]
01:08:15,960 |-INFO in ch.qos.logback.classic.gaffer.ConfigurationDelegate@384f27a1 - Setting level of logger [org.terasology] to INFO

java.lang.NullPointerException
    at org.terasology.world.chunks.internal.ChunkImpl.<init>(ChunkImpl.java:68)
    at org.terasology.world.chunks.internal.ChunkImpl.<init>(ChunkImpl.java:108)
    at org.terasology.world.chunks.internal.ChunkImpl.<init>(ChunkImpl.java:104)
    at org.terasology.world.generator.TreeTests$1.<init>(TreeTests.java:106)
    at org.terasology.world.generator.TreeTests.computeAABB(TreeTests.java:106)
    at org.terasology.world.generator.TreeTests.estimateExtent(TreeTests.java:91)
    at org.terasology.world.generator.TreeTests.testBirchDims(TreeTests.java:64)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:74)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:211)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:67)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:134)

msteiger · 2015-01-28T20:02:11Z

Thanks @ALL for the reviews. The following changes were made:

Renamed birk to birch
Add trees types on plains/forest/snow biomes
Fixed Java7 issues (final variables)
Deterministic flora generation
Added workaround for tests again, so that the vertical-chunk fix is not needed. As soon as that is in, we can remove the workaround.

Yes, @Cervator, TtA has its own tree generators (in GrowingFlora I believe). They are not affected.

Josharias · 2015-01-28T20:41:48Z

modules/Core/src/main/java/org/terasology/core/world/generator/rasterizers/TreeRasterizer.java

+            Vector3i pos = entry.getKey();
+            TreeGenerator treeGen = entry.getValue();
+            int seed = relativeToWorld(facet, pos).hashCode();
+            Random random = new FastRandom(seed);


Does this need similar love to how you handled the flora generation?

It looks similar, but the seed value is generated per world location and independent from the iteration order and chunk/facet. At least unless I'm overlooking something here ..

Cervator · 2015-01-30T03:55:35Z

Retested and all looked well :-)

@MarcinSc - would this be helpful toward fixing up tree gen in TTA? Or if you're low on time would it help if somebody else gave it a shot? I think you said you were working on trees some time back, am unsure if you have any pending code?

msteiger added 19 commits January 27, 2015 17:55

Moved sealevel check from rasterizer to provider

2703f3b

Reworked TreeProvider

680e529

Extract AbtractTreeProvider

efafdb0

Moved Tree definitions into a new class

04a5caa

Moved tree gen prob. to rasterizer

b8f0c1b

Moved tree type def. to provider

62c02e2

Refactor tree provider filters and adjust probabilities

653ecca

Moved tree gen code into trees package and defer block resolving

efd6e5b

Remove MinDistanceFilter and minor tree gen tweaks

5c9ea94

The filter depends on the order of chunk generation. It cannot filter trees in a deterministic manner.

Refactor tree placement filters into PositionFilters

f6acd73

Added different plant types

8e8e55e

Use floor + 1 instead of ceil for flora pos computation

9b83e25

Add per-biome probs. for flora

afa8f74

Use noise implementation instead of noise table

5f5f44f

Merge abstract tree and flora into SurfaceObjectProvider

b8377f2

Minor fixes in tree generation

365017a

Ensure identical seed for tree rasterization

9b3303a

Added test for generated tree AABBs

356c2c9

More surface object position filters

b31222d

GooeyHub added the in progress label Jan 27, 2015

msteiger added the Category: Gameplay Content Requests, Issues and Changes targeting gameplay mechanics and content label Jan 27, 2015

msteiger added this to the Alpha! milestone Jan 27, 2015

skaldarnar reviewed Jan 27, 2015
View reviewed changes

Josharias reviewed Jan 28, 2015
View reviewed changes

msteiger force-pushed the trees branch from 38b85e9 to b31222d Compare January 28, 2015 19:25

Incorporated review comments on tree gen.

267f479

Josharias reviewed Jan 28, 2015
View reviewed changes

Cervator merged commit 267f479 into MovingBlocks:develop Jan 30, 2015

Cervator added a commit that referenced this pull request Jan 30, 2015

Merge PR #1562 by @msteiger - tree/flora improvements

275efff

GooeyHub removed the in progress label Jan 30, 2015

msteiger deleted the trees branch April 25, 2015 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor tree and flora generation for improved extensibility #1562

Refactor tree and flora generation for improved extensibility #1562

msteiger commented Jan 27, 2015

Josharias commented Jan 27, 2015

skaldarnar Jan 27, 2015

Cervator Jan 28, 2015

skaldarnar commented Jan 27, 2015

Cervator commented Jan 27, 2015

Josharias Jan 28, 2015

Josharias commented Jan 28, 2015

Cervator commented Jan 28, 2015

msteiger commented Jan 28, 2015

Josharias Jan 28, 2015

msteiger Jan 29, 2015

Cervator commented Jan 30, 2015

Refactor tree and flora generation for improved extensibility #1562

Refactor tree and flora generation for improved extensibility #1562

Conversation

msteiger commented Jan 27, 2015

Josharias commented Jan 27, 2015

skaldarnar Jan 27, 2015

Choose a reason for hiding this comment

Cervator Jan 28, 2015

Choose a reason for hiding this comment

skaldarnar commented Jan 27, 2015

Cervator commented Jan 27, 2015

Josharias Jan 28, 2015

Choose a reason for hiding this comment

Josharias commented Jan 28, 2015

Cervator commented Jan 28, 2015

msteiger commented Jan 28, 2015

Josharias Jan 28, 2015

Choose a reason for hiding this comment

msteiger Jan 29, 2015

Choose a reason for hiding this comment

Cervator commented Jan 30, 2015