The truss assembly is wrapped in a component, but all the geometry is loose inside. So my approach was to identify the plate faces (and edges) according to the unique material that is used on them. In this case, the material name is <auto>4.
model = Sketchup.active_model
ents = model.entities
ents.each do |entity|
model.start_operation('Delete Truss Plates', true)
if entity.is_a?(Sketchup::Group) || entity.is_a?(Sketchup::ComponentInstance)
objectDef = entity.definition
facesToDelete = []
objectDef.entities.each do |ents|
if ents.is_a?(Sketchup::Face) && ents.material && ents.material.display_name=="<auto>4"
allEnts = ents.all_connected #grab the edges too
facesToDelete = facesToDelete + allEnts #add entities to array to delete later
end
end
if facesToDelete.length > 0
facesToDelete.each do |thing|
puts "erased" + thing.entityID #testing
thing.erase! #error here??
end
end
end
facesToDelete = [] #empty the array??
model.commit_operation
end
I’m getting an error: Code failed with an error: TypeError: reference to deleted Entity
I can’t figure out why, but it seems like somehow an entity inside the facesToDelete array is trying to be referenced/deleted after it has been deleted. I’ve tried emptying the array after each loop (even though I’m just testing it on a single group at the moment). Stuck at the moment. Any ideas?
If I comment out the thing.erase! line, it works fine.
I’ve output all entities and checked to see if somehow the entities were being duplicated in my array, and they aren’t. So I’m not sure how it could be trying to delete the same entity twice.
I’m starting a loop where I loop through all entities in the group/component.
But then in each loop, I’m grabbing all_connected to the face, which in some cases grabs entities that I haven’t looped through yet. So when the next loop tries to access that entity, I’ve already deleted it…
EDIT: NOPE! Alright, I really think I figured it out now. When I create the array that contains all four edges and the face, and loop through to delete each one, the face actually gets deleted automatically as soon as I delete one of the edges (duh). So when the loop gets back to the face entity and tries to delete it, it gives an error because it was already deleted.
I’m thinking instead of using all_connected, I should use edges instead, to just erase the edges of the face.
model = Sketchup.active_model
ents = model.entities
ents.each do |entity|
model.start_operation('Delete Truss Plates', true)
if entity.is_a?(Sketchup::Group) || entity.is_a?(Sketchup::ComponentInstance)
objectDef = entity.definition
objectDef.entities.each do |ents|
if ents.is_a?(Sketchup::Face) && ents.material && ents.material.display_name=="<auto>4"
allEnts = ents.edges
allEnts.each do |thing|
puts thing.entityID
thing.erase!
end
end
end
end
model.commit_operation
end
Don’t iterate an entities collection directly…
Use entities.to_a to ‘freeze’ the list, otherwise the entities change as you delete things and get into a tangle… as you’ve seen
Also keep your ‘start/commit operation’ outside of the main processing loop, otherwise it’s split into myriad parts based on the total entities length, not one single ‘undo’ step !
You also use ‘ents’ referring to the entities, but reuse ‘ents’ in the checking/erasing loop - use something different, it’ll probably cause confusion if nothing else - try ‘ent’ not ‘ents’…
Here’s my own reworking - note it’s untested…
model = Sketchup.active_model
ents = model.entities
model.start_operation('Delete Truss Plates', true)
ents.to_a.each{|entity|
if entity.is_a?(Sketchup::Group) || entity.is_a?(Sketchup::ComponentInstance)
defn = entity.definition
defn.entities.grep(Sketchup::Face).each{|face|
if face.material && face.material.display_name == "<auto>4"
edges = face.edges
edges.each{|e|
#puts e.entityID # why?
e.erase!
}
end
}
end
}
model.commit_operation
model = Sketchup.active_model
ents = model.entities
insts = ents.find_all {|e| e.respond_to?(:definition) }
cdefs = insts.map(&:definition)
marked = {}
cdefs.each do |cdef|
ents = cdef.entities
ents.grep(Sketchup::Face) do |f|
if f.material && f.material.display_name == "<auto>4"
marked[ents]= [f] + f.edges
end
end
end
unless marked.empty?
model.start_operation('Delete Truss Plates', true)
marked.each do |ents,ary|
ents.erase_entities(ary)
end
model.commit_operation
end
Note how it collects the “marked” elements to be deleted in a hash, then afterward walks that hash and uses the bulk eraser method to do the deletions.
It also puts the delete operation within a conditional if block so that it will not create an unneeded operation if it does not find anything to delete.
I was just doing this because I had originally thought I was somehow capturing the same entity more than once inside my array, causing the error. So I wanted to output the ID for testing so I could compare and see if there were any duplicates.
Right. That’s why in my original code I used facesToDelete = [] to collect the entities inside an array first, before looping through the array to delete them. Same thing, right? I know my “solution code” didn’t do this, but that was the idea I had all along because I did remember that.
My fundamental error was this:
Because with all_connected, it includes the face too. But once you delete one edge bounding a face, the face automatically is deleted. So when I tried to delete the face directly in the loop, it gave an error because it was already gone.
Side note to this, it’s faster to erase entities in bulk than one by one.
# This is faster...
entities.erase_entities(array_of_entities)
# ...than this:
array_of_entities.each { |entity| entity.erase! }
Similarly with adding/removing items from the selection. Collect the changes in a temporary array and do it in bulk. When the API is given a set of things to work on it’s possible for it to optimize the operation. When it’s being given one thing at a time it will cause a whole bunch of other things to refresh for every single item.
Using entities.erase_entities would also take care of this scenario for you:
Yes, similar, but as Thomas says, the API prefers bulk methods. (I think that the erase_entities internals will ignore any DeletedEntity objects? @tt_su correct me if I’m off here.)
Even if I did not (in my example) explicitly use an operation, I believe the bulk method call would result in one native undo operation. (The benefit of coding the operation is that a custom name can be given rather than the vague “Undo Delete”.)
Your singular erases without an explicit operation would make the user do multiple undos, one for each deleted element.
Differenced only in that your example uses an array of “marked” elements, whereas mine needs a hash of arrays so that the entities collection objects can be used with the bulk erase method.
Uh huh, but notice in my example I didn’t use this and instead made sure to have the face as the first item in the “marked” arrays so it was erased first.
I see, so marked[ents]= [f] + f.edges you’re including the face as f and the edges after that using f.edges. I suppose this is better than using all_connected in case the face happens to be connected to other loose entities, but is there a benefit to including the face at all? Since the face automatically gets deleted when the bounding edges are erased?
It figures, I saw that #erase_entities method in the API docs but chose to try #erase! instead, lol. Looks like that’s what @DanRathbun chose to use too.
For what it’s worth, I ran the script on each floor, (I ended up tweaking it so it would run on the selection, instead of the entire model), I did one floor at a time, thinking it would bog down. But it only took 2-3 seconds on each floor to cycle through about 750 unique components and remove 50+ “plates” from each component. Probably could’ve done the whole model in one go. I’m sure it would be even faster using the #erase_entities method.
Also, for context, the main reason for doing this (aside from purging geometry I don’t need), is all the plates were exported wrong and for some reason were all on the first floor, even when the truss was above.) The truss manufacturer couldn’t figure out how to fix it.
Most importantly, use model.start_operation('Operation name', true') to maximize speed. Without that you will experience the UI updating and slowing things down.
I logged an issue to add notes in the documentation that hopefully will guide users to choose faster alternatives when applicable.
J/K