VoxeLibre/VoxeLibre
VoxeLibre
/
VoxeLibre
60
134
Fork 177
Code Issues 952 Pull Requests 93 Projects 4 Releases Wiki Activity

Crash while generating dungeon or /spawnstruct ice_spike_small #1212

New Issue
Closed
opened 2021-02-27 11:10:53 +01:00 by AFCMS · 44 comments
AFCMS commented 2021-02-27 11:10:53 +01:00
Member
Copy Link

Since I update Mineclone2 and download Minetest 5.4, I experiment 3 crash (over 100 dungeons generated).
The window closes abruptly and there is no log.

2021-02-27 09:14:19: ACTION[Server]: Mob spawned: mobs_mc:skeleton at (-32,-46.5,291)
2021-02-27 09:14:20: ACTION[Server]: Mob spawned: mobs_mc:wolf at (-5,41.5,247)
2021-02-27 09:14:20: ACTION[Server]: Mob spawned: mobs_mc:wolf at (35,29.5,245)
2021-02-27 09:14:20: ACTION[Server]: singleplayer digs mcl_core:vine at (-34,8,269)
2021-02-27 09:14:21: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (-126,-45,392)


-------------
  Separator
-------------

2021-02-27 09:16:38: ACTION[Main]: hb.register_hudbar: health
2021-02-27 09:16:38: ACTION[Main]: hb.register_hudbar: breath
2021-02-27 09:16:38: ACTION[Main]: hb.register_hudbar: hunger
Since I update Mineclone2 and download Minetest 5.4, I experiment 3 crash (over 100 dungeons generated). The window closes abruptly and there is no log. ``` 2021-02-27 09:14:19: ACTION[Server]: Mob spawned: mobs_mc:skeleton at (-32,-46.5,291) 2021-02-27 09:14:20: ACTION[Server]: Mob spawned: mobs_mc:wolf at (-5,41.5,247) 2021-02-27 09:14:20: ACTION[Server]: Mob spawned: mobs_mc:wolf at (35,29.5,245) 2021-02-27 09:14:20: ACTION[Server]: singleplayer digs mcl_core:vine at (-34,8,269) 2021-02-27 09:14:21: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (-126,-45,392) ------------- Separator ------------- 2021-02-27 09:16:38: ACTION[Main]: hb.register_hudbar: health 2021-02-27 09:16:38: ACTION[Main]: hb.register_hudbar: breath 2021-02-27 09:16:38: ACTION[Main]: hb.register_hudbar: hunger ```
LizzyFleckenstein03 commented 2021-02-27 14:46:01 +01:00
Member
Copy Link

If the window closes, without any debug log, it's a MT issue. The engine SEGFAULTs or aborts etc. It happens to me as well, see https://github.com/minetest/minetest/issues/10995

If the window closes, without any debug log, it's a MT issue. The engine SEGFAULTs or aborts etc. It happens to me as well, see https://github.com/minetest/minetest/issues/10995
kay27 commented 2021-02-27 15:04:24 +01:00
Contributor
Copy Link

I don't think it's related to this issue, currently MCL2 doesn't contain schematic serialization during LBM calls, but I've also seen two random window closings without error records in debug log, these times both were under windows 10 in Singleplayer mode, so CPU was slightly overloaded, and the last record was about the dungeons too. I think it's all magen related, need to get debug messages from c++, but when I compile minetest myself under linux (more powerful pc) with debug messages, there are no such problems for now. Is there a way to get c++ debug messages in windows? :)

I don't think it's related to this issue, currently MCL2 doesn't contain schematic serialization during LBM calls, but I've also seen two random window closings without error records in debug log, these times both were under windows 10 in Singleplayer mode, so CPU was slightly overloaded, and the last record was about the dungeons too. I think it's all magen related, need to get debug messages from c++, but when I compile minetest myself under linux (more powerful pc) with debug messages, there are no such problems for now. Is there a way to get c++ debug messages in windows? :)
LizzyFleckenstein03 commented 2021-02-27 15:27:08 +01:00
Member
Copy Link

The sad news is, there are no error messages. C is a very low-level language, if you say "access the memory at this access" that is actually what the hardware does. If there is invalid memory access the program will just be killed by the SIGSEGV signal, which is one by some microcode (in the MMU i think) that depends on the hardware and takes care of virtual memory and paging, if it would be nothing like that youre computer would in the best case crash, or worse, start overwriting memory it is not supposed to and destroy itself completely (possibly overwriting the OS, or even writing stuff disk etc.). Having something like SEGFAULT is already pretty high-level. C++ does not really differ here, from a ABI perspective C and C++ are mostly the same thing, they only really differ at compile time. There are some things that makes C++ binaries different e.g. the C++ compiler replaces function names to make overloading possible; but invalid memory access stays invalid memory access, even in other compiled languages like Go. I think the only compiled language that mostly prevents this is Rust; Rust has insane compile time checks to make sure the developer does not do anything that could cause undefined behavior. But the dev has to follow a bunch of stupid rules to keep that possible, only one mutable reference to an object at one time and all that stuff, I personally hate it. But even in Rust, there is an unsafe keyword that allows messing around. The sad.

Long story short, the downside of compiled languages is that if they mess up, they actually mess up. You can use -DCMAKE_BUILD_TYPE=Debug in the cmake command and then use gdb to debug a program, effectively inspecting the memory of it after it failed with segmentation fault or arborted etc., and since the call stack is located in mem as well, you can look at it and find out where it happened, what args were passed to the functions etc. and if there are debugging symbols in the binary (therefore the CMAKE_BUILD_TYPE=Debug) the debugger will also tell you in which source file and line the instruction that caused the problem is located.

The sad news is, there are no error messages. C is a very low-level language, if you say "access the memory at this access" that is actually what the hardware does. If there is invalid memory access the program will just be killed by the SIGSEGV signal, which is one by some microcode (in the MMU i think) that depends on the hardware and takes care of virtual memory and paging, if it would be nothing like that youre computer would in the best case crash, or worse, start overwriting memory it is not supposed to and destroy itself completely (possibly overwriting the OS, or even writing stuff disk etc.). Having something like SEGFAULT is already pretty high-level. C++ does not really differ here, from a ABI perspective C and C++ are mostly the same thing, they only really differ at compile time. There are some things that makes C++ binaries different e.g. the C++ compiler replaces function names to make overloading possible; but invalid memory access stays invalid memory access, even in other compiled languages like Go. I think the only compiled language that mostly prevents this is Rust; Rust has insane compile time checks to make sure the developer does not do anything that could cause undefined behavior. But the dev has to follow a bunch of stupid rules to keep that possible, only one mutable reference to an object at one time and all that stuff, I personally hate it. But even in Rust, there is an `unsafe` keyword that allows messing around. The sad. Long story short, the downside of compiled languages is that if they mess up, they actually mess up. You can use `-DCMAKE_BUILD_TYPE=Debug` in the cmake command and then use `gdb` to debug a program, effectively inspecting the memory of it after it failed with segmentation fault or arborted etc., and since the call stack is located in mem as well, you can look at it and find out where it happened, what args were passed to the functions etc. and if there are debugging symbols in the binary (therefore the CMAKE_BUILD_TYPE=Debug) the debugger will also tell you in which source file and line the instruction that caused the problem is located.
kay27 commented 2021-02-27 15:42:58 +01:00
Contributor
Copy Link

I didn't that in linux, I mean I didn't use -Dsomething, except some disables, because I built server only, but get perfect log with map generation events. So probably there would be the something if it would crash, but it works fine. In windows you specify command-line option to get verbose log but there's no log at all, so odd

I didn't that in linux, I mean I didn't use -Dsomething, except some disables, because I built server only, but get perfect log with map generation events. So probably there would be the something if it would crash, but it works fine. In windows you specify command-line option to get verbose log but there's no log at all, so odd
LizzyFleckenstein03 commented 2021-02-27 16:47:46 +01:00
Member
Copy Link

The warning/error messages you see in the log are messages that C++ prints because it realizes something bad happened / an exception was caught. That's not what I was talking about; the reason why the window closes unexpectedly or the server crashes without printing anything into the log means SIGSEGV, SIGABRT or SIGKILL (in this case SIGSEGV). Segfault means invalid memory access, the program is stopped immediately and the only info you get is "Segmentation fault". The program cannot register a custom handler for this signal. SIGKILL happens when the user forcefully kills the program ("kill -9" - SIGKILL has the value of 9 on linux) or an insane memory leak occurs and the program is killed by the kernel (the latter one does not happen very often, so don't think it's this one). The program cannot register a custom handler for this signal either. SIGABRT can have various causes, e.g. an uncaught C++ exception; integer division by zero; a floating point exception; a failed assertion or the program calling abort() itself. SIGABRT usually gives a bit more detailed information; it's only printed to stderr, not to the minetest logfile tho. Programs can register a custom handler, but MT does not.

My post only explains the signals that are relevant here, there are many more, like SIGINT and SIGTERM or SIGUSR1 and SIGUSR2, but they don't matter since they dont kill the program or minetest has handlers implemented and will put something into the logfile before quitting.

Note that SmallJoker already found out what the reason for the crash is using the detailed gdb backtrace I've sent. Some content id -> node name map is empty, why exactly is unclear yet, but he suggested a patch to temporary fix it and print a waring to the logfile if the map is empty instead of just letting the program die in a very ugly way, the patch works. (Ofc that is not the final solution, the fact that the map is empty needs to be fixed, but for now the patch will keep out servers from crashing)

The warning/error messages you see in the log are messages that C++ prints because it realizes something bad happened / an exception was caught. That's not what I was talking about; the reason why the window closes unexpectedly or the server crashes without printing anything into the log means SIGSEGV, SIGABRT or SIGKILL (in this case SIGSEGV). Segfault means invalid memory access, the program is stopped immediately and the only info you get is "Segmentation fault". The program cannot register a custom handler for this signal. SIGKILL happens when the user forcefully kills the program ("kill -9" - SIGKILL has the value of 9 on linux) or an insane memory leak occurs and the program is killed by the kernel (the latter one does not happen very often, so don't think it's this one). The program cannot register a custom handler for this signal either. SIGABRT can have various causes, e.g. an uncaught C++ exception; integer division by zero; a floating point exception; a failed assertion or the program calling abort() itself. SIGABRT usually gives a bit more detailed information; it's only printed to stderr, not to the minetest logfile tho. Programs can register a custom handler, but MT does not. My post only explains the signals that are relevant here, there are many more, like SIGINT and SIGTERM or SIGUSR1 and SIGUSR2, but they don't matter since they dont kill the program or minetest has handlers implemented and will put something into the logfile before quitting. Note that SmallJoker already found out what the reason for the crash is using the detailed gdb backtrace I've sent. Some content id -> node name map is empty, why exactly is unclear yet, but he suggested a patch to temporary fix it and print a waring to the logfile if the map is empty instead of just letting the program die in a very ugly way, the patch works. (Ofc that is not the final solution, the fact that the map is empty needs to be fixed, but for now the patch will keep out servers from crashing)
kay27 commented 2021-02-27 17:16:53 +01:00
Contributor
Copy Link

Maybe I can’t realize it well :) they both speak about LBM and serialization - okay, I’ve removed it immediately, but both crashes were after that. There is no more serialization in LBM (at least in lua), so it’s a bit unclear for me. Don’t you have a dump of SIGSEGV?

Maybe I can’t realize it well :) they both speak about LBM and serialization - okay, I’ve removed it immediately, but both crashes were after that. There is no more serialization in LBM (at least in lua), so it’s a bit unclear for me. Don’t you have a dump of SIGSEGV?
kay27 commented 2021-02-27 17:39:47 +01:00
Contributor
Copy Link

@EliasFleckenstein03 please take a look at this @paramat's comment:

is_ground_content = false is required for any nodes placed in on_generated. If you are placing 'natural' nodes it's best to create new mod nodes that drop the normal version.
River excavation must overgenerate because terrain generation does.

It just drives me crazy. We really generate many things with is_ground_content = true. I can't find proper documentation which would prevent us from doing that. Maybe the problem is this! Don't you have an idea how to implement his suggestion of dropping, well... some stones?

@EliasFleckenstein03 please take a look at [this](https://github.com/minetest/minetest/issues/7379#issuecomment-392823020) @paramat's comment: > ```is_ground_content = false``` is required for any nodes placed in ```on_generated```. If you are placing 'natural' nodes it's best to create new mod nodes that drop the normal version. > River excavation must overgenerate because terrain generation does. It just drives me crazy. We really generate many things with ```is_ground_content = true```. I can't find proper documentation which would prevent us from doing that. Maybe the problem is this! Don't you have an idea how to implement his suggestion of dropping, well... some stones?
kay27 added the
nodes
#P1 CRITICAL
 labels 2021-02-27 17:48:11 +01:00
kay27 reopened this issue 2021-02-27 17:48:19 +01:00
kay27 added the
mapgen
needs more information
 labels 2021-02-27 17:49:30 +01:00
kay27 commented 2021-02-27 20:19:25 +01:00
Contributor
Copy Link

The following command immediately crashes the game without an error in log, as described:

  • /spawnstruct ice_spike_small

The following commands generate ERROR[Emerge-0]: NodeResolver: failed to resolve node name ''. (but the game continues) - probably something's wrong with schems:

  • /spawnstruct boulder
  • /spawnstruct ice_spike_large
The following command immediately crashes the game without an error in log, as described: + ```/spawnstruct ice_spike_small``` The following commands generate ```ERROR[Emerge-0]: NodeResolver: failed to resolve node name ''.``` (but the game continues) - probably something's wrong with schems: + ```/spawnstruct boulder``` + ```/spawnstruct ice_spike_large```
kay27 added
bug
 and removed
needs more information
 labels 2021-02-27 21:22:13 +01:00
kay27 added this to the 0.71.0 milestone 2021-02-27 21:22:29 +01:00
kay27 changed title from Many crashes while generating dungeon to Crash while generating dungeon or /spawnstruct ice_spike_small 2021-02-27 21:23:50 +01:00
iXware71 commented 2021-02-27 23:45:14 +01:00
Copy Link

/spawnstruct ice_spike_large produced the same problem

/spawnstruct ice_spike_large produced the same problem
ryvnf commented 2021-02-28 11:52:14 +01:00
Contributor
Copy Link

I don't think we should attempt to create a workaround for this in Mineclone2 unless we know (from official documentation) that we are using the Lua API wrong. I don't think a 2 year old comment in an issue discussion by a Minetest dev should be taken as an indication that something is done wrong.

One of the release goals for 0.72 is to remove workarounds for old engine versions as part of the refactoring. I think creating a workaround for this which obviously seems to be an engine bug is doing quite the opposite. The way I understand it the problems also only started to become a problem Minetest 5.4. Servers have been running Minetest 5.3 for a long time without problems.

So I suggest we wait until the Minetest devs have made a patch for the problem. They should probably create a minor release once this has been resolved. I don't think it is reasonable for Mineclone2 to be compatible with broken engine versions.

I don't think we should attempt to create a workaround for this in Mineclone2 unless we know (from official documentation) that we are using the Lua API wrong. I don't think a 2 year old comment in an issue discussion by a Minetest dev should be taken as an indication that something is done wrong. One of the release goals for 0.72 is to remove workarounds for old engine versions as part of the refactoring. I think creating a workaround for this which obviously seems to be an engine bug is doing quite the opposite. The way I understand it the problems also only started to become a problem Minetest 5.4. Servers have been running Minetest 5.3 for a long time without problems. So I suggest we wait until the Minetest devs have made a patch for the problem. They should probably create a minor release once this has been resolved. I don't think it is reasonable for Mineclone2 to be compatible with broken engine versions.
ryvnf added the
needs engine change
 label 2021-02-28 11:53:42 +01:00
ryvnf removed the
needs engine change
 label 2021-02-28 11:54:44 +01:00
kay27 commented 2021-02-28 12:53:11 +01:00
Contributor
Copy Link

Servers have been running Minetest 5.3 for a long time without problems.

This is wrong.

It has begun very very long ago and we even don't know from which engine version.
For me it starts here: https://forum.minetest.net/viewtopic.php?p=381747#p381747
But before that I almost hadn't seen structures in MCL2 except rail corridors which uses node manipulations and works fine just because of that.

Now we about to release 'the villages'. And it's just inappropriate to do that with this bug, at least for me. And I'm not so much sure they'll fix it 'in time', if ever.

> Servers have been running Minetest 5.3 for a long time without problems. This is wrong. It has begun very very long ago and we even don't know from which engine version. For me it starts here: https://forum.minetest.net/viewtopic.php?p=381747#p381747 But before that I almost hadn't seen structures in MCL2 except rail corridors which uses node manipulations and works fine just because of that. Now we about to release 'the villages'. And it's just inappropriate to do that with this bug, at least for me. And I'm not so much sure they'll fix it 'in time', if ever.
kay27 commented 2021-02-28 13:45:58 +01:00
Contributor
Copy Link

I hope so much it's fixed now.
05a3b4e60c
Closing.
Please reopen this (or open new issue), if find again crashes like this.

I hope so much it's fixed now. https://git.minetest.land/MineClone2/MineClone2/commit/05a3b4e60ce46ae76c1783debdb534ef8cd72ed6 Closing. Please reopen this (or open new issue), if find again crashes like this.
kay27 closed this issue 2021-02-28 13:46:00 +01:00
ryvnf commented 2021-02-28 15:19:48 +01:00
Contributor
Copy Link

Sorry but how does that commit solve the actual issue? Looks to me like this does not change anything. Is the point that a server admin running on 5.4 would manually remove the annotated lines? How would they understand that without any documentation? In the issue created by EliasFleckenstein on the Minetest engine it looks like it was triggered by an Acacia Tree, not ice spikes. Crashes can probably still occur after removing the annotated lines in your commit.

I can't find any mention of a crash in that forum post and how would you know it is the same issue?

I think we should have a discussion if Mineclone2 should attempt to do workarounds for this kind of stuff. No disrespect but I don't think it is very good to merge changes to the master branch when there is an ongoing discussion.

I would also like to hear what @EliasFleckenstein03 think about making workarounds for stuff like this.

It also looks like SmallJoker has found a solution for this: https://github.com/minetest/minetest/issues/10995#issuecomment-787438358

Sorry but how does that commit solve the actual issue? Looks to me like this does not change anything. Is the point that a server admin running on 5.4 would manually remove the annotated lines? How would they understand that without any documentation? In the issue created by EliasFleckenstein on the Minetest engine it looks like it was triggered by an Acacia Tree, not ice spikes. Crashes can probably still occur after removing the annotated lines in your commit. I can't find any mention of a crash in that forum post and how would you know it is the same issue? I think we should have a discussion if Mineclone2 should attempt to do workarounds for this kind of stuff. No disrespect but I don't think it is very good to merge changes to the master branch when there is an ongoing discussion. I would also like to hear what @EliasFleckenstein03 think about making workarounds for stuff like this. It also looks like SmallJoker has found a solution for this: https://github.com/minetest/minetest/issues/10995#issuecomment-787438358
kay27 commented 2021-02-28 15:44:09 +01:00
Contributor
Copy Link

This patch based on SmallJoker’s find. I have reverted biome decorations to direct placement without serialization. Comment added mostly for myself, to not forget WHY :) I see no reason to delay the release keeping critical bugs in master when they could be wrapped easily, hope you agree with that. Sorry, I’m a bit nervous about this bug, I need to explore mapgen code to better understand what’s going on. But it short: structures had problems like disappearance or slicing, I fixed them one by one and always got new bugs, until the villages, which escalated this quickly. There is no doc requires doing emerge_area before place_schematic, so it’s already a wrap. But when you don’t know the size of schematic you need to serialize it to know it, not to emerge too much, to make it work faster... Now all biome decoration, like trees, boulders and spikes, excluded from this wrap, so we may call it some kind of unwrap, and I totally agree that I’m doing some weird things, so if you know how to make it better — it would be very welcome

This patch based on SmallJoker’s find. I have reverted biome decorations to direct placement without serialization. Comment added mostly for myself, to not forget WHY :) I see no reason to delay the release keeping critical bugs in master when they could be wrapped easily, hope you agree with that. Sorry, I’m a bit nervous about this bug, I need to explore mapgen code to better understand what’s going on. But it short: structures had problems like disappearance or slicing, I fixed them one by one and always got new bugs, until the villages, which escalated this quickly. There is no doc requires doing emerge_area before place_schematic, so it’s already a wrap. But when you don’t know the size of schematic you need to serialize it to know it, not to emerge too much, to make it work faster... Now all biome decoration, like trees, boulders and spikes, excluded from this wrap, so we may call it some kind of unwrap, and I totally agree that I’m doing some weird things, so if you know how to make it better — it would be very welcome
kay27 commented 2021-03-07 00:21:00 +01:00
Contributor
Copy Link

Under Win10 I'm still experiencing failures like described in the first post.

2021-03-07 03:17:01: ACTION[Server]: singleplayer activates mcl_torches:torch
2021-03-07 03:17:03: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2964,-40,-1759)
2021-03-07 03:17:03: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2962,-40,-1759)
2021-03-07 03:17:04: ACTION[Server]: singleplayer activates mcl_torches:torch
2021-03-07 03:17:06: ACTION[Server]: singleplayer activates mcl_torches:torch
2021-03-07 03:17:09: ACTION[Server]: Mob spawned: mobs_mc:slime_tiny at (2957,-46.5,-1788)
2021-03-07 03:17:10: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2957,-48,-1762)
2021-03-07 03:17:19: ACTION[Server]: Mob spawned: mobs_mc:rabbit at (2953,53.5,-1779)
2021-03-07 03:17:21: ACTION[Server]: singleplayer activates mcl_torches:torch
2021-03-07 03:17:21: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2984,-3,-1771)
2021-03-07 03:17:21: ACTION[Server]: Mob spawned: mobs_mc:wolf at (2977,48.5,-1755)
2021-03-07 03:17:21: ACTION[Server]: Mob spawned: mobs_mc:donkey at (3009.5,29.5,-1777.5)
2021-03-07 03:17:21: ACTION[Server]: Mob spawned: mobs_mc:wolf at (2965,63.5,-1779)
2021-03-07 03:17:21: ACTION[Server]: singleplayer activates mcl_torches:torch
2021-03-07 03:17:22: ACTION[Server]: singleplayer activates mcl_torches:torch
2021-03-07 03:17:23: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2983,-2,-1777)
2021-03-07 03:17:31: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (2997,145,-1892)
2021-03-07 03:17:31: ACTION[Server]: Mob spawned: mobs_mc:squid at (2994,-0.5,-1686)
2021-03-07 03:17:31: ACTION[Server]: Mob spawned: mobs_mc:squid at (3037,-1.5,-1690)
2021-03-07 03:17:37: ACTION[Server]: Mob spawned: mobs_mc:baby_zombie at (3150,-12.5,-1696)
2021-03-07 03:17:41: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (3260,-41,-1827)
2021-03-07 03:17:44: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (3249,-42,-1738)

So biome decoration - it was another problem which is luckily fixed now

Under Win10 I'm still experiencing failures like described in the first post. ``` 2021-03-07 03:17:01: ACTION[Server]: singleplayer activates mcl_torches:torch 2021-03-07 03:17:03: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2964,-40,-1759) 2021-03-07 03:17:03: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2962,-40,-1759) 2021-03-07 03:17:04: ACTION[Server]: singleplayer activates mcl_torches:torch 2021-03-07 03:17:06: ACTION[Server]: singleplayer activates mcl_torches:torch 2021-03-07 03:17:09: ACTION[Server]: Mob spawned: mobs_mc:slime_tiny at (2957,-46.5,-1788) 2021-03-07 03:17:10: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2957,-48,-1762) 2021-03-07 03:17:19: ACTION[Server]: Mob spawned: mobs_mc:rabbit at (2953,53.5,-1779) 2021-03-07 03:17:21: ACTION[Server]: singleplayer activates mcl_torches:torch 2021-03-07 03:17:21: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2984,-3,-1771) 2021-03-07 03:17:21: ACTION[Server]: Mob spawned: mobs_mc:wolf at (2977,48.5,-1755) 2021-03-07 03:17:21: ACTION[Server]: Mob spawned: mobs_mc:donkey at (3009.5,29.5,-1777.5) 2021-03-07 03:17:21: ACTION[Server]: Mob spawned: mobs_mc:wolf at (2965,63.5,-1779) 2021-03-07 03:17:21: ACTION[Server]: singleplayer activates mcl_torches:torch 2021-03-07 03:17:22: ACTION[Server]: singleplayer activates mcl_torches:torch 2021-03-07 03:17:23: ACTION[Server]: singleplayer places node mcl_torches:torch_wall at (2983,-2,-1777) 2021-03-07 03:17:31: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (2997,145,-1892) 2021-03-07 03:17:31: ACTION[Server]: Mob spawned: mobs_mc:squid at (2994,-0.5,-1686) 2021-03-07 03:17:31: ACTION[Server]: Mob spawned: mobs_mc:squid at (3037,-1.5,-1690) 2021-03-07 03:17:37: ACTION[Server]: Mob spawned: mobs_mc:baby_zombie at (3150,-12.5,-1696) 2021-03-07 03:17:41: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (3260,-41,-1827) 2021-03-07 03:17:44: ACTION[Emerge-0]: [mcl_dungeons] Placing new dungeon at (3249,-42,-1738) ``` So biome decoration - it was another problem which is luckily fixed now
kay27 reopened this issue 2021-03-07 00:21:07 +01:00
kay27 modified the milestone from 0.71.0 to 0.72.0 2021-03-07 00:21:20 +01:00
kay27 self-assigned this 2021-03-07 00:21:29 +01:00
kay27 commented 2021-03-07 00:44:47 +01:00
Contributor
Copy Link

Seems there are no probs with dungeons themselves.........
image

Seems there are no probs with dungeons themselves......... ![image](/attachments/dc6ae8dd-3211-4d09-87ef-64e6ef867157)
image.png
550 KiB
kay27 commented 2021-03-07 00:56:53 +01:00
Contributor
Copy Link

So, it seems, the problem is not in dungeons, but in place where they recalled - on_generated callback again.
Anyway, I've added /spawnstruct dungeon command, fixed rotation for some other commands - it now depends on the player rotation.
45c0c576f7

So, it seems, the problem is not in dungeons, but in place where they recalled - ```on_generated``` callback again. Anyway, I've added ```/spawnstruct dungeon``` command, fixed rotation for some other commands - it now depends on the player rotation. https://git.minetest.land/MineClone2/MineClone2/commit/45c0c576f740cafb5f31368c66f9f8bf053eb767
kay27 commented 2021-03-07 01:02:58 +01:00
Contributor
Copy Link

Well, they look pretty stable, even if bypass the check and generate them everywhere in this callback, so what's wrong with them....

image

Underground flights cause problem almost immediately though... So it can be ground_content again, checking...

Well, they look pretty stable, even if bypass the check and generate them everywhere in this callback, so what's wrong with them.... ![image](/attachments/65fcfa48-11e9-4d65-9c6a-0d7d6021f9ea) Underground flights cause problem almost immediately though... So it can be ```ground_content``` again, checking...
image.png
518 KiB
kay27 commented 2021-03-07 02:54:03 +01:00
Contributor
Copy Link

And this error again and again :(((
image

Why it grabs it, again and again, if the block has passed to callback as generated already...

And this error again and again :((( ![image](/attachments/66c6d292-84c6-44ac-b24c-9c892fea6745) Why it grabs it, again and again, if the block has passed to callback as generated already...
image.png
356 KiB
kay27 commented 2021-03-07 03:06:59 +01:00
Contributor
Copy Link

I've fixed chests and redesigned the code significally to optimize the speed
66febf158a
Unfortunatelly it doesn't help

If you'd like to help me to debug it - there is a line 381 in MAPGEN/mcl_dungeons/init.lua:

		local param = {p1=p1, p2=p2, dim=dim, pr=pr}

If you tweak it a little - you'll get really lots of dungeons everywhere without a check, next you'll be able to fly forward (/grantme all - GHJ) and catch the crash pretty fast:

		local param = {p1=p1, p2=p2, dim=dim, pr=pr, dontcheck=true}
I've fixed chests and redesigned the code significally to optimize the speed https://git.minetest.land/MineClone2/MineClone2/commit/66febf158ab764dce1c8c56c48b734d728cf2e7c Unfortunatelly it doesn't help If you'd like to help me to debug it - there is [a line 381](https://git.minetest.land/MineClone2/MineClone2/src/branch/master/mods/MAPGEN/mcl_dungeons/init.lua#L381) in ```MAPGEN/mcl_dungeons/init.lua```: ```lua local param = {p1=p1, p2=p2, dim=dim, pr=pr} ``` If you tweak it a little - you'll get really lots of dungeons everywhere without a check, next you'll be able to fly forward (/grantme all - GHJ) and catch the crash pretty fast: ```lua local param = {p1=p1, p2=p2, dim=dim, pr=pr, dontcheck=true} ```
kay27 commented 2021-03-07 03:18:26 +01:00
Contributor
Copy Link

Looks very very similar to https://github.com/minetest/minetest/issues/5900#issuecomment-306071900

Looks very very similar to https://github.com/minetest/minetest/issues/5900#issuecomment-306071900
kay27 commented 2021-03-07 03:34:50 +01:00
Contributor
Copy Link

I've tried an obvious quick fix.
I expected to get many problems.
But I've found only one - light calculation above the Nether.
So probably it worth that, if it would really help...

8ed28adf7c

image

I've tried an obvious quick fix. I expected to get many problems. But I've found only one - light calculation above the Nether. So probably it worth that, if it would really help... https://git.minetest.land/MineClone2/MineClone2/commit/8ed28adf7c750570246d8bc4cad3eab050211be0 ![image](/attachments/678705d3-4b05-4e97-a259-f5d7f0b9d6ee)
image.png
348 KiB
kay27 added the
Testing / Retest
 label 2021-03-07 03:35:08 +01:00
ryvnf commented 2021-03-07 22:35:04 +01:00
Contributor
Copy Link

@kay27 I think your way to go about this is completely wrong.

This is still obviously an engine issue, which has been pointed out by me and EliasFleckenstein. I just think it is wasteful spending time trying to create a workaround for this and to me it clearly goes against the goal for 0.72 to increase code quality.

"Fixing chests" and redesigning code for speed has nothing to do with this issue. Why do you make such fixes here when it is not related to this issue?

Making small "fixes" like this trying to workaround the engine problem is not helpful. Each time you do that increases the chance of introducing another bug and leads to worse code-quality. To be it looks like all the changes are quite random and unmotivated. You cannot rely on over 3 year old comments to issues as a reliable source of documentation about what is wrong. We have no idea if that is related to the issue. And problems with Minetest by that point have most likely been fixed already.

I think it is wrong of you to just push small changes like this to the master branch. If you want to experiment with workarounds like this I think you should do it a separate branch and every change needs to be properly motivated (not just vaguely related to what is described in a 3 year old comment).

But most of all I think it is highly problematic that you continue to work and make changes to the master branch when people do not agree with you. This issue was initially closed by EliasFleckenstein. To me it looks like he closed it because he thought that this is not something Mineclone2 should attempt to fix. Then you reopened the issue and pushed changes to the master branch. Then I said that I think that is wrong and that you should not make changes when there is an ongoing discussion and disagreement about if Mineclone2 should attempt to fix this. I also pointed out that your changes does not solve the problem (crashes would still occur for acacia trees for example). And now you do the very same thing again, making even more unmotivated changes to the master branch. I personally think this is very frustrating and that a maintainer should not act this way.

This has also been fixed in Minetest since 6 days ago. I think all the changes made for this issue should be reverted. I think it was a mistake to work on this in Mineclone2 from the beginning.

@kay27 I think your way to go about this is completely wrong. This is still obviously an engine issue, which has been pointed out by me and EliasFleckenstein. I just think it is wasteful spending time trying to create a workaround for this and to me it clearly goes against the goal for 0.72 to increase code quality. "Fixing chests" and redesigning code for speed has nothing to do with this issue. Why do you make such fixes here when it is not related to this issue? Making small "fixes" like this trying to workaround the engine problem is not helpful. Each time you do that increases the chance of introducing another bug and leads to worse code-quality. To be it looks like all the changes are quite random and unmotivated. You cannot rely on over 3 year old comments to issues as a reliable source of documentation about what is wrong. We have no idea if that is related to the issue. And problems with Minetest by that point have most likely been fixed already. I think it is wrong of you to just push small changes like this to the master branch. If you want to experiment with workarounds like this I think you should do it a separate branch and every change needs to be properly motivated (not just vaguely related to what is described in a 3 year old comment). But most of all I think it is highly problematic that you continue to work and make changes to the master branch when people do not agree with you. This issue was initially closed by EliasFleckenstein. To me it looks like he closed it because he thought that this is not something Mineclone2 should attempt to fix. Then you reopened the issue and pushed changes to the master branch. Then I said that I think that is wrong and that you should not make changes when there is an ongoing discussion and disagreement about if Mineclone2 should attempt to fix this. I also pointed out that your changes does not solve the problem (crashes would still occur for acacia trees for example). And now you do the very same thing again, making even more unmotivated changes to the master branch. I personally think this is very frustrating and that a maintainer should not act this way. This has also [been fixed in Minetest](https://github.com/minetest/minetest/pull/11011) since 6 days ago. I think all the changes made for this issue should be reverted. I think it was a mistake to work on this in Mineclone2 from the beginning.
kay27 commented 2021-03-07 23:01:22 +01:00
Contributor
Copy Link

@ryvnf well, you're making continuous remarks to me, that I'm doing things wrong. It would be better if they would be at least more specific:

  1. Speaking of wrap-arounds made by me - there is currently only one left - it's emerge_area() before making anything, and @sfan5 thinks it's correct, please take a look: https://github.com/minetest/minetest/issues/8232#issuecomment-776926152

I'd like to know what do you think about this issue https://github.com/minetest/minetest/issues/8232 specificly. Maybe some resolution terms, or what we better should to - I don't think it's good to play without schematics, or, moreover, with sliced schematics, don't you?

You think I've made more wraps, but again, it's not true! I've only reverted what I did before, and what actually caused crashes in 5.4 - tiny part of this huge commit: 89e55e9065 which, again, contained still only one wrap-around, "correct" but in fact we've seen at lease 2 crashes with it - with tree growth and with biome decorations, and I undone these things to make game compatible with 5.4. And if you think it's not right, please please write specificly what of this we should revert and especially what's wrong.

  1. All other stuff is just fixing bugs. E.g. the chests - slots for them ordered as a square, but in fact there are only 4 walls for placing them, this bug caused only one chest to be generated almost every time. I don't have time to enumerate all my bug fixes here, I would appreciate if you would read the code before posting things like that.

  2. Also I'd like to know your opinion about this issue https://github.com/minetest/minetest/issues/5900 - it what I considered here in 8ed28adf7c - why do you think it's wrong? Chunks are 5x5x5 by design. In MCL2 they processed as 7x7x7 sometimes and it caused crashes. Why should we revert it? WTF???

@ryvnf well, you're making continuous remarks to me, that I'm doing things wrong. It would be better if they would be at least more specific: 1. Speaking of wrap-arounds made by me - there is currently only one left - it's ```emerge_area()``` before making anything, and @sfan5 thinks it's correct, please take a look: https://github.com/minetest/minetest/issues/8232#issuecomment-776926152 I'd like to know what do you think about this issue https://github.com/minetest/minetest/issues/8232 specificly. Maybe some resolution terms, or what we better should to - I don't think it's good to play without schematics, or, moreover, with sliced schematics, don't you? You think I've made more wraps, but again, it's not true! I've only reverted what I did before, and what actually caused crashes in 5.4 - tiny part of this huge commit: https://git.minetest.land/MineClone2/MineClone2/commit/89e55e9065508febdc96d9c64b28c9024de7dbb3 which, again, contained still only one wrap-around, "correct" but in fact we've seen at lease 2 crashes with it - with tree growth and with biome decorations, and I undone these things to make game compatible with 5.4. And if you think it's not right, please please write specificly what of this we should revert and especially what's wrong. 2. All other stuff is just fixing bugs. E.g. the chests - slots for them ordered as a square, but in fact there are only 4 walls for placing them, this bug caused only one chest to be generated almost every time. I don't have time to enumerate all my bug fixes here, I would appreciate if you would read the code before posting things like that. 3. Also I'd like to know your opinion about this issue https://github.com/minetest/minetest/issues/5900 - it what I considered here in https://git.minetest.land/MineClone2/MineClone2/commit/8ed28adf7c750570246d8bc4cad3eab050211be0 - why do you think it's wrong? Chunks are 5x5x5 by design. In MCL2 they processed as 7x7x7 sometimes and it caused crashes. Why should we revert it? WTF???
kay27 commented 2021-03-07 23:18:35 +01:00
Contributor
Copy Link

3 year old comment

I've rechecked it, and for me "Oct 2, 2020" a little not equals "3 years old", i wonder what you mean?

@Sokomine commented on Oct 2, 2020:

Further experiments have now let to a working solution that does not cause this bug anymore because it works with vm:calc_lighting() without parameters. For that, adjustments of the height of the map (in particular lowering terrain) is now only done to the core (5x5x5) volume. In the next step, holes caused by cavegen are filled up again: If there is air or water somewhere in the shell volume where there ought to be ground level, the hole is filled up. The hole may still be a legitimate hole in the structure (fountain, well, cellar, ..) - but that is fixed later. After filling up the holes, mudflow in v6 is fixed as far as possible. This covers only 3 nodes in each direction around the core area and is thus pretty safe regarding lighting. The next step places the buildings inside the entire 7x7x7 area. If that part of the shell was generated before, any caves dugged into its buildings comming from this current mapblock will thus be fixed. If that part hasn't been generated yet, the building (usually with some air around) will stretch into the not-yet-generated mapblock like a cave does, sourrounded by ignore nodes. Plants and trees are also placed and grown in the shell area.

P. S. @ryvnf feel free to revert whatever you want, it's no prob, just please be sure in things you do, like I am :) I do apologize if I made really wrong things for you.

> 3 year old comment I've rechecked it, and for me "Oct 2, 2020" a little not equals "3 years old", i wonder what you mean? [@Sokomine commented on Oct 2, 2020](https://github.com/minetest/minetest/issues/5900#issuecomment-702846656): > Further experiments have now let to a working solution that does not cause this bug anymore because it works with vm:calc_lighting() without parameters. For that, adjustments of the height of the map (in particular lowering terrain) is now only done to the core (5x5x5) volume. In the next step, holes caused by cavegen are filled up again: If there is air or water somewhere in the shell volume where there ought to be ground level, the hole is filled up. The hole may still be a legitimate hole in the structure (fountain, well, cellar, ..) - but that is fixed later. After filling up the holes, mudflow in v6 is fixed as far as possible. This covers only 3 nodes in each direction around the core area and is thus pretty safe regarding lighting. The next step places the buildings inside the entire 7x7x7 area. If that part of the shell was generated before, any caves dugged into its buildings comming from this current mapblock will thus be fixed. If that part hasn't been generated yet, the building (usually with some air around) will stretch into the not-yet-generated mapblock like a cave does, sourrounded by ignore nodes. Plants and trees are also placed and grown in the shell area. P. S. @ryvnf feel free to revert whatever you want, it's no prob, just please be sure in things you do, like I am :) I do apologize if I made really wrong things for you.
ryvnf commented 2021-03-08 01:05:55 +01:00
Contributor
Copy Link

I think I've been quite specific about what I think you are doing wrong but here are my main points:

  1. You make multiple attempts to solve this in Mineclone2 when multiple people have pointed out that this is a memory error bug that should be solved (and soon will be solved) in the Minetest engine. You also keep pushing changes like this to the master branch when I pointed out that I disagree with that and that this should be discussed further first
  2. Your changes does not seem to be properly motivated. The primary motivation I see is references to comments in the Minetest repo and Minetest forum which might not even be the same issue and might be outdated. Overall I find your changes quite confusing. The first step for fixing issues should be to find a way to reproduce the issue so you can verify it has been fixed, otherwise you are just guessing
  3. You make changes unrelated to this issue (chests should be its own issue)

I'd like to know what do you think about this issue https://github.com/minetest/minetest/issues/8232 specificly. Maybe some resolution terms, or what we better should to - I don't think it's good to play without schematics, or, moreover, with sliced schematics, don't you?

Also I'd like to know your opinion about this issue https://github.com/minetest/minetest/issues/5900 - it what I considered here in 8ed28adf7c - why do you think it's wrong? Chunks are 5x5x5 by design. In MCL2 they processed as 7x7x7 sometimes and it caused crashes. Why should we revert it? WTF???

I can't answer these issues because I'm not familiar with them. My criticism is from the commits that have been mentioned in this issue. I don't think that is very relevant to talk about here.

I've rechecked it, and for me "Oct 2, 2020" a little not equals "3 years old", i wonder what you mean?

This comment you linked to above is about 3 years old.

I don't want to be disrespectful but I think your workflow for working on Mineclone2 is incorrect. I think you should look at how other free and open-source projects organize fixing issues. I think Minetest does it in a good and structured approach for it that works. Also I know that you have good intentions, so don't take any of this personal.

I also think there is some confusion on my part about what the purpose of your changes are. Since you write about them here I assume you are trying to solve this issue (about segmentation faults for generating structures). That also becomes very confusing.

I think I've been quite specific about what I think you are doing wrong but here are my main points: 1. You make multiple attempts to solve this in Mineclone2 when multiple people have pointed out that this is a memory error bug that should be solved (and soon will be solved) in the Minetest engine. You also keep pushing changes like this to the master branch when I pointed out that I disagree with that and that this should be discussed further first 2. Your changes does not seem to be properly motivated. The primary motivation I see is references to comments in the Minetest repo and Minetest forum which might not even be the same issue and might be outdated. Overall I find your changes quite confusing. The first step for fixing issues should be to find a way to reproduce the issue so you can verify it has been fixed, otherwise you are just guessing 3. You make changes unrelated to this issue (chests should be its own issue) > I'd like to know what do you think about this issue https://github.com/minetest/minetest/issues/8232 specificly. Maybe some resolution terms, or what we better should to - I don't think it's good to play without schematics, or, moreover, with sliced schematics, don't you? > Also I'd like to know your opinion about this issue https://github.com/minetest/minetest/issues/5900 - it what I considered here in 8ed28adf7c - why do you think it's wrong? Chunks are 5x5x5 by design. In MCL2 they processed as 7x7x7 sometimes and it caused crashes. Why should we revert it? WTF??? I can't answer these issues because I'm not familiar with them. My criticism is from the commits that have been mentioned in this issue. I don't think that is very relevant to talk about here. > I've rechecked it, and for me "Oct 2, 2020" a little not equals "3 years old", i wonder what you mean? [This comment](https://github.com/minetest/minetest/issues/5900#issuecomment-306071900) you linked to above is about 3 years old. I don't want to be disrespectful but I think your workflow for working on Mineclone2 is incorrect. I think you should look at how other free and open-source projects organize fixing issues. I think Minetest does it in a good and structured approach for it that works. Also I know that you have good intentions, so don't take any of this personal. I also think there is some confusion on my part about what the purpose of your changes are. Since you write about them here I assume you are trying to solve this issue (about segmentation faults for generating structures). That also becomes very confusing.
kay27 commented 2021-03-08 01:55:26 +01:00
Contributor
Copy Link
  1. You make multiple attempts to solve this in Mineclone2 when multiple people have pointed out that this is a memory error bug that should be solved (and soon will be solved) in the Minetest engine.

Seems you still can't realize that the bug caused by new code added by me, LOL

You also keep pushing changes like this to the master branch when I pointed out that I disagree with that and that this should be discussed further first

Like this or like what? Please mention the changes you disagree with and we'll discuss them.

  1. Your changes does not seem to be properly motivated.

The only motivation I have is a gameplay, because I have several publicly listed servers and many reports that there are no strongholds, etc.

The primary motivation I see is references to comments in the Minetest repo and Minetest forum which might not even be the same issue and might be outdated.

They are opened still!

Overall I find your changes quite confusing. The first step for fixing issues should be to find a way to reproduce the issue so you can verify it has been fixed, otherwise you are just guessing

All my issues have well-determinated way of reproducing.
E.g. the final one - here is described what to do to crash the engine, next commit fixes it and after that you can't reproduce it this way anymore.

  1. You make changes unrelated to this issue (chests should be its own issue)

If I understand correctly, you want me to open an issue for every bug I found, instead of just instant fixing it? Bad news, I won't do that, just no time for that, as well as for answering multiple non-constructive critisism, so hope it's the last time I'm doing that.

This comment you linked to above is about 3 years old.

It's just the point when discussion started.

I don't want to be disrespectful but I think your workflow for working on Mineclone2 is incorrect. I think you should look at how other free and open-source projects organize fixing issues. I think Minetest does it in a good and structured approach for it that works.

To be honest I hate the way they do that. And I don't think it's good that MCL2 inherited some features from it, like feature freeze, instead of just dedicating some LTS stable branches.

Also I know that you have good intentions, so don't take any of this personal.

If you'r thinking about it - so definitely, it's personal :) But you might take it easy - I don't have so many ambitions here... I also disagree with some low-quality nor controversal things, but first of all I accept open-source doctrine and decided for myself: if I hadn't time to make better - I let it be (especially if it works as expected)

> 1. You make multiple attempts to solve this in Mineclone2 when multiple people have pointed out that this is a memory error bug that should be solved (and soon will be solved) in the Minetest engine. Seems you still can't realize that the bug caused by new code added by me, LOL > You also keep pushing changes like this to the master branch when I pointed out that I disagree with that and that this should be discussed further first Like [this](https://git.minetest.land/MineClone2/MineClone2/commit/8ed28adf7c750570246d8bc4cad3eab050211be0) or like what? Please mention the changes you disagree with and we'll discuss them. > 2. Your changes does not seem to be properly motivated. The only motivation I have is a gameplay, because I have several publicly listed servers and many reports that there are no strongholds, etc. > The primary motivation I see is references to comments in the Minetest repo and Minetest forum which might not even be the same issue and might be outdated. They are opened still! > Overall I find your changes quite confusing. The first step for fixing issues should be to find a way to reproduce the issue so you can verify it has been fixed, otherwise you are just guessing All my issues have well-determinated way of reproducing. E.g. the final one - [here](https://git.minetest.land/MineClone2/MineClone2/issues/1212#issuecomment-16138) is described what to do to crash the engine, next commit fixes it and after that you can't reproduce it this way anymore. > 3. You make changes unrelated to this issue (chests should be its own issue) If I understand correctly, you want me to open an issue for every bug I found, instead of just instant fixing it? Bad news, I won't do that, just no time for that, as well as for answering multiple non-constructive critisism, so hope it's the last time I'm doing that. > [This comment](https://github.com/minetest/minetest/issues/5900#issuecomment-306071900) you linked to above is about 3 years old. It's just the point when discussion started. > I don't want to be disrespectful but I think your workflow for working on Mineclone2 is incorrect. I think you should look at how other free and open-source projects organize fixing issues. I think Minetest does it in a good and structured approach for it that works. To be honest I hate the way they do that. And I don't think it's good that MCL2 inherited some features from it, like feature freeze, instead of just dedicating some LTS stable branches. > Also I know that you have good intentions, so don't take any of this personal. If you'r thinking about it - so definitely, it's personal :) But you might take it easy - I don't have so many ambitions here... I also disagree with some low-quality nor controversal things, but first of all I accept open-source doctrine and decided for myself: if I hadn't time to make better - I let it be (especially if it works as expected)
kneekoo commented 2021-03-09 13:23:43 +01:00
Contributor
Copy Link

When I first got involved in an international project that had 20 national support sites (NSS), I started by offering a translation on top of the core project, on our NSS. By gaining experience, I noticed that I could change a default in the localized build for a better experience for our users. Then I found more things and it only made my list of small patches longer, and because I set some expectations with the previously added patches, the release time took slightly more effort as a result.

But because what I did made sense for all the national/localized communities, I ended up contributing my changes upstream, so that I don't have to care about them all the time, and for everyone else to benefit from them as well. The end result was something slightly easier and better for everyone using the core project localized by default.

This is why I agree with @ryvnf on how we're supposed to handle engine issues - for the most part anyway. Minetest development and the way certain issues are treated by the developers might not be fully aligned with my ways either, but I'd rather lean on the side of "we can wait", instead of finding workarounds for engine issues. Many other games and mods benefit from having engine bugs fixed upstream compared to everyone trying to write their own workarounds.

Now in the case of old engine bugs, there are two main ways of solving the problem:

  1. everyones spending time and effort to produce workarounds in their (sub-)projects
  2. focusing on fixing the engine issue, to make (sub-)projects easier to maintain

Granted, option 2 is not always possible, but in our case there was no sense of urgency to push for changes in MC2 while the issue was being actively investigated by the Minetest developers.

I only have a friend on my MC2 server, and we'd like to have the latest and greatest features. But ultimately, we both want stability and performance more than new features. So right now we both wait for 5.4.0 to be officially packaged for Debian, so we can move to 0.71 with all its new features (including the 3D player preview). And once that happens, we'll wait again for the official 0.72 release because most of the time the master brach is unstable and it's not fun to have bugs and crashes in a world that you spend tens and hundreds of hours to build stuff, only to have something weird happen and being forced to either make use of backups, or rollbacks, which would also revert the normal work and progress of some players.

Being on the bleeding edge has clear advantages, but also painful disadvantages. So while it's exciting to be able to test the latest and greatest, the "production" worlds should either have players willing to bite the bullet if anything goes wrong, or they should work reliably until a new stable release is out.

Not many people are willing to do extensive testing, so for the relatively small number of those interested in this, we can have separate branches that have specific and isolated fixes that can be reliably tested, while the master would stay pretty clean and stable for casual testers. There are obvious benefits in having this kind of a workflow and it would be nice to see it happen.

When I first got involved in an international project that had 20 national support sites (NSS), I started by offering a translation on top of the core project, on our NSS. By gaining experience, I noticed that I could change a default in the localized build for a better experience for our users. Then I found more things and it only made my list of small patches longer, and because I set some expectations with the previously added patches, the release time took slightly more effort as a result. But because what I did made sense for all the national/localized communities, I ended up contributing my changes upstream, so that I don't have to care about them all the time, and for everyone else to benefit from them as well. The end result was something slightly easier and better for everyone using the core project localized by default. This is why I agree with @ryvnf on how we're supposed to handle engine issues - for the most part anyway. Minetest development and the way certain issues are treated by the developers might not be fully aligned with my ways either, but I'd rather lean on the side of "we can wait", instead of finding workarounds for engine issues. Many other games and mods benefit from having engine bugs fixed upstream compared to everyone trying to write their own workarounds. Now in the case of old engine bugs, there are two main ways of solving the problem: 1. everyones spending time and effort to produce workarounds in their (sub-)projects 2. focusing on fixing the engine issue, to make (sub-)projects easier to maintain Granted, option 2 is not always possible, but in our case there was no sense of urgency to push for changes in MC2 while the issue was being actively investigated by the Minetest developers. I only have a friend on my MC2 server, and we'd like to have the latest and greatest features. But ultimately, we both want stability and performance more than new features. So right now we both wait for 5.4.0 to be officially packaged for Debian, so we can move to 0.71 with all its new features (including the 3D player preview). And once that happens, we'll wait again for the official 0.72 release because most of the time the master brach is unstable and it's not fun to have bugs and crashes in a world that you spend tens and hundreds of hours to build stuff, only to have something weird happen and being forced to either make use of backups, or rollbacks, which would also revert the normal work and progress of some players. Being on the bleeding edge has clear advantages, but also painful disadvantages. So while it's exciting to be able to test the latest and greatest, the "production" worlds should either have players willing to bite the bullet if anything goes wrong, or they should work reliably until a new stable release is out. Not many people are willing to do extensive testing, so for the relatively small number of those interested in this, we can have separate branches that have specific and isolated fixes that can be reliably tested, while the master would stay pretty clean and stable for casual testers. There are obvious benefits in having this kind of a workflow and it would be nice to see it happen.
👍 1
LizzyFleckenstein03 commented 2021-03-09 13:59:16 +01:00
Member
Copy Link

right now we both wait for 5.4.0 to be officially packaged for Debian

https://launchpad.net/~minetestdevs/+archive/ubuntu/stable
I know this is for ubuntu but afaik it should work for debian as well.

> right now we both wait for 5.4.0 to be officially packaged for Debian https://launchpad.net/~minetestdevs/+archive/ubuntu/stable I know this is for ubuntu but afaik it should work for debian as well.
kneekoo commented 2021-03-09 14:03:48 +01:00
Contributor
Copy Link

I can wait, so I won't FrankenDebianize my Raspberry Pi OS with Ubuntu packages. :P Besides, what I need there is an updated minetest-server package, not the client.

I can wait, so I won't [FrankenDebianize](https://wiki.debian.org/DontBreakDebian) my Raspberry Pi OS with Ubuntu packages. :P Besides, what I need there is an updated **minetest-server** package, not the client.
LizzyFleckenstein03 commented 2021-03-09 14:06:50 +01:00
Member
Copy Link

I think your only option then is to use snap. Idk why, but apt seems to always lack 1 release behind. And remember, 5.3 servers are fully supported - you can also enable 3D player preview on such a server, and 5.4 clients will display it properly.

I think your only option then is to use snap. Idk why, but apt seems to always lack 1 release behind. And remember, 5.3 servers are fully supported - you can also enable 3D player preview on such a server, and 5.4 clients will display it properly.
kneekoo commented 2021-03-09 14:12:22 +01:00
Contributor
Copy Link

Nice, so only the client cares about the preview if it's enables. Then I'll move to 0.71. Thanks! :)

Nice, so only the client cares about the preview if it's enables. Then I'll move to 0.71. Thanks! :)
ryvnf commented 2021-03-09 16:05:05 +01:00
Contributor
Copy Link

I apologize to @kay27 if I was a bit harsh in expressing my criticism.

I think @kneekoo expressed my thoughts well. The problem is that changes were made directly to the master branch when there was no need to rush them. For a change to be made directly to master I think these three conditions need to be satisfied:

  1. The changes should not be controversial. Any change which might have opposing view-points among the developers needs to be discussed first.
  2. The changes should not lower the code quality.
  3. We should have high confidence that the changes do not introduce new bugs.

In this case I think all of these points were lacking. I think it was obvious more discussion was necessary. I also found the changes to be confusing and leading to worse code quality. They also introduced new bugs (like broken lighting on nether roof).

I apologize to @kay27 if I was a bit harsh in expressing my criticism. I think @kneekoo expressed my thoughts well. The problem is that changes were made directly to the master branch when there was no need to rush them. For a change to be made directly to master I think these three conditions need to be satisfied: 1. The changes should not be controversial. Any change which might have opposing view-points among the developers needs to be discussed first. 2. The changes should not lower the code quality. 3. We should have high confidence that the changes do not introduce new bugs. In this case I think all of these points were lacking. I think it was obvious more discussion was necessary. I also found the changes to be confusing and leading to worse code quality. They also introduced new bugs (like broken lighting on nether roof).
kay27 commented 2021-03-10 05:11:25 +01:00
Contributor
Copy Link

In this case I think all of these points were lacking.

But when I ask you: what part of the code lacks, you answer: well, I’m not familiar with it.

So why you’re doing THIS instead of trying to become more familiar, to discuss it specifically?

like broken lighting on nether roof

So, would you mind telling me the commit number where it hadn’t been broken? :) Please please!

I can’t get nothing useful from this discussion but wasting the time. Of course, for me, all your ‘rules of committing to master’ abide by, I approve that, despite the fact that currently I’m against using these rules strictly. For me it looks kinda: I don’t like what you do, I’m lazy to discuss it specifically, I better tell everyone you don’t follow my rules. Maybe I’m wrong but more likely I’m right, because you are free to fix the light, or point me to the error if you think I made one, but you do what you do still.

> In this case I think all of these points were lacking. But when I ask you: what part of the code lacks, you answer: well, I’m not familiar with it. So why you’re doing THIS instead of trying to become more familiar, to discuss it specifically? > like broken lighting on nether roof So, would you mind telling me the commit number where it hadn’t been broken? :) Please please! I can’t get nothing useful from this discussion but wasting the time. Of course, for me, all your ‘rules of committing to master’ abide by, I approve that, despite the fact that currently I’m against using these rules strictly. For me it looks kinda: I don’t like what you do, I’m lazy to discuss it specifically, I better tell everyone you don’t follow my rules. Maybe I’m wrong but more likely I’m right, because you are free to fix the light, or point me to the error if you think I made one, but you do what you do still.
kay27 commented 2021-03-10 08:26:38 +01:00
Contributor
Copy Link

@kneekoo

Many other games and mods benefit from having engine bugs fixed upstream compared to everyone trying to write their own workarounds.

Reasonable. But I've already tried to add really tiny PR to Minetest API and see what I've got: https://github.com/minetest/minetest/pull/10430

And see what got @Wuzzy after 2 years of waiting: https://github.com/minetest/minetest/issues/6981

Etc.

They don't have time, don't have energy but always do have different opinion :)

So for me currently mostly the engine is a something just given to us from above.

@kneekoo > Many other games and mods benefit from having engine bugs fixed upstream compared to everyone trying to write their own workarounds. Reasonable. But I've already tried to add really tiny PR to Minetest API and see what I've got: https://github.com/minetest/minetest/pull/10430 And see what got @Wuzzy after 2 years of waiting: https://github.com/minetest/minetest/issues/6981 Etc. They don't have time, don't have energy but always do have different opinion :) So for me currently mostly the engine is a something just given to us from above.
kneekoo commented 2021-03-10 09:36:17 +01:00
Contributor
Copy Link

It looks like Wuzzy gave up on that - maybe because it took so long and not enough dev support went into that. But what paramat said there, as blunt and dry as it looks like, makes sense when you put yourself in their shoes. They have their own many issues and priorities to cover, so it's not surprising that they focus on them, instead of other people's stuff, because everyone's time and energy is limited.

Approaching them can certainly be difficult, especially when people are unaware of what their plans are, so it always helps when the suggestions come from someone who could potentially implement those things themselves in the engine, in a way that doesn't bring more issues of course. Or at least think it all the way through, to make it easier for the MT devs to evaluate the idea and implement it themselves.

And yes, opinions take very little effort. :P Thinking about the impact of hundreds of callbacks and whatever triggers them on both loaded and unloaded blocks... that's more of a problem. To be fair, it makes sense that on a multiplayer server people would make big farms with plenty of observers, so "hundreds of callbacks" looks likely to happen on servers with 50+ active players. But this is something we could discuss in its own issue.

It looks like Wuzzy gave up on that - maybe because it took so long and not enough dev support went into that. But what paramat said there, as blunt and dry as it looks like, makes sense when you put yourself in their shoes. They have their own many issues and priorities to cover, so it's not surprising that they focus on them, instead of other people's stuff, because everyone's time and energy is limited. Approaching them can certainly be difficult, especially when people are unaware of what their plans are, so it always helps when the suggestions come from someone who could potentially implement those things themselves in the engine, in a way that doesn't bring more issues of course. Or at least think it all the way through, to make it easier for the MT devs to evaluate the idea and implement it themselves. And yes, opinions take very little effort. :P Thinking about the impact of hundreds of callbacks and whatever triggers them on both loaded and unloaded blocks... that's more of a problem. To be fair, it makes sense that on a multiplayer server people would make big farms with plenty of observers, so "hundreds of callbacks" looks likely to happen on servers with 50+ active players. But this is something we could discuss in its own issue.
kay27 commented 2021-03-10 10:10:24 +01:00
Contributor
Copy Link

But I think Wuzzy gave up because he also had preferred to

lean on the side of "we can wait", instead of finding workarounds for engine issues.

It just means no progress. Maybe we should say thanks for that time they didn't say that we really hate ourselves, like it happens sometimes...

And maybe it's the case when the compromise makes the progress.

But I think Wuzzy gave up because he also had preferred to > lean on the side of "we can wait", instead of finding workarounds for engine issues. It just means no progress. Maybe we should say thanks for that time they didn't say that we really hate ourselves, [like it happens sometimes](https://github.com/minetest/minetest/issues/10588#issuecomment-733796318)... And maybe it's the case when the compromise makes the progress.
kneekoo commented 2021-03-10 10:21:10 +01:00
Contributor
Copy Link

:)) hecktest overreacted "a bit" back then, so we don't need to paint everyone with the same color just because of a few replies like that.

Yes, compromises are important, and sometimes it can pay off to wait - especially when we have a lot of other things that also require development time. Prioritizing other issues is a compromise in itself.

:)) hecktest overreacted "a bit" back then, so we don't need to paint everyone with the same color just because of a few replies like that. Yes, compromises are important, and sometimes it can pay off to wait - especially when we have a lot of other things that also require development time. Prioritizing other issues is a compromise in itself.
kay27 commented 2021-03-10 10:52:52 +01:00
Contributor
Copy Link

Yes, compromises are important, and sometimes it can pay off to wait - especially when we have a lot of other things that also require development time. Prioritizing other issues is a compromise in itself.

Well, this issue first time was originally named "Release 0.71.0 "animated chests"?". Chests had visual glitches but worked like a charm, so I thought why not.

Booom! We're releasing the villages... WHAT?? This decision 100% wasn't mine. Having this and this engine issues opened it wasn't possible for myself, and I did my best to at least stabilize it. it was really hard and probably doesn't worth it (1) but it had been stabilized.

Booom! Minetest 5.4 released and at least two parts of my fix (1) and one part of old MCL2 code causes many problems (this issue is mostly about them), so I reverted them ASAP to stabilize it back.

Now it seems to be stabilized.

Was it wrong? Heh, planning to release villages was wrong if so...

> Yes, compromises are important, and sometimes it can pay off to wait - especially when we have a lot of other things that also require development time. Prioritizing other issues is a compromise in itself. Well, [this issue](https://git.minetest.land/MineClone2/MineClone2/issues/997) first time was originally named "Release 0.71.0 "animated chests"?". Chests had visual glitches but worked like a charm, so I thought why not. Booom! We're releasing the villages... WHAT?? This decision 100% wasn't mine. Having [this](https://github.com/minetest/minetest/issues/5900) and [this](https://github.com/minetest/minetest/issues/8232) engine issues opened it wasn't possible for myself, and I did my best to at least stabilize it. [it was really hard and probably doesn't worth it](https://git.minetest.land/MineClone2/MineClone2/commit/89e55e9065508febdc96d9c64b28c9024de7dbb3) (1) but it had been stabilized. Booom! Minetest 5.4 released and at least two parts of my fix (1) and one part of old MCL2 code causes many problems (this issue is mostly about them), so I reverted them ASAP to stabilize it back. Now it seems to be stabilized. Was it wrong? Heh, planning to release villages was wrong if so...
kneekoo commented 2021-03-10 13:18:40 +01:00
Contributor
Copy Link

The original title implied an update focused on animated chests. You probably agree that would be strange from a user's perspective. I don't know what prompted the village update, but in many ways it's a great step forward for the project.
And we still need fixes, but it's more functional than anyone could've hoped this soon, so I can't emphasize how grateful I am for everyone's work on this update. Some of the work was done but hidden, some had to be done, and the result is great. Remember we're still in alpha, so it's fine if it's not perfect.

The original title implied an update focused on animated chests. You probably agree that would be strange from a user's perspective. I don't know what prompted the village update, but in many ways it's a great step forward for the project. And we still need fixes, but it's more functional than anyone could've hoped this soon, so I can't emphasize how grateful I am for everyone's work on this update. Some of the work was done but hidden, some had to be done, and the result is great. Remember we're still in alpha, so it's fine if it's not perfect.
kay27 commented 2021-03-10 14:49:08 +01:00
Contributor
Copy Link

The original title implied an update focused on animated chests. You probably agree that would be strange from a user's perspective.

Sure, if I wouldn't seen kids testing chests from release to release, sighing heavily. It's hard to overscore the chests in Minecraft.

> The original title implied an update focused on animated chests. You probably agree that would be strange from a user's perspective. Sure, if I wouldn't seen kids testing chests from release to release, sighing heavily. It's hard to overscore the chests in Minecraft.
kneekoo commented 2021-03-10 15:47:34 +01:00
Contributor
Copy Link

Kids aren't the only ones fond of the new chests. Both me and my friend opened and closed chests just for fun yesterday. :P But then he noticed other updated stuff and liked it, and them something else. It makes sense to have more new things in an update than just an item. After all, this game is not just for kids. Adults enjoy it too. :P If I wouldn't like it, I wouldn't have spent so many hours testing it. :)

Kids aren't the only ones fond of the new chests. Both me and my friend opened and closed chests just for fun yesterday. :P But then he noticed other updated stuff and liked it, and them something else. It makes sense to have more new things in an update than just an item. After all, this game is not just for kids. Adults enjoy it too. :P If I wouldn't like it, I wouldn't have spent so many hours testing it. :)
ryvnf commented 2021-03-21 12:48:16 +01:00
Contributor
Copy Link

Can we close this issue? The engine fix has been merged into Minetest now.

https://github.com/minetest/minetest/pull/11011

Can we close this issue? The engine fix has been merged into Minetest now. https://github.com/minetest/minetest/pull/11011
kay27 commented 2021-03-21 23:51:45 +01:00
Contributor
Copy Link

Yes, thanks for the reminding.

Yes, thanks for the reminding.
kay27 closed this issue 2021-03-21 23:51:48 +01:00
kay27 added the
ground content conflict
 label 2021-03-26 23:59:54 +01:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
master
hammers_spears
projectile-refactor
cart-chest-loot
axolotl-attack
burning_in_thunder
shieldy_shields
optimize-lightning
limit-fire-spread
fake-player-hardening
add-dynamic-rules
environment-physics
smithing_table_patch
bamboo_cap_fix
hollow_logs_improvements
compressed-blocks
mapgen-test-branch
chests_api_expand
improve_planks
tree_growth_buildable_to
fix_bamboo
head-swivel
buildable_flowers
upstream/rewritable_maps
textureconverter-improvement
mapgen_update
pickblock
mob_ignore_fix
mcl_mobs_cleanup
fix-kelp
upstream/unbreaking_on_crossbows
slabs_block_light
redstone-update
add_shader_support
EE_armor
world_aligned_textures
zombie_texture_improvements
mcl_weathered_nodes
orb_of_exchange
patch_skins
hollow_logs_ref_recovery
copper-adaptation-refs-recovery
add_asset_attributions_1
fix_pumpkin_typo_pumkin
cherry_grove
lush_caves
release/0.86.1
mobile_bow_workaround
release/0.85.1
optimize_jukebox_sounds
optimize_theme_ogg
Optimize_backgrounds_jpg
archaeology
spanish-translations
fish_bucket_nametag
villager-golem-fixes
fix_beetroot_fortune_drops
fix_sliding
illager_banner_description
release_0.83.1
skelly_horse_thunder
mobs_armor
mobsarmorfix
smoother_biome_borders
jukebox_sound_distance
cobble_abm_height
callable-tool-rightclick-functions
effects_rewrite
respiration
make-new-music-louder
damage
texture_conversion_wool
dehardcoded_signs
mcl_oxidization
Texture-Rename2
Bamboo-Piston-Dig
hardcore_mode_test
update_german_translation_attempt2
test_mob_tweaks_branch
csm-enabled
move_physics_tweak
water_footstep
biome_skycolor
bambootoo
lower_ow_mapgen
release/0.81.1
revert_chest_models
mcl-craftguide-drop-sfinv
mcl-structures-fixes
turtlestriderbeefox
arrow_node_collision
pvp_knockback
icebergs
light_dump
testing
mobs-redo
wood_api
mobs_head_rotation
bonemeal
dyable-leather-armor
new_mapgen_api
newmobs-master
formspec_overhaul
mobs
mapgen
mineclone5
chat-command-builder-2
formspec-v4
doc-refactoring
production
objects
0.87.2
0.87.1
0.87.0
0.86.2
0.86.1
0.86.0
0.85.0
0.84.0
0.83.1
0.83.0
0.82.0
0.81.1
0.81.0
0.80.1
0.80.0
0.79.0
0.78.0
0.77.0
0.76.0
0.75.0
0.74.0
0.73.1
0.73.0
0.72.1
0.72.0
0.71.0
0.70.0
0.69.1
0.69.0
0.68.0
0.67.2
0.67.1
0.67.0
0.66.2
0.66.1
0.66.0
0.65.2
0.65.1
0.65.0
0.64.0
0.63.0
0.62.0
0.61.0
0.60.0
0.59.2
0.59.1
0.59.0
0.58.1
0.58.0
0.57.1
0.57.0
0.56.2
0.56.1
0.56.0
0.55.1
0.55.0
0.54.1
0.54.0
0.53.4
0.53.3
0.53.2
0.53.1
0.53.0
0.52.1
0.52.0
0.51.1
0.51.0
0.50.2
0.50.1
0.50.0
0.49.0
0.48.1
0.48.0
0.47.1
0.47.0
0.46.0
0.45.1
0.45.0
0.44.0
0.43.0
0.42.0
0.21.0
Labels
Clear labels   
#P1 CRITICAL

for crashes, catastrophic bugs, etc.

  
#P2: HIGH

highest priority level

  
#P3: elevated

2nd-highest priority level

  
#P4 priority: medium

for things that aren't elevated but have been prioritised

  
#P6: low

low priority level

  
#Review

Tickets that may improve or fix an issue but will need a review after a short period of time

  
annoying

non-bugs that annoys people

  
API

   
bug

   
code quality

   
combat

   
commands

   
compatibility

Label to cover migration issues from old worlds or mcl5

  
configurability

   
contribution inside

   
controls

   
core feature

   
creative mode

   
delayed for engine release

   
documentation

   
duplicate

   
enhancement

enhancement request/suggestion

  
environment

Issues that affect weather or light

  
feature request

   
gameplay

   
graphics

   
ground content conflict

mapgen gc-specific quirks

  
GUI/HUD

   
help wanted

   
incomplete feature

   
invalid / won't fix

Invalid, or will not fix

  
items

   
looking for contributor

   
mapgen

   
meta

Relating to VoxeLibre on Mesehub and elsewhere

  
mineclone2+

fit for MineClone2-Plus

  
Minecraft >= 1.13

   
Minecraft >= 1.17

   
missing feature

   
mobile

involving Minetest on mobile devices (android)

  
mobs

   
mod support

Issues related to mod support

  
model needed

A model is needed to be created or updated

  
multiplayer

Issues affecting multiplayer

  
Needs adoption

PR needs author action or to be adopted so changes can be implemented to get the PR closer to merge

  
needs discussion

   
needs engine change

   
needs more information

   
needs research

   
nodes

   
non-mob entities

   
performance

   
player

issues related to the player

  
possible close

   
redstone

   
release notes

This ticket will need information adding to release notes

  
schematics

   
Skyblock

   
sounds

   
Testing / Retest

This needs testing or retesting

  
tools

Related to the tools scripts, which aren't run in game

  
translation

   
unconfirmed
  
mcl5

Possibly relevant for MineClone5

  
mcla

Possibly relevant for Mineclonia

  
Media missing

media files for this feature are missing - contributions welcome

No Label
#P1 CRITICAL
#P2: HIGH
#P3: elevated
#P4 priority: medium
#P6: low
#Review
annoying
API
bug
code quality
combat
commands
compatibility
configurability
contribution inside
controls
core feature
creative mode
delayed for engine release
documentation
duplicate
enhancement
environment
feature request
gameplay
graphics
ground content conflict
GUI/HUD
help wanted
incomplete feature
invalid / won't fix
items
looking for contributor
mapgen
meta
mineclone2+
Minecraft >= 1.13
Minecraft >= 1.17
missing feature
mobile
mobs
mod support
model needed
multiplayer
Needs adoption
needs discussion
needs engine change
needs more information
needs research
nodes
non-mob entities
performance
player
possible close
redstone
release notes
schematics
Skyblock
sounds
Testing / Retest
tools
translation
unconfirmed
mcl5
mcla
Media missing
Milestone
Clear milestone
No items
No Milestone
0.72.0
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
6 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: VoxeLibre/VoxeLibre#1212
No description provided.
Delete Branch "%!s(<nil>)"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?