Abort operation, in a trigger observer

Good morning,
I wrote a class that places a component instance in the model. an observer is placed on the definition, and on the model.tool
This observer on onComponentInstanceAdded initiates a series of actions like opening a form and applying attribute.

Everything works well !
But if user chooses to cancel during form, I want the new placed instance to be cleared.
I can’t close the observers correctly before doing the undo.
I don’t know if I should, and in which case how to implement a begin rescue. I’ve done lots of iterations of different code, but I can’t find a solution to this crash.

I would like a helping hand and explanations so that I understand.

module SimJoubert
  module SJ_Pins
    require 'securerandom'
    class SJDCA_Pins
      attr_accessor :name, :position, :pid, :color, :pins_name
      @@defpath = "D:/Sketchup/Composants/SJDCAnnimation/SJDCA_pins.skp"
      @@defspy = nil
      def initialize()
        @uuid = SecureRandom.uuid
        @name = "new pin"
        @color = "red"
      def add_new_pin()
        # On verifie que la définition n'est pas déja chargée dans le model sinon on la charge
        path = @@defpath
        model = Sketchup.active_model
        model.start_operation('add new pin',true,false,false)
        definitions = model.definitions
        cdef = nil
        definitions.each do |d|
          if d.path == path
            cdef = d
        if cdef == nil
          cdef = model.definitions.load(path)
        # On place un observer sur la définition quand une instance est ajouter
        if @@defspy == nil
          @@defspy = PlaceInstanceSpy.new(cdef, self)
        # On place l'instance sur le contexte courant du model

      def delete_pin(instance)
        #model = Sketchup.active_model
        return false


      def new_pin_form_setting(definition, instance, title="Define new pin",default_name="New pine name",default_color="red")
        pins_names = Sketchup.active_model.get_attribute("sjdca_pins","pins_names",[])
        puts "Get model pins_names #{pins_names}"
        # Formulaire
        colors = Sketchup::Color.names
        colors_options = colors.join("|") 
        prompts = ["Pins label", "Colors"]
        defaults = [default_name, default_color]
        list = ["",colors_options]
        input = UI.inputbox(prompts, defaults, list, )
        #Resultat formulaire
        if input == false
          #Annulation de l'opération
          puts "anulation opération"
          return false
        elsif (pins_names.include?(input[0])) == true
          title = "#{input[0]} already existe, set a uniq name"
          default_name = "New pine name must be uniq"
          default_color = input[1]
        self.new_pin_form_setting(definition, instance, title,default_name,default_color)
          @name = input[0]
          @color = input[1]
          pins_names << @name
          Sketchup.active_model.set_attribute("sjdca_pins","pins_names", pins_names)
          self.new_pin_setting(definition, instance)

      def new_pin_setting(definition, instance)  

        @pid = instance.persistent_id

        instance.name = @name       

        #Application de la matiere
        instance.material = @color
        mat = Sketchup.active_model.materials.filter{|mat|mat.name == @color }[0]
        unless mat == nil
          instance.material = mat
        # Placement de la punaise sur le calque SJDCA Pins
        layer = Sketchup.active_model.layers.filter{|lay| lay.name == "SJDCA Pins"}[0]
        if layer == nil
          Sketchup.active_model.layers.add("SJDCA Pins")
        instance.layer = layer

        #On récupère sa position
        @position = instance.transformation.origin.to_a.map{|p| p.to_m}        
        # edition du texte
        ents = instance.parent.entities
        ents_instance = instance.definition.entities
        bb = instance. Bounds
        point=Geom::Point3d.new(0, 0, (bb.depth))
        vector = Geom::Vector3d.new(0, 0, (bb.depth)/2)
        face = ents_instance.grep(Sketchup::Face).filter{|f| point.on_plane?(f.plane)==true}[0]
        instance_path = Sketchup::InstancePath.new([instance,face])
        text = ents.add_text("SJ DC Animation Pin\n#{@name}", [instance_path, point], vector)       
        text_layer = Sketchup.active_model.layers.filter{|lay| lay.name == "SJDCA Pins Label"}[0]
        if text_layer == nil
          Sketchup.active_model.layers.add("SJDCA Pins Label")
        text.layer = text_layer
        return true

    # component placement event.
		class PlaceInstanceSpy
			def initialize(definition, parent)
				@definition = definition
				@parent = parent
				# Attach this observer also to watch the tools collection
				# because the active tool changes as the placement proceeds:

			def onComponentInstanceAdded(definition, instance)
				# Set attributes on the definition or the instance here,
				# # by calling a method in the parent module:
				model = Sketchup.active_model
        model.start_operation('pin setting',true,false,false)
				status = @parent.new_pin_form_setting(definition, instance)
        if status == false
          if instance.deleted? == false
			end #onComponentInstanceAdded

      def onComponentInstanceRemoved(definition, instance)
        name = instance.set_attribute("sjdca_pins","name",nil)
        pins_names = Sketchup.active_model.get_attribute("sjdca_pins","pins_names",[])
        if (pines_names.include?(name)) == true
        if instance.deleted? == false
        puts "onComponentInstanceRemoved(#{definition}, #{instance})"
      end #fin onComponentInstanceRemoved

		end#Fin Class PlaceInstanceSpy

  end#fin module plugin
end#fin espace de nom auteur

Just a note on coding style …

        cdef = definitions.find { |d| d.path == path }
        cdef = model.definitions.load(path) unless cdef

Ie, it is not necessary to call Object#== to compare results against the nil instance as Boolean tests will evaluate a nil reference (and false) as falsey and everything else as truthy. (So, if obj is bound to be faster than if obj == nil because there is no method call.)

FYI, … the path property is not the unique identifier for the DefinitionList collection. It is the definition name property that is the collection key.

Also, hardcoding the definition load path is not very portable.

Why are you using SecureRandom::uuid() instead of #persistent_id ?


Thank you Dan for taking the time to respond to me.

The idea of the “cdef path” test is to avoid reloading the component definition a second time by testing if another has the same access path. It will be in addition to another test on the name of the definition.

Hard access is for testing. Afterwards, once the plugin is assembled, it will be a calculation.

Secure random is a belt and suspender habit from databases. In case of collaborative work, models can merge without conflict and bypasses the pid change by going from group to component instance. I don’t know if this has a significant impact on the code execution time.

I’ll let you discover the rest of the code. Looking for the source of the crash when canceling on the form input.

Thank you again for this corrective reading of the code, which I am fond of.

Good end of weekend

I asked because SecureRandom had an issue in past versions where the first time it was called, it took a long time to return. This might have been linked to bugs in the OpenSSL library.

Also, Ruby is a English programming language. If you want people to read your code, please also put comments in English. I do not feel like messing with Google translate just to know what your methods are meant to do.

1 Like

This is another problem. model.place_component cancels the current operation and starts it’s own operation.
There is an open API feature request. bug report in the GitHub tracker about this.

Also … You again begin another operation in the PlaceInstanceSpy whilst it is likely that the model.place_component native operation is in process.

So, I am not surprised you are getting a crash.

Really, the code is so convoluted I am not really interested in looking further into it.
You would probably need to code up your own instance placement tool.

1 Like

I agree to Dan that your code is so convoluted - perhaps because you are using “unusual style” (more on that in second parts of my post, and check this: https://rubystyle.guide/).

Also it is hard - impossible - to test your code because we don’t have a sample model to try…
Also dealing with Dynamic Components is hard since the Dynamic Components extension have no public source.

Also I do not have enough experience - or just a bad one - with observers. I do not understand and I do not know if it is possible to attach it as you do, and the same class can be attached as observer to tools and to definition?
Moreover, you are attaching it to tools collection, but you are not using any methods of ToolsObserver ?!?

So, I just noticed two thing that perhaps helps:

:bulb: I would not use the Sketchup.undo but the #abort_operation method.

Instead of this:

I would try this.

        model = Sketchup.active_model
        model.start_operation('pin setting',true)
        status = @parent.new_pin_form_setting(definition, instance)
        if status
          instance.erase! if instance.valid?
          #instance.erase! unless instance.deleted?


The #onComponentInstanceRemoved method is called when a component instance is removed from a model.
Why do you try to set the attribute of the instance which has been already removed?
Why do you try to erase the instance which has been already removed?
I think it is not possible to do any operation with a deleted instance. I guess you will get an error message if you try to reference to a deleted entity.

Now, some notes about your “style”:

You are using Tabs in your second part of the code, that is the reason while the forum motor rendering it strangely. Change the Tabs to space!

The Model #start_operation method last two parameters are defaults to false. You can use in this form:

model.start_operation('add new pin',true)


You can use the Enumerable #find instead, and avoid that format for comparisons to nil, e.g.

cdef = definitions.find { |d| d.path == path }
cdef = model.definitions.load(path) unless cdef

I would do like:

unless @@defspy
  @@defspy = PlaceInstanceSpy.new(cdef, self)


It is not need to compare Boolean result to other Boolean. And better to simplify like e.g.

        return false unless input
        if pins_names.include?(input[0])
          title = "#{input[0]} already existe, set a uniq name"
          default_name = "New pine name must be uniq"
          default_color = input[1]
        self.new_pin_form_setting(definition, instance, title,default_name,default_color)
          @name = input[0]
          @color = input[1]
          pins_names << @name
          Sketchup.active_model.set_attribute("sjdca_pins","pins_names", pins_names)
          self.new_pin_setting(definition, instance)

You can use the Enumerable #find instead

mat = Sketchup.active_model.materials.find { |mat| mat.name == @color }

This will give a same result:

mat = Sketchup.active_model.materials.find { |mat| mat.name == @color }
instance.material = mat || @color

The Layers #add method is used to add a new layer.
If you give the name of a Layer that is already defined, it will return the existing Layer rather than adding a new one.
The find method would have been more effective here as well than the filter you used in your code everywhere else, but even that one is not necessary, because
this will give a same result:

instance.layer = Sketchup.active_model.layers.add("SJDCA Pins")