Ghoulboy78 / Scarpet-edit

A useful tool for developing decorative builds etc in scarpet.
Creative Commons Zero v1.0 Universal
22 stars 4 forks source link

Rotate doesn't behave properly with undo #57

Closed Firigion closed 3 years ago

Firigion commented 3 years ago

For some strange reason, rotating a big structure some angle that is not multiple of 90 degrees and the undoing it leaves blocks behind. I didn't do any code analysis to check why that's the case.

Result of rotating an octahedron (into a space that was formerly just air) image

After undoing the operation image

Firigion commented 3 years ago

I did a tiny bit of code analysis and I think I got the issue pinned down (from past experiece, when I did my undo in other apps). rotate is likely assigning to one block the rotated position of multiple blocks, meaning it sets that block multiple times. That results in the undo history having a block saved more than once. The first time it's saved, the old block it references is the actual old block, whatever was there before the operation. The second time it saves that position, what's saved as old block is what the previous setting operation did, hence not the actual old block (glass, in this case, instead of air).

I can think of two ways to fix this:

  1. when undoing, run the list of blocks to set (affected_blocks) in reverse order. That ensures that the first block to be placed is the last to be removed, fixing the issue.
  2. when saving to history, don't allow to save one position more than once. This would require modifying set_block to do the check each time it sets a block. This solution would make undo more efficient, but all the other set operations less efficient, so I'm leaning for solution 1.