Biome specific nodes in schematics and ground level

[bzt]

Hi,

I'd like to add support for ~, but I'm a bit lost in the source. Can anybody help me a bit?

First part: What I'm trying to do: if a schematic is loaded with a node name like "biome:node_top", then those node should be replaced by the biome's specified node_top node depending on the biome of the position passed to place_schematics.

So, this is what I've gathered so far:
- first I planned to simply create a Lua wrapper, but that's not possible as bulk node placement does not call Lua handlers (which is fine, it just means this has to be done in the C++ engine)
- there's already a Schematic::resolveNodeNames() method in mg_schematics.cpp, but unfortunately it does not get called on schematic placement.
- because schematics are stored by global node ids in memory, I'd have to add nodes for those biome specific nodes. That's because the name-replacement happens in Schematic::loadSchematicFromFile() method, when we don't know yet in which biome the schematic is going to be placed.
- placing a schematic is done in Schematic::placeOnMap() which in turn calls Schematic::blitToVManip(), therefore the best place would be in that function, around line 167, something like if(vm->m_data[vi].isBiomeNode()) vm->m_data[vi].replaceBiomeNode(mg);
- the DecoSchematic::generate() method in mg_decoration.cpp also calls that Schematic::blitToVManip() method, so that is definitely the right place.
- also in mg_decoration.cpp, Decoration::placeDeco() provides an example how to get the biome for a position, however it requires a MapGen object to do that (obviously).

My questions are:
a)
- How could one access the MapGen object in that method Schematic::blitToVManip()?

b)
Assuming I could extend Schematic::blitToVManip() with an mg argument:
- Schematic::placeOnMap() receives ServerMap object, which has a getBiomeParams() method, but not getBiomeObject(). Is there a way to get mg from a ServerMap?
- DecoSchematic::generate() does not even receive a ServerMap object, so how could I get MapGen to pass it as mg to blitToVMapip()?

Second part: this is simple, we just have to substract a value from the placement position, that's all. This looks clear to me, I just need your opinion about the planned modification.
- I've already implemented a way to store ground level in MTS files, which works great (Air blocks have param2 of 0x20 on the ground level, which does not interfere with anything, and 100% backward-compatible).
- I'm planning to add an s16 groundlevel property to class Schematic in mg_schematic.h line 122, after v3s16 size.
- I can detct and set that groundlevel property in Schematic::deserializeFromMts() which already does some post-processing on schemdata anyway.
- There's a need to modify three methods because there are three methods that calculate placement positions, these are:
- Schematic::placeOnVManip() in mg_schematic.cpp, line 197: p.Y -= groundlevel;
- Schematic::placeOnMap() in mg_schematic.cpp, line 228: p.Y -= groundlevel;
- DecoSchematic::generate() in mg_decoration.cpp, line 373: the current line p.Y += place_offset_y; should be replaced by p.Y += place_offset_y - schematic->groundlevel;
I've checked, the placement position checks are implemented in blitToVManip(), so modifying p.Y is fine in all cases. What are your thoughts on this? I could adjust p.Y in blitToVManip(), but that looks not right, as blit should not adjust the position I think.

Thanks, and awaiting for your answers,
bzt

[paramat]

> First part: What I'm trying to do: if a schematic is loaded with a node name like "biome:node_top", then those node should be replaced by the biome's specified node_top node depending on the biome of the position passed to place_schematics.

> Second part: this is simple, we just have to substract a value from the placement position
> the current line p.Y += place_offset_y; should be replaced by p.Y += place_offset_y - schematic->groundlevel;

If you want to work on these as a coding exercise then that is fine. However, to avoid disappointment and perhaps save you wasting time, it is best that i warn you now that i will disapprove these features for addition to the MT engine, as they will add complexity but are rarely useful.

Biome surface nodes should not be contained in a schematic, so i cannot see why you would include them and then try to replace them automatically.
'place y offset' is already usable per decoration, i cannot see the benefit of adding another y-offset per schematic file.
Schematics certainly should not contain additional y-offset information, that belongs in the decoration registration.
Both features may be useful in rare situations, but additions to mapgen code need a lot of justification, any new features need to be very useful to very large numbers of users.

[bzt]

If you want to work on these as a coding exercise then that is fine.

Don't patronize! No, I want to implement these because I need these. And I can tell you that there are many others who want the ground level feature, it is not just me. Long gone the days when I needed coding exercise, about 30 years ago, dear friend! Without offense, that cannot be told about you looking at the Minetest codebase and the number of unresolved issues in the repo. I just want to help, no need to be arrogant and descending.

However, to avoid disappointment and perhaps save you wasting time, it is best that i warn you now that i will disapprove these features for addition to the MT engine, as they will add complexity but are rarely useful.

Excuse me, but if you do disapprove something that you don't understand, and for which you don't know the demand, that's wrong. Have you made a poll? I bet you haven't.

And what complexity are you talking about? Ground level requires only one additional variable per schematic, and it does not increase the existing complexity at all as there's already a loop that iterates through the param values. Literally less than 10 SLoC modification needed!

Biome surface nodes should not be contained in a schematic, so i cannot see why you would include them and then try to replace them automatically.

Don't get me wrong, but you're obviously mistaken. Node types ARE stored in biomes right now, all I want is to utilize those (node_filler, node_top, node_stone, etc.) and make them available from schematics just like you can use mapgen alieses. Just because you cannot see the benefit in that doesn't mean there's no benefit!

And for the why, I've already gave you valid examples, but I'm willing to give you plenty more if you ask.

'place y offset' is already usable per decoration, i cannot see the benefit of adding another y-offset per schematic file. Schematics certainly should not contain additional y-offset information, that belongs in the decoration registration.

You obviously haven't spent time with schematics. Think about a well. Or the igloo. Or anything that has an underground part, like the desert temple, or like the.... do I really need to explain such self-explanatory things?

Ground level is independent of the decorator, it's the same in all cases for the same schematic. This is pretty obvious. Also it would result in bad placement if you'd allow different ground levels for the same schematic depending on the decoration registration.


I must ask, are you sure you know what feature I'm talking about? It surely looks like you're talking about something else.

Both features may be useful in rare situations, but additions to mapgen code need a lot of justification, any new features need to be very useful to very large numbers of users.

Just ask around the forum, and you'll see how wrong you are. These are features which would be welcomed by the community. Do a poll if you're in a doubt, but don't reject something just because of arrogancy, please. At least give a chance for a contribution, after all, that's what Open Source is about, isn't it? And for Christ sake, what mapgen code additions are you talking about???? I really think you got the wrong idea about this.

Cheers,
bzt

[Sokomine]

Hope you two can resolve this problem and find a workable solution. Beeing able to have more flexible replacements in a schematic (right now they're fixed after first load) would sometimes be great. What would help me in mg_villages would be to just get the list of decorations mapgen would like to place, modify that on the lua side (remove tree-decorations etc. where houses are supposed to stand), modify the replacements as well and send it back to mapgen to actually place. This isn't possible right now and probably will not become possible. There's no easy place in the code where that could be put in without causing normal mapgen to get slowed down - and probably no other mod that would ever make use of it. Plus full support of rotation is also needed...in the end, VoxelManip does the job, albeit slower.

Back when I started mg_villages, hmmm (who was working extensively in the past on mapgen) almost considered it abuse to use schematics for houses. They where intended for more nature-like decorations, placed randomly by mapgen, and not for player's creations.

Paramat does a very good job at developing MT further, and he listens to modders. But what would be nice for one game doesn't necessarily have to be suitable for all.

Perhaps you both ought to meet on irc and talk about the issue more directly.

[paramat]

I was not being patronizing, arrogant or 'descending'. I wanted to make it clear that i have no objection to your personal interest in this and am not asking you to stop coding something.

I do understand the basic intention of the first part of this feature, and can judge how useful that is. I have been MT's mapgen specialist core dev for 5 years.
You do not know the demand either. Even if there is demand it does not mean it is right for MT, that is why core devs decide and we do not decide based on polls (which can be very unrepresentative) or some kind of democracy.
This will certainly increase complexity.

> Second part: this is simple, we just have to substract a value from the placement position, that's all.

I am sorry if i have misunderstood the second part, i have tried, but there is no clear explanation at all of this in your post. The above quote is all there is and you have not clearly explained it yet.

I have probably spent more time with schematics than any core dev.

I will repeat what i wrote about the first part:

> Biome surface nodes should not be contained in a schematic, so i cannot see why you would include them and then try to replace them automatically.

You have not explained.
Schematics are defined with no gound nodes around them, only "air" nodes which are automatically replaced by the pre-existing biome nodes in those positions, so i cannot yet see the need for your feature.

'Mapgen code addition' means 'additional code in mapgen code', that seems obvious.

You have not explained clearly and have been aggressive and insulting.

Sokomine, i certainly do not want to talk to this person on IRC, understandably =)

[bzt]

Hope you two can resolve this problem and find a workable solution.

Yes, I agree, that would be nice. However @paramat made it perfectly clear he's not open to solutions.

But what would be nice for one game doesn't necessarily have to be suitable for all.

I wouldn't implement a game specific feature in an engine. Never.

My tone might have not been 100% right I admit, but I got pissed of by that arrogant "do it, but I'll blindly reject it no matter what" attitude. He got decided that without even seeing the actual patch and without asking the modders about the features.

Specially annoying because all of his reasons were clearly wrong btw. Let's examine them one by one:

1. add complexity: not true, because there's already a node replacement code, biomes already have node types, mapgen already can return biome for position, and support for ground level requires less than 10 SLoC and is backward compatible with the mts files. Under no circumstances can we call that complex.

2. rarely useful: also not true, because almost every structures affected (99% of the buildings that's for sure), and there are already structures in the game (both MCL2 and MTG) which use dirty workarounds to overcome the lack of ground level (and they are using different workarounds, may I add). This is not game specific, most games want to load structures on the surface, and there are already games that would need this.

3. "biome surface nodes should not be contained in a schematic" Why not? Give me one good reason at least! On the other hand there are undeniable benefits, and the modification is simple without performance impact.

4. "i cannot see why you would include them and then try to replace them". I've already explained why. Those schematics can be stored on disk and in memory in one copy, and instanciated with stone and pine on the taiga, jungle tree in jungle and with sandstone on the desert. Right now you have to create several schematics for this, which is a plain waste of resources, both storage and memory and very difficult to maintain (you have to edit more schematics in parallel and keep them synced manually!). Look up D.R.Y. principle.

5. "i cannot see the benefit of adding another y-offset per schematic file." Again, I've explained this, @paramat should have read my post. It is painfully obvious that ground level is the property of the structure stored in the schematic file. If you don't believe me, just check ANY blueprints. I meant it, literally ANY blueprint (IRL and MC alike).

6. "additions to mapgen code need a lot of justification" This is what makes me question if @paramat actually understands my suggestion. Mapgen code DOES NOT NEED modifications. Nothing! Everything required for these features are ALREADY IMPLEMENTED in the engine and in place. There's only one issue for the biome nodes, how to query the existing mg object, but that's all. Ground level has all the variables and all the requirements ALREADY IMPLEMENTED, and both suggestions are 100% backward compatible!

Cheers,
bzt

[paramat]

bzt, i realised there was probably a misunderstanding.
By 'a coding exercise' i did not mean an educational exercise or something a beginner does. That phrase can also mean 'a coding project'. Sorry for using an ambiguous phrase.

[ShadMOrdre]

I would like to input here, that I have been working in very similar directions, and have achieved quite a bit of some of this.

Using lib_mat / lib_eco as my base, I began exploring some of the simpler lua mapgens. The lua mapgens do not provide a heightmap or biomemap. However, the noises for heat and humidity exist. Using info provided by paramat in a different thread, I am currently exploring the ability to expose "a" biomemap for singlenode lua based mapgens. I have been emboldened in this endeavor by solving the heightmap issue, and can now easily substitute a lua mapgen heightmap to any mod that currently uses the standard mapgen "heightmap" object.

As for the labeling of nodes within a schematic, whether they be "biome:stone" or "modname:nodename", I feel that this maybe should have been a "no-brainer" from the start.

Currently, the engine will look for mapgen aliases. As long as a single mod provides those aliases, those aliases can be used game wide. An engine feature, and what bzt might actually be proposing, is simply the addition of biome based aliases from within the engine. It would be up to a mod to actually define what those "aliases" are, much as with the current mapgen aliases. Those aliases could then be used game wide, thus also, by default, from within any schematic that defines it.

The solution is actually rather simple. However, support for an engine change must have two core devs support, so if paramat can be convinced of this idea, then it could probably get implemented. I wish I could code C, otherwise you'd have a pull req, paramat.

I support the inclusion of biome aliases into the engine, from which mods could easily obtain biome specific data more readily.

+1

Shad

[bzt]

Hi

@paramat: apology accepted. I hasn't answered in the best tone and for that I do apologize to you as well.

@ShadMOrdre: thanks.

The lua mapgens do not provide a heightmap or biomemap.

The engine provides a way to query the biome for a particular position, and it also does store a biomemap already. This can be accessed from Lua, as it's exposed with the get_biome_data API. Not sure about the heightmap though, however I'm certain that can be workarounded by querying the nodes itself and looking for the first non-Air node.

It would be up to a mod to actually define what those "aliases" are, much as with the current mapgen aliases. Those aliases could then be used game wide

That's the best thing, mods are already doing this when they call register_biome. All biomes have properties like "node_stone" or "node_top", which are essentially good as aliases, and those properties are already usable game-wide (both from the engine and from Lua mods). I only wish to extend this to schematic files too, that's all.

About the other thing, storing the ground level, is there any question? I really don't know how to explain ground level better as it is such a basic thing. Maybe a demonstration would be appropriate, for example look at here (please forget that this is an MC link, and focus on the fact that the most bottom layer is not the ground level on this blueprint, and one layer at the middle is labelled as "Ground"). See also this and this.

Right now the first structure, the igloo is implemented as two separate structures (_top.mts and _basement.mts) in MCL2 simply because there's no ground level in the mts file currently. I think we can agree on that having two schematic files per structure is sub-optimal. Other mods are substracting hard-coded values from the Y coordinate, which is also not optimal. It would be better if the mts files could contain which layer is on the ground (not setting this would mean the most bottom layer is the ground level and thus the engine would subtract 0 from posY, therefore provide 100% backward-compatibility with current schematics).

Cheers,
bzt

[Sokomine]

It would be better if the mts files could contain which layer is on the ground (not setting this would mean the most bottom layer is the ground level and thus the engine would subtract 0 from posY, therefore provide 100% backward-compatibility with current schematics).

My workaround for that in handle_schematics is to store this information in the filename. The filename also includes information at which rotation the schematic was saved. This is of course a completely diffrent setup - the schematics are not spawned by mapgen but by my own mod.

[bzt]

Oh, how could I forgot about that? Sorry!!!
Yes, we should definitely add Sokomine's solution to the list of current workarounds!

(Side note, it's not that different, because your mod uses the place_schematic Lua API, which in turn calls the same Schematic::blitToVManip() method inside the engine)

Cheers,
bzt

[Sokomine]

(Side note, it's not that different, because your mod uses the place_schematic Lua API, which in turn calls the same Schematic::blitToVManip() method inside the engine)

No, it doesn't :-). At least not for important stuff. The minetest.place_schematic function can't handle the replacements necessary. It also fails to rotate nodes with facedir correctly in most situations. I've written my own functions to read .mts files, store them in an internal lua table and place them using VoxelManip. handle_schematics basicly does the described and adds a chest for spawning and/or saving such schematics easily. The backup of the landscape - so that players can revert to the previous state if they don't like the schematic - that is done via minetest.place_schematic directly.

[DrFrankenstone]

Doesn't fix rotation, but there is a way to get minetest.place_schematic() to do node replacements dynamically, I've used it in Hallelujah Mountains here and again in a PR to the Nether mod.

Basically the place_schematic cache is keyed off the .mts filename, and that can be malleated.

So you create an id that is unique for the node replacement table you intend to pass to place_schematic, see if that id is already in your "malleatedFilenames" table, if it is then call place_schematic() using the filename from the malleatedFilenames table, otherwise count how many items are already in the table and prepend that number of '.'..DIR_DELIM to the start of the .mts filename, storing this new malleated filename in the table under that id.

That way the table/cache is only added to as needed while new land is emerged, and resets whenever the server goes down.

It's an ugly workaround, but works cross-platform and dynamic node replacement is too important to me to forgo.

[ShadMOrdre]

So this may not be a popular option, but, ....

Schematics can be saved with any text string as the node name. Why not use stone, wood_door, brick_stair, much like aliases? You can easily alias to any of the aforementioned names. So instead of having to define "default:stone", and then replace the name when place_schematic, one could simply define an alias to "stone" and save that single definition within the schematic. Instead of using the builtin replace functionality, you could then just change the alias.

This is one of many reasons why I'll continue to use lua tables, instead of .mts files.

On another note, Sokomine, a nice feature in handle_schems would be to add the ability to bulk modify the param 2 rotation of a node within a schem. I've found far too many schems where there is a single stone block, rotated in ways are irrelevant. This bloats the size of the .mts file, needing to track each rotated node as a seperate node, when a single node def compresses, and replaces, far easier.

I've resorted to naming files with the following suffix: _x#_y#_z#_r#_o#

The letters represent the x,y,z dimensions, r is rotation, and o is y offset. The # symbol is a number. I add this suffix to each schem, as a quick visual definition of the schem inside the file.

Shad

[bzt]

Doesn't fix rotation, but there is a way to get minetest.place_schematic() to do node replacements dynamically

Yes, but I think it would be better to fix rotation in place_schematic, no? And about the replacement, yes, load schematic can accept a replacement table on the Lua side. The problem with that is, you can't access MTS node names from Lua, and you can't access that replacement table from the engine (from class Schematic) when it places the schematic. Also it is only accessible from Lua, and very inconvenient to use (you must construct a Lua table in advance for every schematic), specially if the engine already has a replacement table (both for mapgen aliases and biome nodes).

Schematics can be saved with any text string as the node name. Why not use stone, wood_door, brick_stair, much like aliases?

I wrote an MTS editor which is capable of converting those text strings (for example MCL2 to MTG or vica versa). Actually I had a feature request to support mapgen aliases too in the MTS files (like "stone", "watersource" etc.), so I had the impression this already works in the engine. Does it not? It surely does work for the mapgens.

This bloats the size of the .mts file, needing to track each rotated node as a seperate node, when a single node def compresses, and replaces, far easier.

True, but then we should have a way to indicate if all layers has the same probability etc. This leads to a completely different, chunk-based format, which would most definitely broke compatibility. It could be a goal for another future project maybe, but for now, I suggest to just focus on the features we can solve with the current format imho. Small steps at once :-)

Cheers,
bzt

[DrFrankenstone]

Yes, but I think it would be better to fix rotation in place_schematic, no? And about the replacement, yes, load schematic can accept a replacement table on the Lua side. The problem with that is, you can't access MTS node names from Lua, and you can't access that replacement table from the engine (from class Schematic) when it places the schematic. Also it is only accessible from Lua, and very inconvenient to use (you must construct a Lua table in advance for every schematic), specially if the engine already has a replacement table (both for mapgen aliases and biome nodes

Oh, I completely agree, place_schematic is important in multiple aspects of world creation and decoration and anything that improves its power improves the ability of Minetest to have interesting worlds.

I was surprised at paramat's resistance, as your improvements will be used if they become available and you do make it sound like little complexity is added.