New bulk node light update

[juhdanad]

Based on #4682, please do not merge this before!
Edit: #4682 is merged.

This is an early version for testing, the code style is not final yet.
Lua changes are not documented yet.
Edit: now the PR is done.

The new lighting algorithm should fix #3421 (mostly, sometimes a well-lit area is overridden, probably by the mapgen). With this v7 mapgen should be playable with the moretrees mod.

This should also fix #4853 and fix #4852.

Changes:

• If the block above the changed area is not loaded, it gets loaded to avoid lighting bugs.
• Light is updated as soon as write_to_map is called on a voxel manipulator
• Therefore update_map does nothing
• Schematics, trees and voxel manipulators use the new lighting algorithm

[paramat]

This work is very much appreciated, these lighting bugs are one of the biggest problems in Minetest.

[juhdanad]

Done! The WIP label can be removed.

[HybridDog]

If many mods add map generation, all of them use the vmanip object and call calc_lighting,
then each time calc_lighting is executed the previous calculation is removed (it was in vain) and light is calculated again, am l wrong?
So instead calc_light could only set a flag indicating that the mapchunk needs light calculation after executing the mods' on_generateds.
Or does this PR remove the calc_light function from vmanip?

I assume this PR breaks my nuke mod, there when firing the rocket launcher, the explosion (nodes removal using vmanip) happens immediately, then, when the projectile reaches the target, sound is played and update_map is executed. So from the players' views the explosion happens later. But for any reason executing update_map later using minetest.after occasionally causes memory access error…

[juhdanad]

@HybridDog yes, it turned out there are now problems with map generating (mods that use singlenode mapgen with nolight flag)
I'll try to fix that.

[paramat]

If many mods add map generation, all of them use the vmanip object and call calc_lighting,
then each time calc_lighting is executed the previous calculation is removed (it was in vain) and light is calculated again, am l wrong?

In this case it would be the mods responsibility to delay the light calculation until after the last 'on generated' operation. Each mod can have a bool to disable lighting calculations.

[juhdanad]

Now I pushed a patch.
The behavior of lighting only changes for not mapgen voxel manipulators.
I have tested with the following mods:

• moretrees
• nuke
• stability
• pathv7
• tnt

I hope this is now free of bugs.

[juhdanad]

I have also made some speed tests:
Average light calculation time when moretrees places a tree: 125 ms->26 ms
TNT exploding in sunlight (old): 12 ms
TNT exploding in shadow (old): 2,4 ms
TNT exploding (new): 2,1 ms

[Fixer-007]

This is very important PR for me, will try it out in some time. This patch also includes ghost light fixes?

[juhdanad]

@Fixer-007 the first commit in itself is #4682 and the second commit is the new stuff. If #4682 gets merged, the first commit will disappear from there.

[Fixer-007]

Checked for now:

• fixes ghost light after lava casts
• fixes incorrect light after tnt in water explosion, lighting dynamics underwater looks good to me
• fixes shadows induced by tnt blown over to far horizon
• fixes that notorious L system trees shadows, moretrees are working nice!
• fixes ghost light after sofar's lightning mod strike, big nukes seem ok too

Please review 👍 👍 👍

[Fixer-007]

Needs rebase.

[juhdanad]

Rebased! Please review #4682 first, as merging it will remove two commits from there.

[paramat]

Unfortunately hmmmm isn't around recently, because it affects voxel manipulators and 'upate map' i feel hmmmm should look at this.

[paramat]

Bump! Unexciting but this is top priority for review as it fixes MTs most serious current bug.
It looks unlikely hmmmm will want to look at this.

[paramat]

Ok so i assume the code in map.cpp is something run when core mapgen generates.
It's worth timing this in a release build, 0.12s added to core mapgen mapchunk generation time is totally unacceptable for a rarely-triggered bug.
Usually usages of a non-mapgen-object-vm are not near generating mapchunks, i think my catacomb mod has an unreasonably fast generation sometimes, something to do with how the ABMs behave.

So essentially my opinion is let's not bother with the code in map.cpp in commit 5, please could you remove it? We could consider it in another PR later, it will only delay this PR.

[paramat]

In case you want to test i have updated my 'catacomb' mod to place full nodes under the stairs to block the light. I will retest your PR soon, with 'projects' mod too.

[HybridDog]

[Why] would your mod become slow?

Worldedit allows setting lots of nodes at once. If someone sets a 100x100x100 cube made of mese it takes ~0.05 s with my Worldedit and ~5.78 s with the original one, the current update_map takes lots of time. My version uses my function_delayer mod to distribute necessary map updates over time without causing noticeable server lag. By updating each chunk separately, l can omit unloaded chunks, which may be the majority, by using minetest.get_node_or_nil.

Are your voxel manipulators huge?

If players set lots of nodes, they're huge. If they set a few nodes, the overhead is negligible.

[Do] you update the same area often?

Yes, often players use the command frequently to change the nodes e.g. just because they noticed that cobble, then stone brick looked more fitting after all. If the map updates are delayed, no redundant ones are executed.

Is updating the map after your change still about 100 times faster than writing to map?

[juhdanad]

@HybridDog the current state is:

• write_to_map automatically corrects light (but light correction is faster than before)
• update_map does nothing

I will see how can I solve this problem, as simply disabling light updates is not enough: all of my lighting methods assume that the lighting on the map is correct (this gives some extra speed) and by placing incorrect light on the map it is not guaranteed that bugs will disappear when you update the light again.

[HybridDog]

thanks

[paramat]

I'm doubt it's worth altering your PR for this single very specialist usage, especially if it makes your PR less effective and slower for all other uses.

[juhdanad]

@paramat I have tested the post-mapgen light update on a release build, it takes 0,024 seconds.
However, I removed that as you asked. I hope that this pull is finished now.

@HybridDog I will open a new PR for you, I hope that you understand that this PR is complex enough already.

[juhdanad]

@HybridDog there are the corrections for you, I hope they will be sufficient: juhdanad@23fdd16
With this, you can disable light updating during write_to_map() and call minetest.fix_light() later (new function, repairs the light in one map block)
I will open a PR when this is mered.

[HybridDog]

thanks, does write_to_map still work about 100 time faster without light fixing?

[juhdanad]

@HybridDog I do not know how fast write_to_map without light update is. But light update is about 3-4 times faster than before.

[paramat]

Thanks, will test.
24ms is quite significant, i think too much for a rare bug. Anyway that can be considered in a separate PR.

[paramat]

Retested with 'catacomb' and 'projects' mods, all seems good now 👍

[SmallJoker]

Code looks ok. I really hope this is tested entirely.

[sofar]

👍 tested with large voxelmanips.

Any remaining issues can be ironed out later - I think this shouldn't wait much longer.

[MarkuBu]

Great. Than I finally can add l-system trees?

[paramat]

ab371cc
Hero.