CleanUp doesn't respect the lock flag

…which is wrong! Booh (And quite dangerous for my own plugin)

Here is an interesting reading about this issue : The Locked property is just a flag | Procrastinators Revolt!
:wink:

@thomthom @tt_su

Haha! Yea - you busted me with my own words there. hangs head down in shame

I have a long standing issue for that: https://bitbucket.org/thomthom/cleanup/issue/2/locked-groups-components-are-cleaned-up

The issue is that CleanUp need a refactor to respect the locked flag - at least when you process the whole model because then it traverse all the definitions and the model root only - it doesn’t know which definition has an instance locked or contained in another locked instance. Once has to traverse the whole model tree to map that out.

Yeah, I’d imagine that if you didn’t do it already, it’s probably because it needs quite some work.

But I wanted to ping you on the subject, since this can be a pretty destructive plugin if the user doesn’t pay attention.
(In my case, the “Remove stray edges” is the real danger)

Since we’re on the subject, being able to lock layers/definitions/materials from purging would be great.

1 Like

Yea, I burned myself on that often in my last job - with imported DWG drawings which I used as reference for modelling… :frowning:

So I had an idea that might avoid a major rewrite - make a pre-pass on the model before processing:

module Example

  def self.locked_definitions(model)
    locked = {}
    self.find_locked_definitions(model.entities, locked)
    locked.keys
  end

  def self.find_locked_definitions(entities, locked)
    entities.each { |entity|
      next unless entity.is_a?(Sketchup::ComponentInstance) ||
                  entity.is_a?(Sketchup::Group)
      definition = self.definition(entity)
      next if locked.key?(definition)
      if entity.locked?
        locked[definition] = true
        self.mark_sub_instances_locked(definition.entities, locked)
      else
        self.find_locked_definitions(definition.entities, locked)
      end
    }
    nil
  end

  def self.mark_sub_instances_locked(entities, locked)
    entities.each { |entity|
      next unless entity.is_a?(Sketchup::ComponentInstance) ||
                  entity.is_a?(Sketchup::Group)
      definition = self.definition(entity)
      next if locked.key?(definition)
      locked[definition] = true
      self.mark_sub_instances_locked(self.definition(entity).entities, locked)
    }
    nil
  end

  def self.definition(instance)
    instance.definition
  end

end # module


=begin
time_start = Time.now
Example.locked_definitions(Sketchup.active_model)
puts Time.now - time_start
=end

I’ll try to hook that up in CleanUp and see how it fares.

Well, it was far from that trivial. Had to dig deep into the code. Think I have a working solution. But I’d like to get this tested before I release. Would you be willing to take it for a spin on a few models? (@jiminybillybob)

Sure! Please PM it to me.