Animal terrain finisher #1642

p-mcgowan · 2014-12-02T10:01:42Z

Adds animals to cChunkDesc entity list on chunk generation.

resolves #1595

Note: animals do not spawn, either the list does not get refreshed and sent to world, or the mobs spawn too far away and are removed shortly after.

madmaxoft · 2014-12-02T10:16:17Z

src/Generating/FinishGen.cpp

+				ASSERT(!"Unhandled world dimension");
+				break;
+			}
+	}  // switch (dimension)


Indenting is wrong inside the switch, there's a needless extra level.

madmaxoft · 2014-12-02T10:22:33Z

Your indents are rather wrong, you'll need to check all of them.

Don't compare set size to zero, use .empty() instead.

p-mcgowan · 2014-12-02T19:23:31Z

thanks for the feedback, should be all set. My formatter has been doing strange things.
I made the corrections suggested, and added the mycelium check for mooshroom spawning.

properly fixed in the next commit

p-mcgowan · 2014-12-02T20:26:25Z

I switched all the fast randoms to noise, but I'm wondering if the pack center and offsets should be fast random. If it's just a matter of making sure there are still cows at chunk [34, -2] then the center would not be important, especially considering they will wander. It may make a noticeable difference during generation if the chunk percentage is higher.
I can edit it in this PR or make a new one for animal chunk generation optimization if it sounds like a good idea.

Howaner · 2014-12-03T06:02:40Z

src/Generating/FinishGen.h

+	// cFinishGen override:
+	virtual void GenFinish(cChunkDesc & a_ChunkDesc) override;
+
+	// Tries to spawn a mob in the center of the pack. If successful, spawns 0-5 more.


Can you use doxy-comments?
/** Tries to ... */

madmaxoft · 2014-12-03T08:19:55Z

src/Generating/FinishGen.cpp

+		}
+	}  // switch (dimension)
+	m_AnimalProbability = a_IniFile.GetValueSetI(SectionName, "AnimalSpawnChunkPercentage", DefaultAnimalSpawnChunkPercentage);
+	if (m_AnimalProbability < 0 || m_AnimalProbability > 100)


Please do use parenthesis for each individual comparison. It makes the conditions more understandable.

madmaxoft · 2014-12-03T08:29:07Z

Sorry for making this so difficult

p-mcgowan · 2014-12-04T03:32:29Z

That's great feedback, thanks for taking the time to look through it.
I'm still learning the formatting but I can see that it is invaluable to have it consistent throughout the code - I will format shortly.

madmaxoft · 2014-12-04T08:47:08Z

I've just noticed your comment about fast random vs noise for pack centers.
I think consistency is more important than speed. If you randomized the pack center, it could happen that the pack won't be generated on a /regen, because the pack center would hit the terrain for that particular instance.

Don't worry about performance, the cNoise::IntNoise2DInt() and ``cNoise::IntNoise3DInt()` functions are really fast, all they do is hash the input coords and the seed together, it's just a few additions, multiplications and xors. They don't actually do any of the octaving or lerping that is normally associated with noise; these functions are only providing the values for the integral points.

p-mcgowan · 2014-12-04T09:11:13Z

Hopefully that helps some, I'm still a bit fuzzy on one specific - should I 'doxy' 1-line comments on top of new methods, or just the new classes?

madmaxoft · 2014-12-04T09:59:57Z

src/Generating/FinishGen.h

@@ -18,6 +18,8 @@
 #include "ComposableGenerator.h"
 #include "../Noise/Noise.h"
 #include "../ProbabDistrib.h"
+#include "../Mobs/Monster.h"
+#include "FastRandom.h"


FastRandom should not be needed anymore, is it?

madmaxoft · 2014-12-04T22:18:56Z

src/Generating/FinishGen.cpp

+	m_AnimalProbability = a_IniFile.GetValueSetI(SectionName, "AnimalSpawnChunkPercentage", DefaultAnimalSpawnChunkPercentage);
+	if ((m_AnimalProbability < 0) || (m_AnimalProbability > 100))
+	{
+		LOGWARNING("[Animals]: AnimalSpawnChunkPercentage is invalid, using the default of \"%d\".", DefaultAnimalSpawnChunkPercentage);


You say you're using the default, but I see no code doing that.

p-mcgowan · 2014-12-05T00:59:38Z

I'm not sure what you mean by the comment "...using the default,... no code doing that.". Frankly, I just copied that from the function above but, as I understand it, the assertion will fail if none of the 3 vanilla dimensions are applicable. Should I add a line DefaultChance = 0 to cover that case?

For the switch, in the case of oceans and mushroom islands, the only thing in the list should be squid and mooshroom, respectively, so the break there skips the default mobs. For the ocelot, horse, and wolf, they are added to the list in addition to the default animals, chick cow etc.. Having the default animals outside the switch would add them to the list even if in an ocean or mushIsle biome, which would just incur more cases in the TrySpawnAnimals section.
If that doesn't work, I can refactor, I just thought that was the cleanest approach.

p-mcgowan · 2014-12-05T01:24:38Z

re-read wiki, I thought animals spawned on grass in deserts on generation, but seems like only in random chunk spawning. Incorporated that into the latest build

madmaxoft · 2014-12-05T06:58:43Z

The warning message says that you're using the default value. But there's no code that actually sets m_AnimalProbability to DefaultAnimalSpawnChunkPercentage. That's all I meant.

madmaxoft · 2014-12-05T07:03:42Z

As for the switch, unfortunately it doesn't work that way. When there's a break missing, it falls through to the next case, and the next, and the next...
So your code will, for an ocean, add the squid, then an ocelot and a horse and then all the defaults.

I suggest you put the default mobs into a separate function so that you can call it from the individual cases that do use the defaults, and then add calls to that function and breaks to the switch.

p-mcgowan · 2014-12-05T09:03:01Z

Of course, if only cases worked as I wish they did...

Thought about making a new function, but I did what you originally said and took the defaults outside the case. The only 2 cases where they do not get added are desert/end/nether and mushroom islands, which return out, so I figured it was easier that way.

madmaxoft · 2014-12-05T16:04:35Z

Please merge current master into this branch, run the CheckBasicStyle.lua script to find out the bad formatting, and fix that.

Also, there's still the issue of default probability ( #1642 (comment) )

…rainFinisher update

p-mcgowan · 2014-12-05T19:33:51Z

Haha, you must have got tired of telling me to format those conditionals... I see what you mean now about the default m_prob, should be all skippy now. Thanks again for being patient!

madmaxoft · 2014-12-05T21:11:40Z

Honestly, I just came up with a way to code the check in Lua's limited regexps, you may have been a catalyst, but certainly not the only target.

Animal terrain finisher

madmaxoft · 2014-12-05T21:47:26Z

I've made some fixes after merging. It seems to work for me, animals are spawned properly. :)

p-mcgowan · 2014-12-05T22:12:24Z

That's awesome. Looks like:
cEntityList ChunkEntities = a_ChunkDesc.GetEntities();
ChunkEntities.push_back(NewMob)
was the problem. It seems that I was just copying the list, adding a mob to it, then destroying the copy.
a_ChunkDesc.GetEntities().push_back(NewMob) solves that.

p-mcgowan added 3 commits December 2, 2014 01:37

animal terrain finisher

9831220

animal terrain finisher

2d93274

fix comments after replacing spaces with tabs

978c996

madmaxoft reviewed Dec 2, 2014
View reviewed changes

indentation, repeatable random, small corrections

585662e

p-mcgowan added 2 commits December 2, 2014 11:26

fastRandom unused

8d7c2d4

typos and oversights for lua and CI

6803df1

p-mcgowan added 2 commits December 2, 2014 15:55

forgot initializer

2ea8a36

forgot generation defaul init

7f8f2f1

Howaner reviewed Dec 3, 2014
View reviewed changes

p-mcgowan added 2 commits December 2, 2014 22:35

doxy-commenting

d348433

class description and doxy-commenting

8c3b9ae

madmaxoft reviewed Dec 3, 2014
View reviewed changes

awful comment fixing, randomizer fix

83d4bec

madmaxoft reviewed Dec 4, 2014
View reviewed changes

format blocks and randomizing

99a5b38

madmaxoft reviewed Dec 4, 2014
View reviewed changes

formatting fixes

53a3359

p-mcgowan added 3 commits December 4, 2014 16:59

handle non-vanilla dimensions

538991c

handle non-vanilla dimensions

78f0aeb

restructured random animals, added check for desert (update as per wiki)

bd8c185

p-mcgowan added 2 commits December 5, 2014 00:57

restructure, with logic this time

c655d97

reformat

750b4a3

p-mcgowan added 2 commits December 5, 2014 11:24

Merge branch 'master' of github.com:p-mcgowan/MCServer into animalTer…

a8bbd5e

…rainFinisher update

formatting and default spawn percentage

b25fcb0

madmaxoft added a commit that referenced this pull request Dec 5, 2014

Merge pull request #1642 from p-mcgowan/animalTerrainFinisher

5712fad

Animal terrain finisher

madmaxoft merged commit 5712fad into cuberite:master Dec 5, 2014

p-mcgowan deleted the animalTerrainFinisher branch December 5, 2014 21:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Animal terrain finisher #1642

Animal terrain finisher #1642

p-mcgowan commented Dec 2, 2014

madmaxoft Dec 2, 2014

madmaxoft commented Dec 2, 2014

p-mcgowan commented Dec 2, 2014

p-mcgowan commented Dec 2, 2014

Howaner Dec 3, 2014

madmaxoft Dec 3, 2014

madmaxoft commented Dec 3, 2014

p-mcgowan commented Dec 4, 2014

madmaxoft commented Dec 4, 2014

p-mcgowan commented Dec 4, 2014

madmaxoft Dec 4, 2014

madmaxoft Dec 4, 2014

p-mcgowan commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014

Animal terrain finisher #1642

Animal terrain finisher #1642

Conversation

p-mcgowan commented Dec 2, 2014

madmaxoft Dec 2, 2014

Choose a reason for hiding this comment

madmaxoft commented Dec 2, 2014

p-mcgowan commented Dec 2, 2014

p-mcgowan commented Dec 2, 2014

Howaner Dec 3, 2014

Choose a reason for hiding this comment

madmaxoft Dec 3, 2014

Choose a reason for hiding this comment

madmaxoft commented Dec 3, 2014

p-mcgowan commented Dec 4, 2014

madmaxoft commented Dec 4, 2014

p-mcgowan commented Dec 4, 2014

madmaxoft Dec 4, 2014

Choose a reason for hiding this comment

madmaxoft Dec 4, 2014

Choose a reason for hiding this comment

p-mcgowan commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

madmaxoft commented Dec 5, 2014

p-mcgowan commented Dec 5, 2014