Chisels & Bits - For Fabric

Chisels & Bits - For Fabric

2M Downloads

Undo/redo operations still affect blocks even when the API's undo groups are never used.

Phylogeny opened this issue ยท 9 comments

commented

I decided to add configs for disabling undo/redo functionality for EBM tools, since some users may consider it a waste to fill their history with certain operations. It turns out that even when IChiselAndBitsAPI#beginUndoGroup and endUndoGroup are never called, since BitAccess#commitChanges always interacts with the undo tracker, all the changes are still added to the history and can be undone.

Ironically, it adds them one block at a time, rather than in a group (as was what always occurred with #145), so refraining from adding undo groups fills the history worse then ever.

commented

Groups just define which operations are grouped, everything should be recorded.

I don't know that it makes sense to ignore operations with Undo, if for instance I spend 30m fine tuning a block only to click on it with the feature you remove from the undo pipeline suddenly all my work is lost, I can't undo or redo that block, because it was modified outside of the undo logic.

Unless the undo pipeline has all the data, it falls apart.

commented

A good example is the wrench. The user may not want every use to take up an entry in the undo history.

In any event, it should be made clear, then, that refraining from utilizing IChiselAndBitsAPI#beginUndoGroup and endUndoGroup when committing changes on the client when multiple blocks are modified at once will effectively clear one's history and replace it with many individual actions. It means that the only way to commit changes without decimating the user's undo history is to only commit them on the server.

commented

I admit that the documentation on the methods seem to note that they are responsible for all undo logic. However that wasn't the intention.

But like I said, if you use the wrench, then try to undo a change you made to that block earlier it will now fail with "Block Has Changed." To me this sounds far worse then having to hit undo 1 or 2 times to get back to the previous changes.

Since now I cannot use my undo history at all, its basically ruined. That would be like if you were using a text editor, only to find out that you cannot undo anything after you paste in a link.

commented

I was envisioning a scenario where committing changes would not be considered in the undo tracker at all, if you don't call IChiselAndBitsAPI#beginUndoGroup and endUndoGroup. Then you could wrench and change the block all you want without "Block Has Changed" ever coming up; the undo tracker would just have no knowledge of the wrenching operations. (which is what happens already, when changes are committed only on the server).

commented

Let me try to explain.

Consider this,

  1. Spend 30 minutes working on a block with a chisel.
  2. Wrench the block twice to get a different look at it.

Now I decide that my last two changes were no good, So I hit undo.

In the current situation, the block rolls back to the previous rotation, then I hit it again, again, again. I've rolled back the two rotations, then the two previous changes.

However in your requested case, I hit undo and I get an error "Block Has Changed"

This happens because your wanting that those changes not be logged into the Undo Tracker, which in turn means that it has no means of understanding those changes, and when it goes to preform an undo, the system outright fails because the block has been modified outside of the undo pipeline.

I'm not saying that your wrench will produce "Block Has Changed" I'm saying that if the Undo Tracker doesn't know your modifying the block, it breaks the undo tracker because your making changes to the block without teaching it about them ( thus causing "Block has Changed" ).

commented

Ahh, that makes sense. Thanks. Yes, I agree, then.

Perhaps a warning could be put in the documentation that urges API users to 1) make sure to commit all changes on the client and the server, since server-only changes can destroy a users undo history, and 2) make sure to always use undo groups, rather that treating them as optional.

Or not; either way, I agree.

commented

Well undo-groups are really only important with multiple block updates. I do agree that documentation could use some sprucing up.

And yes, Something demoting that all changes should be made on both client and server would be good.

commented

2445093 does this help clarify the situation on the items you mentioned before?

commented

Yes, that's very clear. Thanks.