Model Operation blockform methods.

Dan Rathbun

(https://groups.google.com/forum/?fromgroups#!topic/sketchupruby/BrlXUOswqnk)":2bj3err2]

Operations ( model.start_operation, etc) in the Ruby API have always been a pet hate of mine, and recently I had to do a write up on them, so I'd thought I'd share it.

You can read the suggestion write-up here, in order to save putting the full script here. (The link it to a .rb file, but please open and read it - do not execute it)

The basic idea/gist is that operations should be done using Ruby blocks, and a rethink of aborting operations.

Dan Rathbun

Interesting.

I would agree on some things, disagree on others.

1) One major problem IMHO about the current API way, is that newbies do not yet have a good "handle" on how to wrap an operation in a begin .. rescue block, that protects the model from corruption, or their code from losing it's way.
And then, I have caught coders who should know better, not coding correctly, usually because they are in a hurry, etc.

2) So .. I do not know if I'd agree with always raising an exception, and I do think that the methods should detect errors and abort the operation. I do agree it is nice to inform the end users of why the operation failed. Perhaps logging the op failure rather than an exception?

3) Sketchup::Operation#operation, is not Ruby best practice, and sounds like a Little Caesar's ad ("Pizza!Pizza!")
Standard would be #append(), alias #<<().

4) It is not really clear what the difference between #abort and #erase! are.

5) Instances vs Return values: In order to have instance methods, there needs to be a method, (usually a constructor, normally named " new",) that returns a reference to the new instance object. Sometimes the constructor is private, and called by a wrapper method. The Sketchup module and Model class, have quite a few of these wrapper methods that return instances to new objects, .. options(), layers(), etc.
You indicate that Model#operation() should return a reference to an Operation instance object.
That is not strange, given the other constructor wrappers in the API, however it brings up an dilemma. Yes we want a "handle" on the new operation instance, BUT tradiationally authors write their entity creation methods so that they return a collection, group or component, representing the added entities (so that they need not go through slow model iterations to get the new Drawingelement objects.)
Add also that in Ruby block form methods traditionally return the result of the block's execution. This is what I would argue for, and it would need to be up to the programmer to return whatever object(s) upon success or failure (be it false or nil.) Only the programmer knows what the block should produce.

So IMHO it looks like the calls that cause the operation to be executed, should return the results of the block.

This would mean that we cannot hide the Operation class constructor " new" (which would take a block argument,) create the instance, and return it's reference.

Later.. when the programmer decides WHICH model he wants to execute the operation upon (the Mac can have more than one open, and the PC could have opened a different model since the operation was instanced,) ...
... they would do something like this:
@op.call(Sketchup.active_model)
or (we can have both,):
Sketchup.active_model.operation(@op)

This raises a fundamental question whether the [ruby:p4vs7ul8]Operation[/ruby:p4vs7ul8] class, should be a subclass of [ruby:p4vs7ul8]Proc[/ruby:p4vs7ul8] ???

The other major issue is co-ordination with the [ruby:p4vs7ul8]Sketchup::ModelObserver[/ruby:p4vs7ul8] class, and hopefully 2 new "onOperationStart" and "onOperationEnd" callbacks. See:
http://forums.sketchucation.com/viewtopic.php?f=180&t=39516&hilit=operation&start=15#p349408

This is likely to be of interest to ThomThom & TIG.

Dan Rathbun

@dan rathbun said:

This raises a fundamental question whether the Operation class, should be a subclass of Proc ???

Thinking on it.. and playing with a class definition, I do not think a Proc subclass is necessary.

Dan Rathbun

Does anyone here, actually NEST operations within each other ?? (... or is this just asking for trouble?)

It seems frivolous, the nested code might just as well be a begin .. end block, or a method call.

Jim

I thought nesting was a no-no, but I can no longer recall why.

thomthom

@dan rathbun said:

Does anyone here, actually NEST operations within each other ?? (... or is this just asking for trouble?)

It seems frivolous, the nested code might just as well be a begin .. end block, or a method call.

You cannot nest operations. If you begin with a start_operation before an previous one has committed you interfere with the whole operation. I think the first one will commit - not sure what happens to the remaining actions.
That's one of the reasons that making operations in an observer event cause problems.

The transaction model SketchUp uses is linear.

thomthom

I can see the desire of operations in blocks. I've done some wrappers like that, one implements a progressbar feature.
However I've also had tools where start and commit happens in different Tool events - block operations would not work there.

There is much to be desired about the transaction model in SketchUp - but what I'd really like to see is being able to make fully transparent operations.
Currently, if you make an operation with the prev_trans flag set to true it will piggyback on the previous operation - but the downside is when you go to undo, the name of the operation is your own "transparent" one. It is in fact no the operation you make that's transparent, but the previous one, as the argument name indicate. But doing so cause confusion, you want the original operation's name to appear in the Undo/Redo stack.

And some times it would be nice to create truly transparent operations, so they do not appear on the undo list - like Group#move!
Think observer events.

[off:258j3kq6] Group#move! doesn't "apply" a transformation as the docs says - it sets a new transformation. And the view needs to be invalidated in order to see the results.[/off:258j3kq6]

Dan Rathbun

@thomthom said:

You cannot nest operations. ... The transaction model SketchUp uses is linear.

Yep .. I came to the same conclusion, pacing back and forth on the back porch, a few minutes ago.

Let's review: The SketchUp redo and undo stacks are LIFO collections. When a user does an Undo, or a script calls Sketchup.send_action("editUndo:"), the last operation is popped off the undo stack , and pushed onto the redo stack. And the reverse is true in the case of a manual redo, or a Sketchup.send_action("editRedo:") call.

This makes D.B.'s idea of an erase! method for an Operation instance unattainable.
Only the last operation can be "undone." We cannot go into the middle of the undo stack and undo an operation. All the operations on top of it, must first be undone.

In addition, reference the following code fantasy:


  model.operation('root', true ) {|rootOp|

      # ...some rootOp code...

      model.operation('child') {|childOp|
        # ...some childOp code...
      }
      # ...some more rootOp code...

      method_call()
  }

This is not good for newbies to see, even if the name of the nested block form method is changed to append, ...
... it still may mislead the novice (and even the experienced if they haven't yet had enough coffee,) that the nested operation will execute before "some more rootOp code".
It actually will only execute after the entire "root" block and operation has completed, because SketchUp operations can only be chained, not nested.

@thomthom said:

Currently, if you make an operation with the prev_trans flag set to true it will piggyback on the previous operation - but the downside is when you go to undo, the name of the operation is your own "transparent" one. It is in fact not the operation you make that's transparent, but the previous one, as the argument name indicate. But doing so cause confusion, you want the original operation's name to appear in the Undo/Redo stack.

I realized this would probably happen, and am writing my test case so that the "Top Op's" name is propagated onto the "follower Op."

Dan Rathbun

@thomthom said:

... I've also had tools where start and commit happens in different Tool events - block operations would not work there.

Could be why these methods were written the way they are.

And a good reason, why they themselves must remain the way they are, without adding restrictions to them.

thomthom

@dan rathbun said:

I realized this would probably happen, and am writing my test case so that the "Top Op's" name is propagated onto the "follower Op."

How? You're hooking into Model#start_operation ? That was my initial idea - but the problem occurs when your operation follows a native operation...

Dan Rathbun

No I'm not playin' "hookey"... Should have been more precise.

Presently, it only works if YOU make the "first" operation (of the test Operation class,) and use it's append method. Since it is an instance method of the first op, that creates the "follower", it can pass it's own name to the new() call.

So.. yes IF you wanted to have it work for all operations, we would need to hook into the API "op" methods, and build a "operations" collection. (.. or a undo and a redo collection.)
The community would NOT accept that, so that's not on my list of things to do.

I thought about it for like a minute, then said "nay."

I think I will instead define library methods:
operation_start(), operation_commit() and operation_abort()
(Notice the swapping of noun and verb, I like related methods to be together in the source, and in the API, rather than being scattered apart form each other.)

thomthom

@dan rathbun said:

I think I will instead define library methods:
operation_start(), operation_commit() and operation_abort()
(Notice the swapping of noun and verb, I like related methods to be together in the source, and in the API, rather than being scattered apart form each other.)

I've been going back and forth on that myself. I also like when looking up references that similar methods are grouped. Though I have the impression that the "Ruby way" is to use more natural language - so it reads more naturally when you read the actual code. I've not managed to come to a conclusion.

Dan Rathbun

Last night I pushed the first commit of the SAE::Operation class to a Public BitBucket Mercurial repo.

I haven't yet got much chance to test it much.

Check it out:
https://bitbucket.org/dan_rathbun/sae-operation-class

Clone the repo:
ssh;//hg@bitbucket.org/dan_rathbun/sae-operation-class

[floatr:biungwe1][/floatr:biungwe1]ADD: If you do not have a Mercurial client like TortoiseHg installed, you can grab an archive using the "get source" popup menu, at the top right:

Line 8 of file header, is a boo-boo, should read:
" %(#008000)[# idea by Daniel Bowring]"

danielbowring

Fairly late response, but here's my thoughts/responses:

@dan rathbun said:

here

This is one advatage of a fallback system - the operation is aborted and then the error is allowed to propogate
I believe the error should continue to be raised - an exception isn't (normally) what you're expecting, so execution should halt.
My suggested nomenclature isn't set in stone. The reason I chose this is due to the duality of the object it may reference (the current operation for appending, or the (externally identical) interface for starting a new operation
#abort aborts the operation as it is operating - erase! removes it after the fact. It should be noted erase can only be performed on the "most recent" operation (erasing any other doesn;t make sense)
This I agree with - the handle would already be available (as the most recent operation, and as the argument to the block). The value returned should be the result of the block

[6]) [which model to execute on]
Ah, something I didn't think out very well - I think passing the active model is a good way, but I think the user should also be able to pass arguments, ie
op.call(Sketchup.active_model, arg1, arg2, ...)
to allow the user to perform operations on entities.
But then again, when would you ever want it on anything other than the active model?

@dan rathbun said:

here

Nesting means you don't have to worry if code your calling has its own operation. Some API operations, for example, will litter the undo stack.
If you don't want to support this, you can just have calls to start a new operation do "nothing" except execute the block and return the value - the downside to this is you lose the ability to abort (with rollback).

@dan rathbun said:

here

#erase! would only be applicable to the most recent (ie, top of the stack). Anything else would throw an error.
The "code fantasy" is just that - you wouldn't nest operations "directly" like that - Imagine the operation is a method, the code becomes
def root() #... child() #... end def child() #... end
You can simulate this behaviour using clever transparency flags, or by ignoring the child operation (as noted before, with its disadvantage)

@thomthom said:

However I've also had tools where start and commit happens in different Tool events - block operations would not work there.

Operations over different tool events? I was under the impression you had to finish your operation before control returned to sketchup (not that I ever tested, I have just assumed)
Given this anyway, you could just keep "appending" to the previous operation by keeping its instance around (see Equiv. code in the description document) (or since its within the lifetime of the tool, even just using model.last_operation)

Dan Rathbun

Issue: Nesting Operations

@unknownuser said:

@unknownuser said:

(http://forums.sketchucation.com/viewtopic.php?p)":1e6klaes]Does anyone here, actually NEST operations within each other ?? (... or is this just asking for trouble?)

It seems frivolous, the nested code might just as well be a begin .. end block, or a method call.

You cannot nest operations. If you begin with a start_operation before an previous one has committed you interfere with the whole operation. I think the first one will commit - not sure what happens to the remaining actions.

**%(#BF4000)[I tested this. In reality NOTHING gets committed.

puts() statements will output to the console, but ALL model modification statements are ignored (or perhaps automatically aborted.)]**

@unknownuser said:

Nesting means you don't have to worry if code your calling has its own operation. Some API operations, for example, will litter the undo stack.

There is no way for me to parse or detect whether methods (or procs,) have an operation within them.

(Unless I hook the API's three operational methods, which are a HUGE no-no. It is a violation of my number 1 law for the SAE utilities, and has now become an explicitly stated violation of the Trimble API Terms of Service. Google was not interested that much in the SketchUp API, as they were not a "Rubyist" company, and were more interested in the native integration of SketchUp with certain Google applications and services. Trimble has the opposite viewpoint, since they have announced their intention to provide professional plugins for SketchUp, and enhance their other applications with the SketchUp engine. So expect Trimble to be much more aggressive in protecting the integrity of the API.)
Again, I am opposed to showing, implying, and actually allowing coders to get the idea that nesting operations is something valid.

It goes against Ruby Best Practices, and is contrary to the LIFO sequential nature of the Undo/Redo stacks.

@unknownuser said:

If you don't want to support this, you can just have calls to start a new operation do "nothing" except execute the block and return the value - the downside to this is you lose the ability to abort (with rollback).

Not necessarily true.

A method call or statement, within a operation block, can always call model.abort_operation() (which is one good reason to always pass the model reference into the block.)

@unknownuser said:

... when would you ever want it on anything other than the active model?

In an MDI interface, operations CAN be performed on the other open models.

There are plugins that use another open model as a "scratch pad" to create components and save them out to a collection, or have some kind of co-ordination between two open models. (This can only be done on the Mac currently, of course.)

Dan Rathbun

Issue, the term: "Child" Operations

I am going to remove any reference to operations being a "child". (It is proving problematic.)

In reality, the C-side undo/redo engine is the parent of ALL operations.
Parents (usually,) produce children in a sequential, one-by-one manner.

Operations as recorded on the undo & redo stacks are actually siblings of each other.

Appending an operation to it's elder sibling, is more analogous to conjoined twins, than a parent-child relationship.

~

danielbowring

@dan rathbun said:

....stuff...

I think you're overlooking that I wrote this not to be implemented in ruby, but as something to be implemented in future Sketchup Versions.
I too have tried to implement a "perfect" way of doing this in ruby, and the same pitfalls (of course) arise.

Dan Rathbun

@unknownuser said:

I think you're overlooking that I wrote this [suggestion for an Operation class] not to be implemented in Ruby, but as something to be implemented in future Sketchup Versions.

No not overlooking that.

Of course it MIGHT be easier for the Trimblers to detect when a start_operation has been called a 2nd time, before the first has been committed, and ignore the 2nd.

But I think the pitfall is that they will not find a consensus on whether they should ignore the next commit they "see" (assuming it's is paired with the 2nd start_operation,) and "hope" for getting the "outer" commit.

I suspect that it will prove to be too confusing (if that's the correct word,) to attempt to compensate for poor programming practice.

danielbowring

@dan rathbun said:

No not overlooking that.

Of course it MIGHT be easier for the Trimblers to detect when a start_operation has been called a 2nd time, before the first has been committed, and ignore the 2nd.

But I think the pitfall is that they will not find a consensus on whether they should ignore the next commit they "see" (assuming it's is paired with the 2nd start_operation,) and "hope" for getting the "outer" commit.

I suspect that it will prove to be too confusing (if that's the correct word,) to attempt to compensate for poor programming practice.

And that's the advantage of using code blocks - by definition they have a defined start and end
If you we're going for support, you'd have to assume that the scripter got it right - you shouldn't try to solve every problem they could cause. If an error is the result, that's because there was an error to begin with.

Dan Rathbun

@unknownuser said:

And that's the advantage of using code blocks - by definition they have a defined start and end
If you we're going for support, you'd have to assume that the scripter got it right - you shouldn't try to solve every problem they could cause. If an error is the result, that's because there was an error to begin with.

OK. (Head spinning!)

That's what I said.

Then you made the argument that the Operation's block should be able to call methods that could have an operation in them (making a nested operation, which is invalid.)

The main problem we have with the API is that these invalid operations, just fail silently, when they should raise a ScriptError (or some other exception.)