[Potlatch-dev] Introducing Magic Roundabout tool

[Steve Bennett]

On Fri, Feb 25, 2011 at 11:21 PM, Andy Allan <gravitystorm at gmail.com> wrote:
> add a separate undo step for every junction. Since only one keypress
> causes MakeJunction, it should be undoable in one button press.
> MakeJunction should have its own composite action and push things onto
> it when it loops round like this, then passes just one action back to
> the caller.

Err, yes, that was certainly the goal :)

> Oh, and finally, I suspect some of these composite actions should be
> in net/systemeD/connection/actions - especially MakeJunction since
> you've now shown with MagicRoundabout that it's a useful too for other
> actions to build on top of.

Good idea - didn't occur to me.

> Right, well, if this all turns out to be gobbldy-gook don't worry too
> much - I'm happy to rewrite them when I get a chance. If you want to
> give it a go yourself I'm 100% happy to explain further or answer
> questions.

Thanks, I'll have a bit of a read up and explore. The big problem I
was encountering was that I needed the intermediate results of some of
those actions, in order to take later steps. It was no good saying
"after this method, we'll create a circle, then we'll split some
things, then we'll delete some ways. Ok, now go and do that stuff". I
actually needed the split to have taken place so I could identify
which half of the split was the one that I wanted to delete.

I was since then thinking that what I might need was a kind of
CompositeUndoableAction that also carries out the actions straight
away (like the main undo stack), as well as piling them up for later.
As your doc points out:

>A Composite action doesn't actually make any changes itself - it just figures out what list of changes are going to be needed to meet the overall goal.

and

>Although usually irrelevant, this becomes important when iterating over things, and sometimes tricks like iterating backwards over the list become necessary. You can see the ultimate in juggling while working out relations handling during SplitWayAction since you need to insert the new way perhaps multiple times into a given relation, but you need to work out all the insert indexes up-front before any actual inserting is done.

Does this absolutely have to be the case? I can't quite understand,
from a theoretical point of view, why this principle is necessary. Why
not add the action to a stack, and also carry it out now? What's the
benefit of maintaining the current state as long as possible, then
doing all the actions in one flurry?

Also, while I'm irreverently questioning everything:

>There's two conventions that don't help much in understanding, but they are widely used

This is very true. Could we perhaps change them? Maybe rename "undo"
as "actionStack", and rename the ubiquitous "performAction" argument
as "actionStackFunc" or something.

Until last night, I honestly had no idea that actions weren't getting
executed immediately. And even then I only had a suspicion about it,
until I caused an infinite loop with some code that kept trying to
delete a node until it ran out of nodes (which never happened because
they never got deleted). So, I think these conventions are perhaps
beyond quaint and have reached harmful and counterproductive.
(Something about the term "undo" in particular made me assume that we
were piling up a list of actions in case we needed to undo, whereas
actually we're piling up a list of actions *to be done*. The fact that
the user may later undo them is actually quite irrelevant.)


And just to add a bit more incoherent rambling, would a simpler model
be, instead of passing around these stacks everywhere, to have just a
main undo stack with two functions:
1) RecordAction which adds an action to the undo stack, and carries it out.
2) FinishUndoBlock, which groups all actions since the last time it
was called, into one block, which will all be undone with a single
keystroke. Sort of the equivalent of closing a changeset.

But, since I clearly don't understand how this stuff works, these
impertinent suggestions may be completely off the mark.
Steve