Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Voxel Manips fail on mapchunks with caves and light glitches #13712

Closed
kromka-chleba opened this issue Aug 2, 2023 · 8 comments
Closed

Voxel Manips fail on mapchunks with caves and light glitches #13712

kromka-chleba opened this issue Aug 2, 2023 · 8 comments
Labels
Action / change needed Code still needs changes (PR) / more information requested (Issues) @ Mapgen @ Script API Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible

Comments

@kromka-chleba
Copy link
Contributor

Minetest version
Minetest 5.7.0
OS / Hardware

Operating system: Guix System x86_64, commit: 676508ac858928a2ec66f18ccfae17c9cec3dda2
CPU: AMD Ryzen 7 2700

Summary

I created a soil replacer based on Voxel Manip that replace whole mapchunks, yet it sometimes fails for areas smaller than one chunk. The failed areas are most often made of mapblocks and are often in places where there's a cave, river bed or glitched light. The bug is not reproducible for a given mapchunk - rerunning the VM often fixes the issue.
These VMs are basic VMs, they run after mapgen.

Steps to reproduce

Download Exile: https://codeberg.org/Mantar/Exile.git
Switch branch to "v4". Start a new creative world, type "/grantme all".
Switch season to winter by running "/set_day 63", fly around and witness the broken chunks.
You can switch back to spring by typing "/set_day 13" then again to winter by "/set_day 63" and the glitch should get fixed.
The code for my map updater is in mods/mapchunk_shepherd.

Example photos

Here you can see the stripe of soil that failed to be replaced. At the top there's a light glitch.
The VM doesn't save light so it shouldn't matter. As you can see it is 16 nodes wide.

stripe

Here a single mapblock failed and it even has a light glitch inside:

chunklet

Here we got a strange shape that appears to be a continuation of the river/cave:

river

Here another one, this time near a cave.

cave

@kromka-chleba kromka-chleba added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Aug 2, 2023
@sfan5
Copy link
Collaborator

sfan5 commented Aug 2, 2023

Where's the code that does this?

@kromka-chleba
Copy link
Contributor Author

@sfan5 does what?
I suspect the bug is on the C++ side.
If you mean my Mapchunk Shepherd - the thing that manages mapchunks, assigns labels and updates soil depending on in-game seasons then the thing is part of Exile (the game for Minetest).

The mod is here:
https://codeberg.org/Mantar/Exile/src/branch/v4/mods/mapchunk_shepherd
The VMs are registered by this code:
https://codeberg.org/Mantar/Exile/src/branch/v4/mods/mapchunk_shepherd/dogs.lua
The function that creates the VM is called ms.create_simple_replacer or it's twin ms.create_param2_aware_replacer but it doesn't matter which one. Soil is replaced using the first, plants are replaced using the second. Both fail on these glitched places.

@kromka-chleba
Copy link
Contributor Author

These patches smaller than one mapchunk should not exist because I make sure to use whole mapchunks.

-- By default chunksize is 5
local blocks_per_chunk = tonumber(minetest.get_mapgen_setting("chunksize"))
local chunk_side = blocks_per_chunk * 16
-- this logic comes from Minetest source code, see src/mapgen/mapgen.cpp
local mapchunk_offset = -16 * math.floor(blocks_per_chunk / 2)

Then I keep chunks with hashed pos_min of each chunk:

-- Converts node coordinates to mapchunk coordinates
function ms.node_pos_to_mapchunk_pos(pos)
    pos = vector.subtract(pos, mapchunk_offset)
    pos = vector.divide(pos, chunk_side)
    pos = vector.floor(pos)
    return pos
end

-- A global function to get hash from pos
function ms.mapchunk_hash(pos)
    pos = ms.node_pos_to_mapchunk_pos(pos)
    pos = vector.multiply(pos, chunk_side)
    pos = vector.add(pos, mapchunk_offset)
    return minetest.hash_node_position(pos)
end

And this is how I get pos_min and pos_max for Voxel Manips:

function ms.mapchunk_borders(hash)
    local pos_min = minetest.get_position_from_hash(hash)
    local pos_max = vector.add(pos_min, chunk_side - 1)
    return pos_min, pos_max
end

@fluxionary
Copy link
Contributor

the third image looks like the issue described in #9357, not related to a river or cave.

@kromka-chleba
Copy link
Contributor Author

I have num_emerge_threads set to 1 and this bug happens anyway.
Also these are basic Voxel Manipulators, not mapgen VMs.
My code checks if the mapchunks are loaded or active and only then proceeds.
Though these two bugs look similar.

I don't know how the C++ backend works, but if the same code VMs use is reused for map generation then I guess the bug is there.

@fluxionary
Copy link
Contributor

I have num_emerge_threads set to 1 and this bug happens anyway.

#9357 doesn't actually require more than one mapgen thread. your-land tried to mitigate it by using only 1 mapgen thread and the problem still happens, though less often. see https://gitea.your-land.de/your-land/bugtracker/issues/850 for an extensive list of that issue on that server.

@kromka-chleba
Copy link
Contributor Author

kromka-chleba commented Aug 17, 2023

My wild guess is that somehow cave gen runs into a race condition with my VM.
My code checks if the chunk is loaded/active so it should be already after generation, yet this happens.
But it almost always happens when I move faster than mapgen makes new chunks while my VM tries upgrading the chunks. Idk if this helps.

Another one:

another_cave

@Zughy
Copy link
Contributor

Zughy commented Oct 3, 2023

@kromka-chleba
image
image

Is this what you're talking about? Otherwise I wasn't able to find anything (MT 5.7). Running /set_day 13 and again /set_day 63 doesn't fix it. Also, I had no luck in finding glitches with shaders on, I don't know if it's relevant.

@Zughy Zughy added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 3, 2023
@Zughy Zughy closed this as not planned Won't fix, can't repro, duplicate, stale Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Action / change needed Code still needs changes (PR) / more information requested (Issues) @ Mapgen @ Script API Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible
Projects
None yet
Development

No branches or pull requests

5 participants