Undo in the editor

[Simon]

Undo in the Lix editor will happen.

This is long overdue. It's hard to implement on top of an editor that doesn't already support it, but it doesn't matter, it must happen. It's fine if it takes several weeks, as long as I focus on it whenever I develop.

Reason: Yesterday, I trashed 20 minutes of unsaved work and raged.


There are more severe usability issues in the editor. Giga's rant is still 80 % accurate after two years, mainly about how disorienting the editor is, and how bad/long the feedback loops are.

It's glaring when one hasn't build levels with the Lix editor for a year, and then comes back to it.

-- Simon

[namida]

In cases where I've had to retroactively implement Undo - and even in some cases where it isn't retroactive - I've achieved this simply by having a List<MemoryStream>, and each time a change occurs to the level, the level is saved (exactly the same way it might be saved to a file) to a MemoryStream which is then added to this List<>. When the user hits Undo, load the previous stream. Have an int variable keeping track of the current position in the Undo list; if the user is not at the end of the list and makes a change, nuke all MemoryStreams that are newer than the current position. (Not nuking them until this allows for a Redo function to exist.) You may need to skip certain aspects of loading (for example, you probably don't want to re-center the screen just in response to Undo) when loading from the undo list.

Probably a bit memory-hungry compared to a delta-based implementation, but also likely to be less bug-prone (especially in terms of bugs that cause data loss rather than just annoy the user) and far easier to implement when the system wasn't designed from the ground up with Undo support. I'd also guess that IRS levels are generally not complex enough for this kind of Undo to become a problem memory-wise on modern PCs.

[Ramon]

Very nice! An undo function will definitely improve workflow during level design sessions. It is not rare to do some unwanted button- or keypress that, while not terribly wiping all the work put in, is sometimes annoying to manually revert.

(On second read that sentence sounds really weird.)

[Simon]

Thanks. Yes, if I can't get anything else to work properly, I will savestate the entire level.

First, I will attempt a delta-based solution, with the OO pattern of doable/undoable command objects. The undoable actions will be really low level: For a rotation of 7 selected tiles around their averaged center, the undoable action is a 14-fold composite action of 7 individual undoable rotations and 7 individual undoable moves.

I had a similar idea 3 years ago, but wanted really high-level undoables. It was hard though to revert the 7-tile rotation exactly: There are rounding errors when computing the centers and the 7 tiles' new integer positions, with hackish special-case code to kill the rounding errors. This is a nightmare to revert high-level, but will be much easier low-level.

Ramond: Yes, the mistake is usually annoying to revert, but it was rarely annoying enough to push undo. The way I want everything, it will be a large-scale refactor of the editor, and I had been too scared to attempt it seriously for years. >_>

There will be many undoables, you can change the level in so many different ways. Maybe I'll only implement undoable deletion first, and clear the undo stack whenever a not-yet-undaoable command comes. Mis-deletions and mis-moves are the most annoying mistakes.

-- Simon

[ccexplore]

It was hard though to revert the 7-tile rotation exactly: There are rounding errors when computing the centers and the 7 tiles' new integer positions, with hackish special-case code to kill the rounding errors. This is a nightmare to revert high-level, but will be much easier low-level.

Interesting.  Does that mean it would've been difficult for a user to manually revert too?  It sounds like the user couldn't just rotate it back the other direction by the same amount and always expect to get back exactly to the original setup.

It's probably easier to implement undo of movement of a single tile by simply remembering the old position/orientation, and just restore them directly upon request to undo, instead of trying to honor it via an "opposite" movement.

[Simon]

rounding errors when computing the centers and the 7 tiles' new integer positions, with hackish special-case code to kill the rounding errors. This is a nightmare to revert high-level, but will be much easier low-level.

Does that mean it would've been difficult for a user to manually revert too?

Yes, exactly, this is a problem.

The special cases fix only the most common case when all these apply:

• You have one or more terrain tiles selected,
  
• you have no gadgets selected, and
  
• the encompassing rectangle of your selection has no sides that exceed wrapping dimensions of the map.
  

In this good case, if you rotate your selection 4 times, it should be back where it started.

Selected gadgets don't rotate when we tell them to rotate, this is the OO fruit basket problem with the fat-interface solution. Thus, gadgets in the selection break the assumption of the anti-rounding-error code that the (encompassing rectangle of the pieces that rotated in unison) is, per dimension, at most 1 pixel off the (theoretical rotation of the original encompassing rectangle).

Torus dimensions mod the tile coordinates after every rotation, this can squish the selection together too early, e.g., when you press rotate-quarter-turn twice to rotate half a turn.

remembering the old position/orientation, and just restore them directly upon request to undo, instead of trying to honor it via an "opposite" movement.

This is smart. My hunch would have been to separate UndoableMove : Undoable and UndoableRotation : Undoable. But your approach sounds even more robust and reasonable.

-- Simon

[Forestidia86]

Do you plan undo and redo then for grouping of tiles? There is an issue (#280) that ungrouped tiles go into their original position instead of the one that they were in when they were grouped (Edit: remembered that wrongly, the issue only occurs when rotating/mirroring the tile group, the tiles go then in the original grouping position). How does that interact/relate then? Does undo put them then in the grouping position instead of the original?

[Simon]

Undo a grouping: Yes, this should be undoable. Whenever the most recent action has been a grouping, undoing that grouping can't run into issue #280 because the group is in its original orientation.

Issue #280 is interesting and needs more work. I feel like issue #280 is rarely a problem, but when it hits, it feels so odd, indeed. I haven't invested enough time to think about a nice solution to #280, maybe it's easier than I think.

-- Simon

[Forestidia86]

Ah, I understand, I had the issue wrongly remembered.

[Simon]

After heavy refactoring, this is the status quo of unreleased development lix-unstable, branch editor-undo:

• Editor has undo/redo buttons.
  
• Inserting a tile from the tile browser is undoable. This is good as-is, although rarely useful.
  
• Deleting an arbitrary selection is undoable as a unit. Undo re-adds the tiles. They aren't selected after getting added. Should they be selected? This will be a doable but nontrivial task; ideally, we'll decide later.
  
• Copying an arbitrary selection is buggy at the moment, this must be fixed, and this should also be undoable as a unit.
  
• Moving an arbitrary selection is undoable, but in a terrible way: While the mouse isn't still, each individual moving tile generates 60 undoable commands per second only for this single tile. These must certainly be grouped. But how? Complex scenario: You drag 3 tiles, and mid-motion, hit the hotkey to rotate them. Should this generate 1 undoable, or 3 undoables (move, rotate, move)? I'm leaning towards 3.
  
• Rotation/mirroring/dark isn't undoable yet.
  
• Changing level properties via dialogs isn't yet undoable. This is the most boring and I have 100 % compassion for NL to have had a bug there.
  
• When stuff happens that isn't yet undoable, but we undo the most recent undoable, consistency checks fire and crash the app with an error.
  
• When the undo stack is empty, the undo/redo buttons are shown anyway. This is a general problem in the editor, it has clickable buttons that do nothing, e.g., rotate button when you have nothing selected.
  

Current no-undo Lix version is 0.9.31. Maybe I'll have enough undo in shape for 0.9.32, maybe not. Anyway, the first version with undo will probably get this:

• Fix copying tiles. Then all adding/removing will be undoable. This will be useful already: Undoing deletions is the important use case.
  
• Make the tile moving non-undoable, i.e., cut this half-baked feature. Undoable moving can happen in a future version. I haven't released in months, it's time to release something.
  
• Empty the undo stack whenever we do something yet non-undoable in the editor. This will keep the undoing always consistent. No more crashes from detected inconsistency.
  
• Show/hide the undo buttons when the stack is empty.
  

In the yet-distant future, everything should be undoable. The undo stack should never be emptied during normal work.

-- Simon

[Proxima]

Changing level properties via dialogs isn't yet undoable.

Does it have to be? If you want to change the level properties, even if it's reverting a change, you are more likely to go into the dialogue again so as to make sure the values are correct, since most of them are not visible from the main editor screen. Conversely, making this kind of change undoable carries a high risk of the user accidentally undoing and not realising something is wrong.

I guess the one time you would want to undo a dialogue change would be if you know you want the old value back but don't remember exactly what it was.

[Simon]

Right -- skills, name, number of lix etc. don't have to be undoable. I was unsure with this too. I suspected that it should be undoable for consistency with everything else, but good usability here will be delicate and costly, right.

Map dimensions and wrapping: When this changes, tiles may move/wrap. This seems much harder to keep orthogonal to what happens in the undo stack. Likely, I'll have to make this undoable.

-- Simon

[namida]

Deleting an arbitrary selection is undoable as a unit. Undo re-adds the tiles. They aren't selected after getting added. Should they be selected? This will be a doable but nontrivial task; ideally, we'll decide later.

I would personally think it desirable but non-critical for them to be selected.

But how? Complex scenario: You drag 3 tiles, and mid-motion, hit the hotkey to rotate them. Should this generate 1 undoable, or 3 undoables (move, rotate, move)? I'm leaning towards 3.

I would not have expected this situation to be possible in the first place - I would expect the rotate either terminates, or is ignored during, the drag. To be fair, I wouldn't even attempt it in the first place.

I should note that I tend to follow the opposite workflow of this, though. I would likely be using the clickable buttons to rotate / etc, while using the keyboard to move pieces. This is due to the keyboard movement being more precise. I may occasionally move a piece a long distance with the mouse; I would not try to rotate / etc it while doing this (I'd most likely do so afterwards, if I were going to do so at all, though I'm not 100% sure it would never be "before" - I am sure it would not be "during"), and would almost certianly follow up with some keyboard movement afterwards to get it to the exact desired spot.

Make the tile moving non-undoable, i.e., cut this half-baked feature. Undoable moving can happen in a future version. I haven't released in months, it's time to release something.

I would consider undoing a move to be almost as important, perhaps even more so, than undoing a delete. Accidental moves, or accidentally including something in a move, etc, are far easier to do than an accidental delete, in my experience.

Empty the undo stack whenever we do something yet non-undoable in the editor. This will keep the undoing always consistent. No more crashes from detected inconsistency.

Unsure about this. Perhaps valid if the intent is "this will never be undoable", though perhaps preferable there is to make the thing in question outside of Undo / Redo altogether, rather than making it clear the stack. For things that will be undoable in the future, perhaps instead of clearing the stack, it should give a warning when the user attempts to undo, then allow them to continue undoing from the last undoable action?

[Simon]

Thanks, several good ideas.

You drag some tiles, and mid-motion, hit the hotkey to rotate them. Should this generate 1 undoable, or 3 undoables (move, rotate, move)? I'm leaning towards 3.

I would not have expected this situation to be possible in the first place

I think I do it because I know how it's implemented. The mouse dragging moves the tiles 60 times per second; these moves (in 0.9.31 = current no-undo Lix) are committed instantly to level every time, thus the intermixed rotation is meaningful and uninterruptive.

The downside of this frequent committing is that it's more work now to merge the entire dragging into a single undoable.

This frequent committing isn't standard in GUI. Other programs will let user drag a shadow or an outline of the dragee. I'm really 50:50 on this. The rotation on the fly is cool and matches how we move real-life household objects.

clickable buttons to rotate / etc, while using the keyboard to move pieces.

follow up with some keyboard movement afterwards to get it to the exact desired spot.

Right, this tile nudging via keyboard is also extremely common. The keyboard is more precise than the mouse when grid alignment won't suffice.

When the mouse isn't dragging, and the key is merely tapped, not held, then I'll make the tap generate a single standalone undoable for its effect: moving all selected pieces, rotating selected pieces, deleting selected pieces, ... When we tap 5 times in quick succession to nudge selection rightwards by single pixel each, this generates 5 undoables that I won't merge. Sounds OK.

When a directional key is held, not merely tapped, the selection moves once, then waits (threshold), then moves 60 times per second. I would like the entire keyholding to generate only a single undoable.

There is more to reply. Until soon!

-- Simon

[Simon]

I would consider undoing a move to be almost as important, perhaps even more so, than undoing a delete. Accidental moves, or accidentally including something in a move, etc, are far easier to do than an accidental delete, in my experience.

Undo of tile moves is now implemented! Moving multiple tiles generates only a single undoable, and (continuous dragging) or (directonal key holding) also generate only a single undoable. Comfortable.

Thanks for the push! It was interesting to design the two possibilitis of merging moves: Several tiles moving the same distance as the first tile, or the same tiles move further than they moved originally while dragging or key-holding.

I agree that moving is the most common action. Even though a single wrong move is easier to correct manually than a wrong deletion, it's likely possible that wrong moving generates the most frustration overall.


Thus, tile moves and insertions/deletions are already fully undoable in the unstable repo.

What's left:

• Duplicating tiles must be undoable. The most likely candidate for this is a compound undoable of an insertion followed by a move. (A proper copy buffer that doesn't immediately paste would also be nice, e.g., to copy between levels, but that's a separate feature.)
• Rotations and mirroring must be undoable.
• Resizing of the map must be undoable, because it prompts tiles to move, and activating wrap (torus) imposes new restrictions on allowed tile coordinates. If we don't make resizing undoable, then it might violate the consistency checks of existing undoables on the undo stack that move to/from now-forbidden coordinates.

Editing the name/skills/numbers of levels is completely orthogonal to everything on the undo stack, and we can easily leave it out.

-- Simon

[Simon]

I have undoable duplication of tiles (copy-paste without a copy buffer). There is a regression left behind: The newly inserted tiles aren't yet selected. The old selection remains, which is practically never what you want.

New to-do list:

• Enrich all Undoables to suggest which tiles should be selected after apply/undo. Main reason are copy-pasting Undoables, but it will be handy for insertions and undone deletions.
• Rotations and mirroring must become undoable.
• Z-ordering (move to foreground/background) must become undoable.
• Hotkeys Z and Y will bind to undo/redo by default. Both are still free in the default editor layout.
• Resizing of the map must become undoable, or clear the undo stack if we want to rush the feature's release.

There will also be lots of lovely refactoring for cleanup, but that's not necessary to release the undo feature. Since the Undoables do the bulk of the work, I will be able to ditch the Hover class hierarchy, which is parallel to the tile hierachy and has bugged me since I made the editor.



-- Simon

[Simon]

Spent all Saturday on Undo. Progress:

• Hotkeys Z and Y are bound to undo/redo by default. You can change it in Lix's options menu.
  
• All Undoables will now suggest what tiles shall be selected after apply/undo. In particular:
  When you undo a deletion, the undeleted tiles will be selected.
  When you duplicate tiles, the newly inserted tiles will be selected.
  
• Lots of refactoring towards immutability of the involved classes. But the asserts in the area where I considered triple dispatch have begun to fire when I duplicate a large selection of both gadgets and terrain tiles.
  

New to-do list:

• Fix above bug in the undoable copy-paste.
  
• Rotations and mirroring must become undoable.
  
• Z-ordering (move to foreground/background) must become undoable.
  
• Resizing of the map must become undoable, or clear the undo stack if we want to rush the feature's release.
  

Cleanup work that isn't necessary to release undo, but that I would like to tackle:

• Ditch the Hover class hierarchy in the editor. This is a parallel class hierarchy that wrap a selecte tile. Work more directly with the raw selection of tiles.
  
• Open a pull request against Phobos, the D standard library, to allow RedBlackTrees with immutable classes as payload.
  
• Open another pull request against Phobos to make SortedRange const-correct. I would have preferred SortedRange over RedBlackTree for the Lix code from the beginning, but RedBlackTree had fewer roadblock bugs.
  
• Attempt to fix a compiler bug: 21321: Class with unimplemented interface method compiles, links, then segfaults, if inherited through abstract base class. I ran into this yesterday. It would be the first time that I seriously look at the D compiler's source, it's a large and scary codebase.
  

^ Illustration of a pull request. You must be careful, watch for feedback, and don't mind the time spent. I had one pull already merged into Phobos a month ago.

In the end, running into 4 library/compiler issues within one big Lix feature shows what parts of D are rarely treaded: Heavy object-orientation, exhausting and combining the OO features of D, making stuff transitively immutable where possible, and using the more obscure areas of the standard library.

Crane would be delighted, if it weren't all so template-heavy.

-- Simon

[Simon]

Lots of bugfixing this evening. Copypasting does the right thing now in all cases, I don't believe I have introduced other bugs yet. This resolves the bug in yesterday's to-do list.

I've begun with the class that applies/undoes tile rotation/mirroring/darkening. This will be exactly how ccexplore suggested I should implement tile moving. The exception is that the existing TileMove objects merge with one another if they're the same tiles that get dragged further during the same mouse dragging, or if they're simultaneously selected tiles that move the same distance. Rotating/mirroring/darkening will never merge with subsequent editing; it will only form a CompoundUndoable with the simultaneously selected tiles.



Thus, very successful evening. Now time for bed.

-- Simon

[WillLem]

I'm enjoying all these cute gifs

[Simon]

Grouping/Ungrouping is undoable.

Small issue left behind during Ungrouping: If you have more than one group selected and click Ungroup, only one of the groups will ungroup. I assume that this is unimportant.

Darkening (turn terrain piece into eraser piece) is undoable.

To-do:

• Rotations and mirroring must become undoable.
  
• Z-ordering (move to foreground/background) must become undoable.
  
• Resizing of the map must become undoable, or clear the undo stack if we want to rush the feature's release.
  

-- Simon

[namida]

If you have more than one group selected and click Ungroup, only one of the groups will ungroup. I assume that this is unimportant.

If I specifically thought about this / tried it out, I would expect the behaviour to be that it ungroups all. But, I probably would never find this unless I was specifically thinking of edge cases to try, in the first place.

[Simon]

expect the behaviour to be that it ungroups all.

Right, that is what I would expect, too, and this is how it has worked before undo.

The new limitation (only ungroup one group, keep all others grouped) comes from the strong encouragement (by the undo design) to describe the entire modification to the level before committing anything to the level.

After one group tile has been deleted and its parts have been inserted in the level, the next group tile will potentially sit at a different z-order (counted from the front of the terrain list), and this messes with the iteration if we commit the first ungrouping to the level. If we don't commit the first ungrouping yet, then the iteration continues well, but I save absolute positions (index numbers within the terrain list) in the Undoables, and the future absolute positions for the next ungrouping will be computed wrongly.

There are hack solutions around this: Create ungrouping Undoable, commit to level, create next ungrouping Undoable, commit to level, ..., then undo all these Undoables, create a CompoundUndoable of them, and recommit this CompoundUndoable. Or track meticulously all the index shifts in some complex data structure.

But ungrouping is already very rare, and, as you say, normallly used on single groups. Hacking or complex counting code doesn't seem appropriate, even though it would be technically correct.

We could also limit this command to work only on single selections, i.e., offer it only in the menu when a single group tile is selected. Generally, in the UI, I should offer only the buttons that are meaningful for the current selection, and hide/grey out the others. This will be a nice long-term improvement for the editor.

-- Simon

[Simon]

Mirroring of tiles and rotating of tiles are now undoable.

The Hover parallel class hierarchy is ditched. The editor is refactored to work directly with sets of iterators to tile occurrences. This wasn't strictly necessary, but I've looked forward to this since the beginning of undo.

These were two wholesome evenings with solid progress. Now it's time for early sleeping.



To do:

• It must become undoable to move baby anteater on top of mom, thereby moving mom anteater below baby, but ideally without having to assume in the usercode that they're anteaters.
  

Nice to have:

• Resizing of the map must become undoable. In a pinch, we can clear the undo stack if we want to rush the feature's release, but it's better if the resizing is undoable.
  
• Clicking the in-editor button for new level: This may or may not be undoable, would be nice-to-have. Epecially if we make resizing undoable, this shouldn't be hard to make undoable, too.
  

-- Simon

[Simon]

Started with undoable resizing, because it's easier than z-ordering tiles. But haven't used this newly written Undoable yet.

Mouse hovering has new bugs since ditching the Hover class hierarchy. The editor will offer the wrong tile during mouse hovering if several tiles overlap. Mouse hovering is a nontrivial mixture of checking which tiles' bounding rectangles are under the cursor and which tiles' opaque pixels are under the cursor. The code before was complicated, a rewrite might have been good anyway, but I was too aggressive.

-- Simon

[namida]

Is the opaque pixel check necessary? As far as I'm aware, neither NL editor has had this check, and it has not been too difficult to deal with. I will note that the current one at least (not sure about the old one) has a priority invert key.

[Simon]

From experience, both checks are necessary for the user experience; I'm really annoyed about the mishovering that this bug reintroduced.

When the mouse is on a solid pixel, this pixel's tile should be selected. Reason: If we went by the tiles' bounding boxes, we would select a very large and thin tile instead, e.g., we select a Crystal-mesh that overlays filled smaller tiles, even though the mouse points through the holes of the mesh to the filled tiles behind.

When the mouse points to air, we select the tiles by bounding box. Reason: If the thin mesh sits all alone in space, pointing somewhere within its bounding box will unambiguously mean the mesh.

During priority invert, I don't have such strong opinions what's best. I think I want to select the tile that produces the hindmost solid pixel (that we don't see), or, if the mouse points to air, the hindmost bounding box.

-- Simon

[namida]

When the mouse is on a solid pixel, this pixel's tile should be selected. Reason: If we went by the tiles' bounding boxes, we would select a very large and thin tile instead, e.g., we select a Crystal-mesh that overlays filled smaller tiles, even though the mouse points through the holes of the mesh to the filled tiles behind.

I would personally 100% expect to select the mesh in this case, and not have to specifically locate a pixel where the mesh is solid.

I would wonder though if this holds true for someone who's not used to LemEdit and/or the two NL editors.

[Simon]

Interesting, thanks. I'll have to philosophize over this. Bounding-box-only was old behavior during very early D Lix editor development. I spent extra time to introduce selection by solid pixel, exactly because I didn't want the mesh to snatch all the careful pointing to the stuff behind it. I.e., personal strong feeling.

Accompanying reasoning was that when we select by solid pixels first, we will always be able to point to the interesting tile, because tiles will only be in the level when they produce at least some visible pixels.

Still, a feeling should probably come before such a reason. I agree that it would be helpful to see what others feel.

-- Simon

[namida]

I should note that, in the case of a piece that isn't exactly rectangular, I have no strong feelings about what should happen if I click in the empty space around it. I'd lean slightly towards "it should be selected, even if another piece has solid pixels below the mouse", but not nearly as strongly in this case. I feel far more strongly about it in the mesh case, or in general, in cases where we're dealing with a hollow space inside the piece rather than empty space around it (but within the bounding rectangle).

[Simon]

Right, if the pieces are rectangular, selection by visible pixels and selecting by bounding box always produce the same result.

If there is space around a rectangular tile, then Lix, already during tile loading from disk, cuts the selection box to the smallest rectangle that contains all solid pixels. The only effect of the cut-away space is that it still affects how the tile aligns with the 16x16 grid.

Slopes have lots of empty space, but usually you put nothing into that space, to allow the lemmings to walk up the slope. Thus, slopes aren't good tiles to investigate this either.

Decorative mesh laid over terrain still seems the best example. Here, the mesh must be in the foreground.

Or a long diagonal pole that you put in the foreground, it has a huge bounding box.

Other very thin, concave tiles come to mind, e.g., the palm tree from L2 beach. But you'd probably put the palm tree in the background, not the foreground, and then it won't snatch your hovering even when the program goes only by bounding box.


Fixed the hovering regressions. All mouse-hovering in the Undo branch should behave exactly as in 0.9.34.

-- Simon

[namida]

I suppose I should more clearly demonstrate what I'm saying.

Imagine the below arrangement. Assume this is three terrain pieces, no use of "No Overwrite".

The yellow and green spots, I would lean towards the dirt clump and the bones being selected respectively, but this is not a super strong preference. On the other hand, the cyan spot, I would 100% expect the bones to be selected, even though they have no solid pixel at that point.

[Simon]

That is a nice example.

Cyan: Yes, the bones are so prevalent there that it's usually a mishap when we click in their small hollow gap, expecting the bones to be selected, but the dirt would be preferred by pixel. Selecting the dirt here is annoying no matter what.

The best justification here is that Lix already highlights the hovered tile (that isn't yet selected) with a darker rectangle (hover is grey, selection is white), and you would see that the wrong tile is hovered before you click. You would instinctively move the mouse slightly to hover the bones, then click.

I am used to this visible hovering, can't tell how nasty it is without. I'd expect it to be much nastier without visible hovering. But I also can't tell either how often this mishap happens of clicking in the tiny hollow space. I'm personally weak with mouse pointing in general, I often misclick stuff even in non-Lemmings GUIs.

Green: My prevalence is weak, too, but merely because the dirt is so large; it feels as if the user should point more clearly to the dirt to select the dirt. Still, it sounds reasonable to program the green spot to select the dirt. I wouldn't be annoyed if it selected the bones.

Yellow: This is much more clearly a spot on the single bones. I might get annoyed if it picks the dirt, but at least the annoyance would be over quickly because I'd immediately re-click with priority invert. Priority invert in general is super useful in any editor here, and it's good that both editors have it.


Thanks, I see that both algorithms have their drawbacks and nothing does 100 % what the user thinks. It might even depend on what tiles are common in the games.

If we were to optimize this to the extreme, maybe tile designers should tell the game how they want each tile to be treated: Selectable by convex hull (bones) or by pixels (mesh).

-- Simon

[Simon]

Z-orderings of single tiles work. Hack/regression: If user selects multiple tiles, then gives command to z-order: Editor will z-order only one tile (ostensibly at random), and deselect (!) all others. Reason: By design, Undoables always suggest what tiles should be selected afterwards. The hack is that only one z-ordering works well, thus only one tile is selected afterwards.

I feel like I can't release with such a regression. I want to find a solution (will likely be elaborate and inelegant) that (1.) works with the existing undo design and (2.) z-orders many tiles at least somehow meaningfully.

Regression is not good. Simon seal of quality demands so such infuriating UI regressions.



Thus todo:
* Somehow allow multiple z-oredrings, or at least keep selection selected
* Clear undo stack on (change of map dimensions)

-- Simon

[namida]

How is Z ordering currently handled, and what makes it difficult to handle a group of them? In particular, why could it not be handled as a series of single-piece reorderings, with the before / after selection being part of the composite undoable rather than the individual steps?

[Simon]

Apologies for not answering this. The problems come from several designs, each highly useful in isolation to prevent bugs:

• I want the multiple z-orderings to go on the stack as a single compound undoable.
  
• I want the Undoable to be completely computed when they go on the undo stack. Reason for this is D's language design: There is no mutable keyword in D, unlike C++, to compute the result lazily in a const/immutable object; immutable also makes guarantees about thread safety that I don't need here, but that a C++-like mutable would subvert.
  
• All changes to the level shall happen by applying undoables to the stack. All other views into the level should be const.
  
• Avoid creating temporary copies of the level to compute the effect of an undoable action.

The problem with multiple z-ordering under these rules is:

The meaning of a tile-list iterator changes after earlier reorderings. I don't want the iterators to remember tiles; they must work solely by position: The reason is that they must continue to work even if the underlying tile is removed and an equal one, but not the very same one, is inserted in its place. Thus, by not tracking identities of tiles, it's hard to track/compute several z-orderings in advance.

Planned hack: Subvert the desire to operate on the level only via the stack. Z-order one-by-one physically in the level, compute single-tile Z-ordering undoables while doing that, revert the physical changes to the level (level is now as before the operation), compound the computed z-orderings, then apply the compound to the stack (thereby applying the same changes a second time, this time for good).

I will finish Undo during the holidays. It will be released during 2020!

-- Simon

[Simon]

Z-Ordering multiple tiles works. None will be deselected.

Todo:

• On changing map dimensions/wrapping, either clear the undo stack -- or make it undoable and wrap the tile coordinates in an undoable way.
• Minor cleanup.
• Release Lix with complete undo support this Christmas.

-- Simon

[WillLem]

Release Lix with complete undo support this Christmas

Great stuff, Simon, looking forward to this! I'll coincide its release with my long-promised video review of the Lix Editor early in the New Year, in order to help showcase the update.

[Simon]

Looking forward to it!

Several things from Giga's rant are still valid, the level view is disorienting. But I want to learn the bigger pain points in general.

-- Simon

[Simon]

Topology changes are now undoable.

This means: Changing width/height/wrap of a level is undoable. The tiles' coordinates will be fixed accordingly, i.e., they wrap according to the new wrappings, and they move if you added/removed space on the left or top of the level. These coordinate fixes are also undoable, and are grouped with the topology change that provokes them.

I'll have to bug-test this manually a couple more times this evening. It's a complicated feature because the coordinate-fixing needed a variant of the normal tile-moving Undoable. The normal tile-moving Undoable wraps, this was necessary because I wanted to easily group moving multiple tiles, only some of which are dragged past a torus seam. The coordinate-fixing must ignore the wrap; it will carry raw coordinates.

If I don't find nasty bugs, I'll release tomorrow. It's already possible to build branch editor-undo of the unstable repo and test yourself. Undo has been released! Undo is a massive feature:

$ git diff --stat master
[...]
55 files changed, 2673 insertions(+), 922 deletions(-)



Happy Simon.

-- Simon