Model Operation blockform methods.

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.)

Dan Rathbun

Weird...

If you specify an empty string, as the name of the first operation, in a set which the followers are transparent,... the name in the menu is: "Undo Macro"

Dan Rathbun

I pushed several changes today.
https://bitbucket.org/dan_rathbun/sae-operation-class

Works much more like it should:

call/ commit method can take an array of values to pass into the block.
class method ::raise_errors() and ::raise_errors=() to control whether exceptions are raised. (This will likely be used for debugging mostly.)
class method ::redo wrapper for Sketchup.send_action('editRedo:')
class method ::undo wrapper for Sketchup.send_action('editUndo:')
an OpStat object is passed into the block, holding information about the operation itself that might be useful. (This was done instead of passing a direct reference to the operation itself, which would open the door to dangerous things, like calling commit from within the block and causing a vicious loop that would probably cause a stack overflow.)
onError() {|opStat,model,exception| ... code block ... }

Allows custom handling of errors (ie, translate error messages into Spanish, etc.)

onSuccess() {|opStat,model,results| ... code block ... }

Allows custom handling if everything goes well. (ie, prompt the user to save something, etc.)

Dan Rathbun

Here's an example of using the one-shot block form class wrapper method:

1) Say you write a simple "Add Face" plugin with no operations (because either you did not know about them being a newbie, or you just wished to get it working first and polish it later.)

So you have code that looks like:

require('sketchup.rb')

module Author ### <-- this would actually be your toplevel namespace

  module FaceAdder
  
    class << self
      private
      def add_face
        #
        mdl = Sketchup.active_model
        depth = 10
        width = 10
        pts = []
        pts[0] = [0, 0, 0]
        pts[1] = [width, 0, 0]
        pts[2] = [width, depth, 0]
        pts[3] = [0, depth, 0]
        # Add the face to the entities in the model
        grp = mdl.entities.add_group()
        face = grp.entities.add_face( pts )
        return grp
        #
      end
    end # proxy class
    
    unless file_loaded?(File.basename(__FILE__))
      UI.menu('Plugins').add_item('FaceAdder') {
        add_face()
      }
      file_loaded(File.basename(__FILE__))
    end

  end # module FaceAdder
end # module

2) Now you want the the entire command to be one Undo operation.

You can modify the menu item proc, like this:

      UI.menu('Plugins').add_item('FaceAdder') {
        begin
          require('sae/operation.rb')
        rescue LoadError
          add_face() # User does not have file.
        else
          SAE.operation('Add Face') { add_face() }
        end
      }

In this example, the "Undo" name that will appear in the menu (or the popup tooltip for the undo button on the toolbar,) is specified as "Add Face". All other options will use the defaults, .. ie, the 2nd model argument will default to the active_model, the 3rd argument will be set to disable the UI during the operation's block execution, and the operation will not be hidden.

Putting it all together, the example file, would look like:

require('sketchup.rb')

module Author ### <-- this would actually be your toplevel namespace

  module FaceAdder
  
    class << self
      private
      def add_face
        #
        mdl = Sketchup.active_model
        depth = 10
        width = 10
        pts = []
        pts[0] = [0, 0, 0]
        pts[1] = [width, 0, 0]
        pts[2] = [width, depth, 0]
        pts[3] = [0, depth, 0]
        # Add the face to the entities in the model
        grp = mdl.entities.add_group()
        face = grp.entities.add_face( pts )
        return grp
        #
      end
    end # proxy class
    
    unless file_loaded?(File.basename(__FILE__))
      UI.menu('Plugins').add_item('FaceAdder') {
        begin
          require('sae/operation.rb')
        rescue LoadError
          add_face() # User does not have file.
        else
          SAE.operation('Add Face') { add_face() }
        end
      }
      file_loaded(File.basename(__FILE__))
    end

  end # module FaceAdder
end # module

If this kind of method were to be added into the API, it would likely be an instance method of Sketchup::Model, which would not need the 2nd model argument.

Of course there could also be a model "flexible" duplicate class method (like I've implemented it,) that does take a model arg, which would either be a module method Sketchup.operation(), or a class method like Sketchup::Model.operation()

Thoughts ??