Just thought we should create an issue for this, and keep the discussion centralised.

The point is I think we should consider rethinking the biome code. Specifically moving a lot of code out of the individual biome classes, a lot like the new terrain wrapping. If it was possible to get a list of decorations for a biome, it would also make quite a few features possible, both for us and 1st or 3rd party addons.

The main issue with this is we could be removing some level of customisability for the individual biomes by making things more preset based.
However I think, with a few variables this is easily resolved.
Currently both savanna plateaus and all mesa plateau variations use the exact same terrain code with only a few variables changed. And mesa Bryce looks nothing like savanna plateau M.

I suggested something like this a while back for terrains and Pink, who's the big proponent of individual classes, came up with the mass wrapping as a compromise. It would make complex configs easier to have a more standardized system. Rewriting the decorators would be more complicated than rewriting the terrains, though - they can get pretty complicated.

The main reason I wanted to keep things in their own classes is because that was the only way (at the time) that I was certain we could keep all of the biomes as their own little islands and not constrained aesthetically by a standardized system.

However, thanks to you wonderful Java wizards, I've learned a thing or two since then and I'm a lot more open to such a system, as long as we retain the ability to 'break free' if necessary, which I'm sure we'd be able to do even within a standardized system.

However, as Zeno mentioned, I foresee a lot of trouble in our future if we try to incorporate the decorations into that system. But again, as long as the decoration system accounts for all of the complexity, then I wouldn't be against something more standardized.

Would it be worth doing a 'surface wrapping' project like we did with the terrain? I'm just thinking if we start off with static wrappers, it might make it easier to spot the anomalies along the way, and also make it easier to setup a proof-of-concept for the new biome system? If so, I'll gladly volunteer for that one.

If you start with that, id love to look into the specifics of the final system and maybe make some propositions (if that means what I think it does)

Terrain wrapping makes it enormously easier to work on the terrain. Not only are there a LOT less files to change it's a lot easier to figure out what's going on. That's a big argument in my book for some kind of standardization of decoration, if it's possible (which it may not be.)

I assume by "surface wrapping" you mean the decorators, not the "surface" routines? It's worth a look, just to get a handle on what's going on in the decoration system. It's possible that rather than wrapping entire routines, you might just end up wrapping "bits" to place a certain tree in a certain height range or to place a given log with a given tree.

I actually meant the "surface" routines as we're going to need to incorporate those into the standardized biome system as well.

Might be better to start with the decoration system though.

Refactoring the decoration system is more useful as we'll definitely be changing that. If you do, figure out a way to have height restrictions in the generators or meta-generators because we'll definitely need that (I'm trying to figure out how to go at the Highlands decorators to change just that). Surfacing is not so much of an issue, except for how we want to handle lake-adjacent blocks.

So I added a basic proof-of-concept for the new decoration system in f04c832

The basic idea is that we create realistic decoration objects (DecoBoulder in the example) with public variables that can be configured in the RealisticBiome class.

If you look at the modified Jungle biome, you can see how this would be implemented. Not all options would need to be explicitly set - if you just wanted the 'standard' boulder treatment, you could just go:

this.decoBoulder.allowed = true;

and it would just use the default values.

And then after all of the decorations have been configured for that biome, you would then add them to the biome's decos ArrayList in the order you want them to execute. The Chunk Generator would then loop through these, calling the DecoXXX's generate() method and generating the decoration if it's been enabled.

This approach is effectively a glorifed 'wrapping' treatment of existing decoration procedures, but given the complexity of the decoration system, I thought this might be a good approach to take, but I thought I'd open this up for discussion here to see what everyone thinks before I take it any further.

Yeah, this is a good next step at least. I'm actually finding I'm doing a similar thing with the terrain routines because it's too tricky keeping track of which float is which in these long parameter calls.

Out of curiosity (I'm not disagreeing), why did you pick an inner class?

To be honest, it was just because I was 'in there' already (RealisticBiomeBase) and couldn't think of a good package for them, so I just started writing in there. No idea of the pros or cons of doing it that way - it just kinda happened.

I suppose it would make sense to move them into their own package though, so I'll probably do that if no one has any objections to the overall approach.

i think this is reasonable, and i think we should be doing the same thing for surfaces and terrain. Cause with the current terrain wrapping TerrainBase becomes extremely long, and all the biome specific terrain classes are quite redundant. Also ill look at the code a bit more in depth later or tomorrow, there may be some advantages to making a few tweaks in some places. But the overall ArrayList and classes for decorations are pretty much the way to go i think.

left some comments at this commit f04c832

Yeah, I'll definitely have to move things out of TerrainBase one way or another because every third biome I end up saying "It needs to be like that but with ONE change" so the code would proliferate. But I'll see how I do things before diving into that. It's not that the wrapping was a bad idea, because it really helps understand what's going on now, it's just not the final state.

Ok peeps, I think the framework for the new decoration system is finally ready for review. I've only created about a dozen Deco classes, and so far I've only added the new system to the following 3 biomes:

Vanilla Forest
Vanilla Jungle
BOP Flower Field

(I didn't want to re-decorate too many biomes because we may want to modify the new system before going all-in.)

The deco classes are heavily commented, so hopefully they'll help explain what's going on. I've also created some documentation here:

https://teamrtg.gitbooks.io/rtg-code-documentation/content/biome_decoration.html

As for what's going on in the realistic biomes classes, the re-decoration basically involves removing all of the procedural decoration sections from the rDecorate() method and adding the equivalent Deco objects to the constructor. The goal is to eventually remove the rDecorate() method from all biomes.

So yeah, let me know what you guys think - I'm happy to try different implementations of this approach, but I don't think we'll be able to deviate too far from the approach itself without a full re-write of the chunk management/generation system, or sacrificing some aesthetic diversity.

It looks good! Two minor comments:

I'd do LogDistribution as a static class, with defaulted fields that are public and not final. (essentially, a parameter object). So LogDistribution could be set the same way you set other parameters for the decorator objects; changed manually in the constructor before disappearing into private objects.

Do Decorator5050 as an object that takes two decorator objects, does the boolean random check on which to call, and then dispatches the call. That could be re-used for other infrequent decorations.

I've done a Decorator5050 helper but haven't pushed it yet (ran out of time this morning). And I think I know what you mean about making the LogDistribution/TreeDistribution a static class, so I'll look at that today as well.

After that's been done, are we saying that it's ready to merge into dev?

Yes, and the sooner the better because time increases the chance of collisions.

Cool, I've pushed the 5050 helper, the static class for tree/log distribution, and an optional boolean flag for addDeco. Just one more tiny thing to add and then I'll be merging into dev later tonight after a good test drive.

Ok, the framework for the new decoration system has been merged into dev (f2088e5)

I'm going to start re-decorating the rest of the vanilla biomes, so you can expect some new deco objects soon.

I'm getting a sense of deja vu here -

I suggested something like this a while back for terrains and Pink, who's the big proponent of individual classes, came up with the mass wrapping as a compromise.

Anyway, there are currently three systems - calls to the wrapped routines (the vast majority), custom code in the class (mostly Highlands routines) and functional object juggling (a couple of classes in BoP, but there will be more to come, because it's proving a powerful approach). I find when I really care about a biome I rarely use the wrapped routines, although there are a lot of plains/rolling hills where I really don't care.

then maybe we should just skip the individual terrain classes and add the generateNoise to the biome classes. Then we can call TerrainBase.canyonTerrain(x,y) or whatever they are called, or we can put custom code in there. All i know is, most of those terrain classes are very redundant

the terrain wrappings should not be methods in TerrainBase IMO, but subclasses. What do you think?

Not so good as subclasses, because sometimes a routine uses more than one. When I want something currently not available through the set routines, I'm using a functional system which makes it easy to combine most effects. Good examples are TerrainBOPMountain and TerrainBOPOutback. I don't feel particularly compelled to go back and rewrite all the existing stuff - if the terrain effect is good, why change it? Also Ted and I have very different approaches to scaling (although, ironically, his approach would work better with the functional stuff) and I'm not eager to try to rewrite it all.

That said, I'm finding everything goes better with a little jitter, which is a good argument for rewriting everything to be functional (because you can just wrap it in a jitter and blam, you're done).

In terms of programming, these new routines are done as external plug-in classes.

all i know is, the current system seems weird, with terrain classes for every biome, and most of them just forwarding to one function

Im guessing this can be removed from the 0.8.0 milestone

also this is probably a great example of what could be a zube ticket instead

Agreed - these kinds of 'mini projects' would be good candidates for Zube tickets. Maybe we should use this mini project as a guinea pig to see how the tickets work in Zube?

yep, sure!

Closing this as we now have a new terrain system and a new decoration system. Surfaces are the only thing missing, but I don't think it's worth writing a new system for those in 1.7.10 as there really aren't that many biomes that need re-surfacing. Happy to re-open if anyone thinks otherwise though.