Ruby operation left open

Developers Ruby API
1

I’ve had this bug in my plugin for a while. My code is:

# Get handles to our model and the Entities collection it contains.
@model1 = Sketchup.active_model
entities = @model1.entities
	
if @Trusstype == "King Post"
  @Model_operation = 'Draw King Post Truss'
  draw_status = @model1.start_operation(@Model_operation)

  ....  lots of code here ....
end

.... more code here ....

draw_status = @model1.commit_operation

Everything looks good but I keep getting this error:

warning: Ruby operation left open: "Draw King Post Truss"
2

Strange thing is that I have this exact code in my other two plugins and it works perfectly. Even though it does throw this error it does not appear to abort the command. And the undo function allows me to reverse the operation. I don’t know I’m confused. I’ve been scratching my head over this for over 2 hours now and I’ll probably have to throw in the towel.

3

It appears start_operation only is called when a certain condition is true, but commit_operation is called regardless.

2 Likes
4

That’s by design. SU is able to recover from the Ruby operation left open. But we warn when we detect this because it’s bad behaviour that should be addressed.

Christina stopped the logic flaw that is probably your culprit.

2 Likes
5

Some things that aren’t really related to the question, but might be useful to consider in the feature:

@model_operation doesn’t hold an Operation object (there is no such class), but the name of the operation. I’d typically call this operation_name for a more honest and easier to understand code.

I suspect you never use the draw_status variable as it in my experience would always contain the same value. If so, you can just purge it to have less things going on, and make the code easier to read.

The capitalization of “King Post” suggests it is exposed in the UI. Generally it’s good to separate UI strings from internal identifiers, as the UI may be translated or in other ways changed.

String comparison is also slow in Ruby, so for an internal identifier a symbol, :king_post, or a constant referencing an Integer, KING_POST, would be faster. Using another type for identifiers could also help distinguish them from UI string.

In the long term though, I’d advise to use a class hierarchy rather than identifiers to structure the code. Rather than comparing @Trusstype to a String, you could check if @truss is of a certain class, like so @truss.is_a?(KingPost). Even better could be to define the behavior common to all trusses in the Truss class, and then have the KingPost class override that method, perhaps with a call to super.

1 Like
6

Fortunately, SU is able to recover otherwise this ongoing (minor) issue would have broken the Truss plugin a long time ago.

Your right I never use the draw_status variable even though there was some intent to do something with it at some point.

Thank-you for the informative suggestions on structuring the code. I’m kind of in clean up mode right now so it might be a good time to address some of the less efficient coding practices now.

1 Like