SketchUp / sketchup-shapes

Shapes adds new tools for drawing more shapes and primitives in SketchUp.
http://extensions.sketchup.com/content/shapes
MIT License
26 stars 13 forks source link

Parametric class used by multiple extensions #41

Open DanRathbun opened 8 years ago

DanRathbun commented 8 years ago

The Parametric class is used by multiple extensions. su_windows su_shapes

Making some comparisons, it looks like the class definition in SU_Shapes has been "getting the most love."

It has a bunch of "TODO" comments by Thomas thoughout.

Should it be moved to a CommunityLibrary module ? ... and be referenced by the other extensions in some way ?

Or should SU_Windows become dependent upon SU_shapes being installed and the Parametric class defined and maintained in the shapes extension ?

There are some coders that are wanting to mixin this parametric functionality into their own modules and classes.

johnwmcc commented 8 years ago

It's also used by my regular Polyhedra plugin (on SketchUcation).

And one thing I found out just recently, when I loaded both into SU 2016, is that you get two copies of the Edit in the context menu when you R-click on a parametric component - one from each of the copies of 'parametric.rb'. And both work on either kind of shape - 3D Shapes or Polyhedra!

I'm not a good enough coder to work out how to fix that with conditional code. I've sorted it for my own use merely by commenting out the Context menu code from one of the two plugins.

And as noted in issue 30 here, the way parametric.rb is coded means that you don't get the parameters returned in the same order for editing as you do when setting up the shape in the first place. Again, I'm not able to see how to fix that. Something to do with using hashes rather than an array, I think Thomas suggested, but he didn't have time to fix it then, and it has languished since.

DanRathbun commented 8 years ago

And one thing I found out just recently, when I loaded both into SU 2016, is that you get two copies of the Edit in the context menu when you R-click on a parametric component - one from each of the copies of 'parametric.rb'.

Hmmm..., the one in Shapes uses a global variable $parametic_loaded wrapped around the context menu handler block, so that should have only put one "Edit" command on the menu.

Did you use the same global reference ?

(Ordinarily I am against using globals but this looks like a good case for it, until a common mixin library can be implemented.)

johnwmcc commented 8 years ago

This is the relevant section of the code in parametric.rb in my Polyhedra plugin

  # Add a context menu handler that will add a menu choice to a context menu
  # for editing parametric objects

#### These lines commented out for personal use to avoid duplicate context menu entries when 3D Shapes (JWM) is also loaded
#   unless file_loaded?(__FILE__) # If not, create menu entries
#     UI.add_context_menu_handler do |menu|
#       klass_name = Parametric.selection_parametric?
#       if klass_name
#         short_klass_name = klass_name.split("::").last
#         menu.add_separator
#         menu.add_item("Edit #{short_klass_name}") { Parametric.edit_selection }
#       end
#     end
#     file_loaded(__FILE__)
#   end

end # module CommunityExtensions::Shapes

Looks as if I didn't get the latest version from Thomas's updates in SU Shapes. No global variable here.

This is the corresponding code from SU Shapes


  # Add a context menu handler that will add a menu choice to a context menu
  # for editing parametric objects
  if (not $parametric_loaded)
    $parametric_loaded = true

    UI.add_context_menu_handler do |menu|
      klass_name = Parametric.selection_parametric?
      if klass_name
        short_klass_name = klass_name.split("::").last
        menu.add_separator
        menu.add_item("Edit #{short_klass_name}") { Parametric.edit_selection }
      end
    end
  end

Looks as if a short term improvement would be for me to copy the SU Shapes parametric.rb into my JWM Polyhedra program.

DanRathbun commented 8 years ago

Looks as if I didn't get the latest version from Thomas's updates in SU Shapes. No global variable here.

Which is the pitfall when a library does not have it's own home, and must be copied. Also wastes memory.

Looks as if a short term improvement would be for me to copy the SU Shapes parametric.rb into my JWM Polyhedra program.

Yes and no.

It is wrapped within someone else's module. And even worse, it (as a class library) is NOT versioned. So how can anyone control which version gets loaded. If you have a later version, but a user has not updated their "Shapes" extension, it's older version will overwrite the newer version.

So the idea stated at the top of "parametric.rb", that other extensions should just subclass CommunityExtensions::Shapes::Parametric [u]or[/u] Sketchup::Samples::Parameteric will not work well as implemented.

Your short term solution, is to copy the class into your own library (or plugin) module. But also this way you can modify or fix the Parametric class. As it doesn't seem it get done that often for most Trimble plugins.

# file: "JWM_Parametric"
module JWM
  module Library
    class Parametric
      # copied from "parametric.rb"
      # (also copy the license info)
    end
  end
end
# file: some plugin "ThreeDWireFrames.rb"
require "JWM_Parametric"
module JWM
  module ThreeDWireFrames
    class ParametricHelix < Library::Parametric
      # object specific overrides
    end
    # other plugin code, etc.
  end
end # module JWM

But then you setup the same "multiple library copy situation" in miniature, where you yourself may have multiple extensions that need to use a common library.