Video captured a SketchUp BUG when undoing erased entities at 11:30 minute mark

Hi, I was recording a video of my workflow before starting to code while also showing an extension I am currently working on.

And, at the 11:30 minute mark, you’ll see a BUG that happened after I used ‘undo’ for bringing back previously deleted entities. When I tried deleting them again it didn’t allow me and I could not select them properly either.

Below is the code of a module I am including on my extension for erasing selected entities.

I hope I can get some feedback on what I am doing wrong.

# frozen_string_literal: true

# ------------------------------------------------------------------------------
module Renderiza
  module Grabby
    # # #
    module Eraser
      # # #
      def erase_selection
        model = Sketchup.active_model
        entities = model.active_entities
        selection = model.selection

        return unless selection.length >= 1

        faces = selection.grep(Sketchup::Face)

        if faces.length >= 1
          face_edges = []
          faces.each do |face|
            face_edges << face
            face_edges << face.edges
          end
          face_edges.flatten!
          # ------------------------------------------ START
          model.start_operation('Eraser', true)
          group = model.entities.add_group(face_edges)
          group.erase!
          entities.erase_entities(selection)
          model.commit_operation
          # ------------------------------------------ END
        else
          entities.erase_entities(selection)
        end
      end
    end
  end
end

Thanks in advance and happy Mother’s day!

Looking at the Ruby Console I am getting this error…

Error: #<ArgumentError: All Entities must have a common parent>

  • To get the error you need to select faces inside a group while being outside the group with the Extension.
  • Then right-click and choose eraser to delete the faces
  • Click undo
  • You will encounter a bug when trying to erase faces again

I have made the Extension public in my GitHub for you to analyze code"

https://github.com/Renderiza/Grabby

I hope to get feedback.

Oh, yea, I’m going to give ya’ feedback right here …

This is one reason why we say to always work with the model.active_entities like the user must do via the GUI.

So HOW would an end user (or tester) be able to create a selection outside the active entities ?

Why would anyone wish to do this when the Delete key … or the native context-click > Erase will serve just fine in such a situation ?


Looking at your code … I see convoluted workflow.

if faces.length >= 1 is calling two methods, …
… why not call 1 boolean method, ex: unless faces.empty? or if !faces.empty?

It looks like you want faces and edges, but not objects (groups, components, dimensions, section planes, etc.)
so …

faces = selection.grep(Sketchup::Face)

        if faces.length >= 1
          face_edges = []
          faces.each do |face|
            face_edges << face
            face_edges << face.edges
          end
          face_edges.flatten!

… could be …

primitives = selection.find_all { |element|
  element.is_a?(Sketchup::Face) || element.is_a?(Sketchup::Edge) 
}

… or even this …

primitives = selection.grep(Sketchup::Face)
primitives += selection.grep(Sketchup::Edge)

We’ve told everyone many, many, many times (especially TIG,) that you cannot take entities from one context and try and stuff them into a group in another context using #add_group.

So your posted code is an excellent monument to how NOT to code, and is not a bug.
The error message was added because in previous versions, doing this would crash the SketchUp application.

So, in my opinion, worrying about anything beyond the erroneous #add_group call is a … waste of time.

Is your end goal to delete entities that do not bind other entities? If so you can achieve that just by checking the relation between entities without having to rely on the behavior in make group.

I haven’t tried this code myself and it may have some errors but hopefully it helps you get in the right direction.

# Erase entities, excluding edges binding other faces.
# Similar selection behavior as when grouping existing geometry.
#
# @param entities Array<Sketchup::DrawingElement>
def soft_erase(entities)
  entities = entities.reject do |entity|
    next unless entity.is_a?(Sketchup::Edge)
    !(entity.faces - entities).empty?
  end
  entities.first.parent.entities.erase_entities(entities)
end

In the rare cases when the best approach is to use something in another way it was made to be used, I’d strongly recommend to comment the code with what it is doing on a higher level, and why it uses such approach.

1 Like

I have redone the code and now I don’t get bug after undo.
The .gif image gives you an example of the code running.

The goal of the code below is to create an eraser method that will delete selected faces along with their edges, even if the edges are not in the current selection.

The logic of the algorithm goes as follows.

If faces are selected, then we want to create an array of face_edges. Then we erase selected faces, leaving us with some loose edges in array face_edges, which will be deleted next.

For that, we iterate the edges and create an array named ‘del_edges’ containing the edges which don’t have any faces connected to it. Finally, we delete these edges without faces.

# frozen_string_literal: true

# ------------------------------------------------------------------------------
module Renderiza
  module Grabby
    # # #
    module Eraser
      # # #
      def erase_selection
        model = Sketchup.active_model
        entities = model.active_entities
        selection = model.selection

        return unless !selection.empty?

        faces = selection.grep(Sketchup::Face)

        model.start_operation('Eraser', true)
        # ---------------------------------------------------------------- START
        if !faces.empty?
          # The goal of the code below is to create an eraser method
          # that will delete selected faces along with their edges, even
          # if the edges are not in the current selection.

          # The logic of the algorithm goes as follows...

          # If faces are selected, then we want to create an array of
          # face_edges. Then we erase selected faces, leaving us with some
          # loose edges in array face_edges, which will be deleted next.

          # For that, we iterate the edges and create an array
          # named 'del_edges' containing the edges which don't have any faces
          # connected to it.  Finally, we delete these edges without faces.
          face_edges = []
          faces.each { |face| face_edges << face.edges }
          face_edges.flatten!

          # Important to erase faces now after we stored face edges in the array.
          entities.erase_entities(faces)

          # After selected faces are erased we expect some edges without face.
          # An array name del_edges is created for storing the faceless edges.
          del_edges = []
          face_edges.each { |edge| del_edges << edge if edge.faces.empty? }

          entities.erase_entities(del_edges)
        end
        # After we deal with faces any entities left still selected
        # will be erased to complete our eraser method.
        entities.erase_entities(selection)
        # ------------------------------------------------------------------ END
        model.commit_operation
      end
    end
  end
end

@eneroth3 I tried your code, but it is not working for what I want it to do, maybe I am testing it wrong. The faces are getting deleted but not the edges.
Also, I tried your suggestion about commenting my code.

@DanRathbun thanks for the input, but I am not sure you understand what I want to do.
However, I did use some of your suggestions and I understood some of what not to do in code.

1 Like

I have a feature request for you, that you may already being doing, or thought about it.

With the eraser tool you can run over a lot of lines, and if you go over one that you didn’t mean to, you can hold a modifier key down to Deselect. Command on Mac, Alt on Windows. You could do something similar. Though as the erase or hide is done later, presumably someone could toggle a highlighted square too.

1 Like

I thought you had both edges and faces and wanted to skip edges holding other faces, not that you had just faces. Now I see it.

1 Like

Hi @colin,
Below you’ll find some of the keys currently implemented in the Extension. They will probably change if I find better logic for modifier keys.

You mentioned the erase tool which is very cool and I might create a dedicated eraser tool with its own toolbar icon that will activate the eraser method onKeyUp for the given entity type.

I also need to implement shortcut keys for toggling hidden geometry.

Thanks for the suggestion!

onKeyDown

Key Action
shift Activate Deselect tool until key is released.
[ Increase brush size
] Decrease brush size
1 Activate Edge Tool
2 Activate Face Tool
3 Activate Group Tool
4 Activate Component Tool

onLButtonDoubleClick

DoubleClick Action
on Empty Space - Deselect all entities
on Entity with Edge Brush - Select all connected edges .
on Entity with Face Brush - Select all connected faces .

I added my Github repository for people to download the Extension there, but maybe it is easier if I include the SketchUp Extension here.

Grabby v1.0.9.rbz (230.2 KB)

The API allows out of context selection, which can be very handy, but it should be seen as a use-on-your-own-risk behavior.

1 Like