CleanUp doesn't respect the lock flag


#1

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

Here is an interesting reading about this issue : http://www.thomthom.net/thoughts/2012/10/the-locked-property-is-just-a-flag/
:wink:

@thomthom @tt_su


#2

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.


#3

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)


#4

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


#5

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


#6

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.


#7

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)


#8

Sure! Please PM it to me.