minetest / minetest

Luanti (formerly Minetest) is an open source voxel game-creation platform with easy modding and game creation
https://www.minetest.net/
Other
10.86k stars 2.04k forks source link

Decoration spawn into existing decorations of type `schematic` and `lsystem` #15135

Open cx384 opened 2 months ago

cx384 commented 2 months ago

Summary

When two decorations get generated at the same position, the second one overrides parts of the first one, resulting e.g. in a tree where the lowest trunk becomes a grass node.

It only happens for deco_type schematic and lsystem, but not for two decorations of type simple with e.g. one of height = 2.

This is not the same problem as #14919, since the priority does not prevent a later decoration of type schematic and lsystem from being generated, they just override each other resulting in corrupted structures. A solution would have to somehow take already generated decorations into account before generating new ones.

Also relevant #11774

Steps to reproduce

Use this in MTG:

minetest.register_decoration({
    name = "test1",
    deco_type = "simple",
    place_on = {"default:dirt_with_grass", "default:sand", "default:desert_sand"},
    fill_ratio = 1,
    decoration = "default:cobble",
})
minetest.register_decoration({
    name = "test2",
    deco_type = "schematic",
    place_on = {"default:dirt_with_grass", "default:sand", "default:desert_sand"},
    fill_ratio = 1,
    schematic = {
        size = {x = 1, y = 3, z = 1},
        data = {
            {name = "default:glass", param1 = 255, param2 = 0},
            {name = "default:glass", param1 = 255, param2 = 0},
            {name = "default:glass", param1 = 255, param2 = 0},
        },
    },
})
SmallJoker commented 2 months ago

DecoSimple: aborts as soon as non-air and non-ignore is found. https://github.com/minetest/minetest/blob/733a019bf5aefddb824e5643b791ca25fc620b03/src/mapgen/mg_decoration.cpp#L375-L376

Schematic: only ignore and air nodes are replaced. Perhaps an early abort would be more suitable because currently it appears like nodes were replaced. https://github.com/minetest/minetest/blob/733a019bf5aefddb824e5643b791ca25fc620b03/src/mapgen/mg_schematic.cpp#L186-L191

As far as I can see there's no such check in make_ltree (unhandled force placement option).

Hence this testing code might depend on the mod loading order, which affects the decoration placement order.

EDIT: to solve this issue in every case, the following flag concept could be used: (see also: dungeon gen)

#define VMANIP_FLAG_DECO_GENERATED VOXELFLAG_CHECKED1

vm->clearFlag(VMANIP_FLAG_DECO_GENERATED);

// in Decoration::canPlaceDecoration
if (vm->m_flags[vi] & VMANIP_FLAG_DECO_GENERATED)
    return false;

// in *::generate(..)
vm->m_flags[vi] |= VMANIP_FLAG_DECO_GENERATED;
kno10 commented 1 month ago

14919 can help here by having a deterministic order, and using this to force certain decorations to be placed last.

For the schematic, if you do not specify force_place it is somewhat intentional that this gets corrupted, and by not specifying flags to select floors, you will ask it to rely on the heightmap (which does not get updated by the cobble decoration you placed before). I believe all_floors might help, too.

I am not saying the decoration placement is flawless, or easy to understand, or well-documented (see #14919 and #15012 - in particular order-dependent randomness is a pain), but to some extend you "got what you asked for". You probably wanted flags = "force_placement, all_floors".