Editor crash on torus maps

[geoo89]

I managed to get two crashes on the same map (which was newly created in 0.10.18). The map was a torus map with tile groups and black terrain, unfortunately I don't know the exact steps for reproduction. Logs below.

Lix version:  0.10.18
Session date: 2024-01-27 09:23:42
  3770.98 src/editor/undoable/move.d:40:
  3770.98 Apply: Expected [0].loc == source of (394,128) to then move it to destination (10,131), but found [0].loc == (10,128). The first element must match without Topology.wrap!
  3771.00 ??:? _d_assert_msg [0x55d00067f68c]
src/editor/undoable/move.d:44 const void editor.undoable.move.TileMove.apply(level.level.Level) [0x55d00051d46b]
src/editor/stack.d:33 const(std.container.rbtree.RedBlackTree!(level.oil.oil.Oil, "a.appearsBefore(b)", false).RedBlackTree) editor.stack.UndoRedoStack.apply(level.level.Level, editor.undoable.base.Undoable) [0x55d00051af9d]
src/editor/stack.d:94 const(std.container.rbtree.RedBlackTree!(level.oil.oil.Oil, "a.appearsBefore(b)", false).RedBlackTree) editor.stack.UndoRedoStackThatMergesTileMoves.apply(level.level.Level, editor.undoable.move.TileMove) [0x55d00051b437]
src/editor/editor.d:136 void editor.editor.Editor.applyAndTrustThatTheSelectionWillNotChange!(editor.undoable.move.TileMove).applyAndTrustThatTheSelectionWillNotChange(editor.undoable.move.TileMove) [0x55d0004fdb82]
src/editor/movetile.d:31 void editor.movetile.moveTiles(editor.editor.Editor) [0x55d000515ade]
src/editor/calc.d:66 void editor.calc.implEditorCalc(editor.editor.Editor) [0x55d0004f9aa0]
src/editor/editor.d:94 void editor.editor.Editor.calc() [0x55d0004fd7a4]
src/gui/root.d:97 void gui.root.calc() [0x55d0005c75e8]
src/mainloop/mainloop.d:97 bool mainloop.mainloop.MainLoop.calc_returnsTrueIfWeShouldExitApp() [0x55d0005ddda6]
src/mainloop/mainloop.d:48 void mainloop.mainloop.MainLoop.mainLoop() [0x55d0005ddbc5]
src/main.d:38 int main.main(immutable(char)[][]).__lambda3() [0x55d0005ddb1c]
~/.dub/packages/allegro-4.0.6_5.2.0/allegro/allegro5/system.d:24 extern (C) int allegro5.system.al_run_allegro(scope int delegate()).main_runner(int, char**) [0x55d00063e9b6]
~/.dub/packages/allegro-4.0.6_5.2.0/allegro/allegro5/system.d:36 int allegro5.system.al_run_allegro(scope int delegate()) [0x55d00063e991]
src/main.d:35 _Dmain [0x55d0005ddaaf]

Lix version:  0.10.18
Session date: 2024-01-27 09:24:44
   228.02 src/editor/undoable/addrm.d:108:
   228.02 removeTheOcc: Expected  at (400,52)  to then remove it, but instead found  at (16,52) . Inconsistent history. These occs should match without Topology.wrap.
   228.03 ??:? _d_assert_msg [0x563f3555d68c]
src/editor/undoable/addrm.d:111 const void editor.undoable.addrm.TileAdditionOrRemoval.removeTheOcc(level.level.Level) [0x563f353f9873]
src/editor/undoable/addrm.d:36 const void editor.undoable.addrm.TileInsertion.undo(level.level.Level) [0x563f353f958c]
src/editor/undoable/compound.d:44 const void editor.undoable.compound.CompoundUndoable.undo(level.level.Level) [0x563f353fa4de]
src/editor/stack.d:44 const(std.container.rbtree.RedBlackTree!(level.oil.oil.Oil, "a.appearsBefore(b)", false).RedBlackTree) editor.stack.UndoRedoStack.undoOne(level.level.Level) [0x563f353f90f5]
src/editor/stack.d:113 const(std.container.rbtree.RedBlackTree!(level.oil.oil.Oil, "a.appearsBefore(b)", false).RedBlackTree) editor.stack.UndoRedoStackThatMergesTileMoves.undoOne(level.level.Level) [0x563f353f94ba]
src/editor/editor.d:139 void editor.editor.Editor.undoOne() [0x563f353db902]
src/editor/paninit.d:73 void editor.paninit.makePanel(editor.editor.Editor).__lambda9() [0x563f353f4e38]
src/gui/button/button.d:109 void gui.button.button.Button.calcSelf() [0x563f3548a420]
src/gui/element.d:147 void gui.element.Element.calc() [0x563f354914d7]
src/gui/element.d:146 void gui.element.Element.calc().__lambda1!(gui.element.Element).__lambda1(gui.element.Element) [0x563f354919f8]
/usr/include/dmd/phobos/std/algorithm/iteration.d:983 std.typecons.Flag!("each").Flag std.algorithm.iteration.each!(gui.element.Element.calc().__lambda1).each!(gui.element.Element[]).each(ref gui.element.Element[]) [0x563f35491a50]
src/gui/element.d:146 void gui.element.Element.calc() [0x563f354914c9]
src/editor/calc.d:60 void editor.calc.implEditorCalc(editor.editor.Editor) [0x563f353d7a68]
src/editor/editor.d:94 void editor.editor.Editor.calc() [0x563f353db7a4]
src/gui/root.d:97 void gui.root.calc() [0x563f354a55e8]
src/mainloop/mainloop.d:97 bool mainloop.mainloop.MainLoop.calc_returnsTrueIfWeShouldExitApp() [0x563f354bbda6]
src/mainloop/mainloop.d:48 void mainloop.mainloop.MainLoop.mainLoop() [0x563f354bbbc5]
src/main.d:38 int main.main(immutable(char)[][]).__lambda3() [0x563f354bbb1c]~.dub/packages/allegro-4.0.6_5.2.0/allegro/allegro5/system.d:24 extern (C) int allegro5.system.al_run_allegro(scope int delegate()).main_runner(int, char**) [0x563f3551c9b6]
~/.dub/packages/allegro-4.0.6_5.2.0/allegro/allegro5/system.d:36 int allegro5.system.al_run_allegro(scope int delegate()) [0x563f3551c991]
src/main.d:35 _Dmain [0x563f354bbaaf]

[SimonN]

Thanks for the report and the logs.

I should investigate what happens when you group lots of black terrain, then move it, then ungroup it. Does the ungrouping put tiles at wrongly wrapped coordinates?

All editor operations are independently responsible for wrapping coordinates. This responsibility is not centralized in the level or the tiles themselves, which would prevent the bug cleanly but would go heavily against the stroke of the level data structures.

[SimonN]

Here is an example with grouping and ungrouping that leads to wrong torus invariants. Repro below.

Definition (torus invariant): When a dimension is wrapped (the x-dimension, the y-dimension, or both x and y for a torus map), all tiles on that map must have coordinates in [0, L[ for that dimension, where L is the map size in that dimension. E.g., Hopscotch has size 256x160 and wraps only the y-dimension. On Hopscotch, each tile must have a y-coordinate of 0, 1, 2, ..., 157, 158, or 159. It's not allowed to have a y coordinates of −16, 160, or 500.

The purpose of the invariant is to enforce a unique representation of each tile.

This example wont doesn't necessarily lead to your crash, but it shows invariant-violating coordinates. I'll investigate it and fix the invariant preservation.

This example has only vertical wrapping, but horizontal wrapping should violate the invariant too.

Repro:

1. Put a black terrain tile across the torus seam, overlapping the seam.
2. Put a regular terrain tile under the torus seam, not overlapping the seam, but overlapping the black tile.
3. Group the two tiles.
4. Move the group around.
5. Move it back to where it came from.
6. Ungroup the two tiles.

After 2, the invariant still holds. After 3, the invariant is violated: The tile group has a y coordinate that is larger than the y-length of the map. After 4, the invariant holds again. After 5, it still holds. After 6, the invariant is violated again: The black tile has a negative y coordinate.

[SimonN]

In unpublished code, I've added wrap to the grouping (computation and creation of the group, not the resulting Undoable subclass that removes the source tiles and inserts the group) and the ungrouping visitor (again, not the resulting Undoable).

Above invariant violations are fixed. But, in a debug build, I can nonetheless force a crash as follows. Steps 1-3 are identical to above post's steps 1-3, except that it's irrelevant whether tile A is black or normal.

1. Put a terrain tile A across the torus seam, overlapping the seam.
2. Put a terrain tile B under the torus seam, overlapping tile A, but not the seam.
3. Group tiles A and B.
4. Activate Undo once to get back the loose tiles A and B. (Don't use un_group_, but un_do_.)
5. Move tile B.

This crashes because, after step 4, tile B's coordinates violate the torus invariant (see previous post). Between steps 2 and 3, tile B's torus invariant was still fulfilled. I'll have to investigate and fix this, too.

[SimonN]

I have a fix candidate for all known crashes in my unstable Lix repo, branch master there. I don't find anything wrong with it, I'll release that in Lix 0.10.20 in 1-2 weeks.