Undo/Redo errors happening on terrains of size not multiple of 16

[suptoasty]

Hi, @Zylann . I was testing your plug-in and found that when I hit the Undo Shortcut keys too many times, that the Terrain node (which was the root node of the scene) was deleted, but failed to free the actual node from the scene view. I don't know if the node being freed by the Undo command is a flaw in the new plug-in system or not, so I thought I would post here to notify you before going to the Godot git page. And I would recommend checks using weakref or something to fix the Nil instance errors, if possible.

Linux Mint 18, Godot Engine 2.1

Steps to reproduce:

1. Start A New Scene
2. Make A Terrain Node The Root
3. Possible Set A Size For The Node
4. Hit The Undo Command A Few Times

Here is a screenshot of what happend.

PS: Love the plug-in so far, it is fast and easy to use. I love that It gives more control to developers than the old method of sketching a map in 'your preferred imaging tools here' and makes the process fun. Can't wait for this to mature into something along the lines of world machine or unity's terrain system (if you are willing to take it that far).

[Zylann]

I cannot reproduce this on Windows 10 64 bits :(

[suptoasty]

Do you think it could be a platform specific bug in the editor, that should be reported?

[Zylann]

Possibly, are you using the latest Godot?

Also the node is freed because Undo is undoing the fact you added it, this also happens on my side. It's expected behaviour for any node. However, I wonder how the error happens... It says Nil instance, but as the log shows, the plugin tries to undo a Paint operation, so the terrain data shouldn't be null at this point.

The only "special" thing this node does is to create a bunch of MeshInstances under it (chunks), but for some reason they aren't shown in the editor. But everything worked like a charm so far. There must be something, a state, idk, that breaks Undo/redo at some point.

[suptoasty]

I am using the latest stable build. I now realize, the editor now allows for removal of root nodes, unlike previous builds I have used. I always would get an error message when trying to delete them in 1.1 and 2.0. I assumed this build would retain this caveat.

As long as everything is working for you as expected, there probably is no real major problem here besides the Nil errors preventing the complete removal of the Terrain node. I'll keep messing around and see if I can track down the cause of the errors.

[Zylann]

I tested on a blank project with the assetlib version, there is indeed something fishy. I get no errors, but undo doesn't seems to complete. For example, if I create a terrain, set its size to 32, then undo, I can see nothing actually happens. Size stays at 32. I believe undoing property changes were supposed to be handled by the editor... If I continue editing, some things don't get fully undone, for example I can paint, undo, but mesh height is still there (while normals are reverted correctly Oo). It looks like errors happen and interrupt the flow, but they aren't logged at all D:

[suptoasty]

I just tried to repeat the process and I also obtained no log messages, other than the common PaintTerrain, Undo, and Redo. The node yet again failed to remove its self. And when trying to use the Redo Command to recreate the node, it recreates all changes previously applied, but the node icon often (not always) fails to be shown in the scene editor. This leads me to believe part of the problem is with the new plug-in system.

I'll check the Godot Issues for similar happenstances.

[Zylann]

Ok I found why undoing until terrain creation did weird things: I left a check preventing to set a null size. And the default size is... zero. But it's not really needed tbh, so I removed it and now the result is consistent. Still not sure if it relates to the errors you got.

The fix must be done here: https://github.com/Zylann/godot_terrain_plugin/blob/master/addons/zylann/terrain/terrain.gd#L55 new_size > 1 must go away. Feel free to try if it fixes the errors g :) EDIT: pushed the fix.

I also found another Godot bug while doing that: https://github.com/godotengine/godot/issues/6125 but it doesn't seems to be related.

[suptoasty]

I have not come across any errors again. Hopefully, the errors I got were some freak accident. I have gotten errors in things like the ogre3d rendering engine, that disappeared after another run of the project I was using it for, so fingers crossed :four_leaf_clover: .

[Zylann]

Ok I managed to reproduce errors, but only after Redo operations. The first error being:

 res://addons/zylann/terrain/terrain_utils.gd:129 - Invalid get index '31' (on base: 'Array').

and the last:

 res://addons/zylann/terrain/terrain_utils.gd:134 - Invalid call. Nonexistent function 'size' in base 'Nil'.

I found out it happens only if the terrain size is not a multiple of the chunk size (16). So at the moment, using multiples of 16 would be a workaround. I'll have a look later, it's 2 o clock here :D

[suptoasty]

Oh man, I could have helped get to this point faster. I forgot my terrain size was an odd number of 25 to fit the 3d editor plane's 4th quadrant. My apologies, for this slip of mind.

[suptoasty]

Please excuse the closing. Slip of the mouse and slip of the mind in one day. I am on a losing streak or something. lol

[Zylann]

I updated the asset on the assetlib, you can try if you still get the errors.