Issue with unused definitions

I am having an issue with some definitions that I don’t understand, and hope you can help me explaining why this is happening. I want to extract the centroids of every instance in the model. This is my code:

model = Sketchup.active_model
coord_hash = {}

model.definitions.each do |cd|
    name = cd.name
    n_instances = cd.count_instances
    n_used_instances = cd.count_used_instances
    next if n_instances.zero?

    cd.instances.each do |ci|
        coord = ci.bounds.center
        v = ci.visible?
        coord_hash[ci.name] = coord
    end
end

According to the API doc, count_used_instances counts the total number of component instances in a model using the current component definition, so theoretically, n_used_instances must be always major than n_instances.

Most of the cases are fine, but for some definitions this is not true. It seems that there are some instances for specific definitions that are counted with count_instances and not with count_used_instances. These definitions don’t appear in the Outliner list, and when I check the visibility in the code: v = true. If I go to the 3D point, there is nothing there.

For example, in one iteration of the loop:

    name = cd.name     # name = '6520_Faucet_CH'
    n_instances = cd.count_instances     # n_instances = 1
    n_used_instances = cd.count_used_instances     # n_used_instances = 0
    next if n_instances.zero?

    cd.instances.each do |ci|
        coord = ci.bounds.center     # coord = (-0,001793", -0,027353", 4,271654")
        v = ci.visible?     # v = true
        coord_hash[ci.name] = coord
    end

Of course there is nothing in that 3D point (very close to origin, btw). It’s like a component has been imported and then erased, but somehow the info is still there, but not in the Outliner list or using the count_used_instances method.

FYI, I did not create the model so I don’t know where these unused definitions come from, but they shouldn’t be in the model.definitions list.

Apart from that, I have another question. How to iterate through all instances of a definition in the full hierarchy? Shoudn’t be a method in the API called cd.used_instances or so?

I think you have the interpretation of the methods reversed (the documentation doesn’t exactly help!). #count_instances returns the total number of instances anywhere in the model, including nested inside components that themselves have no instances in the model. #count_used_instances counts the number of instances that are actually in use in the model vs sort of “pending” inside other unused components.

2 Likes

This topic title is misleading. You are checking for hidden instances not definitions.
Hidden definitions really means “unlisted” in the component “In Model” collection.

The instances are not hidden, because the visible? flag is true, but still do not show up in the model. Also if we hide an instance it will be counted in both methods count_instances and count_used_instances, and this is not the case.

I have just realized there is a purge_unused method for definitions, and I think this can help me. Maybe if I get rid of those “unused” definitions I can find a solution.

You are right, hidden is not the right word, unused is a better appellation. Unlisted is not correct because they actually are listed in the model DefinitionList. I already edited the title.

1 Like

They don’t show up on the model because they are nested inside another component that has no instances in the model. That’s a separate question from whether their hidden flag is set or they use a layer that is hidden.

If definition A has only 1 instance and that instance of definition A is inserted into definition B, but definition B has no instances that are instanced in the model’s entities (or any of it’s nested components,) then definition A is still counted for count_instances() but not for count_used_instances().

So purge_unused() will not help here.

It might help. You can use a refinement until there is an API method defined …

# assumes this is defined within module Finfa
#  or nested within some other namespace module ...
module GetUsedInstances
  refine ::Sketchup::ComponentDefinition do
    def get_used_instances()
      if count_used_instances() == 0 ||
         count_used_instances() == count_instances()
        return instances()
      end
      instances().reject do |inst|
        if inst.parent.is_a?(Sketchup::Model) && inst.parent == model()
          next false
        else # it's a Sketchup::ComponentDefinition
          inst.parent.count_used_instances == 0
        end
      end
    end
  end # refine overlay module block
end # refinement library module

# Then elsewhere in the code ... use the refinement.
# Must be at the top of a file, at top level [outside all modules]
#  IF SketchUp version uses Ruby 2.0.0 / ie, v2014..2016.
# Later Ruby versions define Module#using allowing refinements 
# inside modules and classes. (But it still applies only to the file,
# and is not automatically 're-used' if the module or class is re-opened
# later in another file.) 
using GetUsedInstances
# or Finfa::GetUsedInstances or Finfa::Refines::GetUsedInstances, ...
# ... etc., ... Ie, use namespace qualification as needed.

# cd is a reference to an existing definition ...
ui = cd.get_used_instances
1 Like

Using this refinement now I am able to localize the used instances and filter them properly.

Thank you for helping me understand the concepts of count_used_instances and count_instances. As @slbaumgartner said, the documentation does not exactly help.

I’d be cautious about using refinements.

  1. If we add the method you’ll run into conflicts.
  2. We run automated analysis tools on extensions shipped to Extension Warehouse - making sure they don’t modify the API namespace. I’m not sure we can reliably tell the difference between that and refinements. It’s a good chance you’d be flagged and rejected.
  3. This is doable with standalone methods that takes the definition as a parameter - safe from any potential conflicts.

Thank you @tt_su for your advice.

Regarding point 1, is it planned to add the method in short/mid-term?. In the meantime I can use this refinement or take the idea of point 3. Point 2 is not a problem in my case.

This was submitted as a feature request yesterday. It haven’t even been scrubbed - so I cannot say anything to whether it will be added or not.

In general, the functionality of this proposed method can be achieved with existing API. So it’ll rank a lot lower than other issues where the functionality is blocked.

Also, golden rule of an API is that it should be minimal. This type of method might fit better in a utility library.

Using refinements is using someone else’s namespace. That automatically expose you to a number of challenges. All these challenges are omitted if you simply take the object as a parameter.

There is another concern about refinements, someone less experience with refinements might not fully understand them, and focus on the part of the code that modifies the API namespace. Modification of API namespace have been a long standing issue we’ve been working very hard on eliminating. I fear refinements might have a negative effect in that respect.

Taking an extra parameter is the safe, pragmatic way to go.

For reference to whoever else might be reading, here’s the same code refactored into taking an extra param:

module Example
    # @param [Sketchup::ComponentDefinition]  definition
    # @return [Array<Sketchup::ComponentInstance, Sketchup::Group, Sketchup::Image>]
    def self.get_used_instances(definition)
      if definition.count_used_instances == 0 ||
         definition.count_used_instances == definition.count_instances
        return definition.instances
      end
      definition.instances.reject do |inst|
        if inst.parent.is_a?(Sketchup::Model) && inst.parent == definition.model
          next false
        else # it's a Sketchup::ComponentDefinition
          inst.parent.count_used_instances == 0
        end
      end
    end
end # module
2 Likes

Moved response to new topic thread (as debating use of Ruby refinements is off-topic) …

But may be slower running in Ruby.

I do not avoid filing request(s) if the added feature will enhance the API, or I believe that the feature should have already been implemented. Usually complex filtering and searching collection objects are good candidates for C-side implementation, when Ruby-side is likely to be either slow or cumbersome.

See: Golden Rules of API Design (silde 18) - “Don’t offload complexity onto the API enduser”


In this case, I think that this proposed method should have been implemented when the count_used_instances() method was implemented. At that time, the implementer should have put on the “user hat” and ask what else would the users need or want in this regard ?

See: 5 Golden Rules for Great Web API Design: Perspective

Well if the count_instances() has a instances() complimentary method, wouldn’t it be obvious that when implementing a count_used_instances() that someone is going to need it’s array compliment used_instances() method ? (It’s a “no brainer.”)

Boolean #used? and #not_used? methods are also obvious.

Since you moved the refinement debate to another thread, I would like to share with you a scenario where the proposed solution does not work. Let’s say I import a Component (a shower tray) with a Group inside (a shower drain), and I copy it thrice in the model:


outliner

If I run the code:

The shower drain is a Group because it was created with the Substract option of the Solid Tools toolbar, and that is why it’s called Difference (by default). The definition of these three groups is common because it has been copied.

In this case n_used_instances = 3 and n_instances = 1, so the get_used_instances function won’t work correctly. Besides, if I retrieve the centroid of that single counted instance (-21,18", -10,98", 1,55"), the point is in the red mark below:

coordinate

which actually makes no sense.

Do you know why this is happening and if it’s possible to retrieve the centroid of the three shower drains, which basically are copied Groups of the same definition?

The centroid issue is because the value is returned in the coordinates of the definition in which the drain is nested, but you are displaying it in the model coordinates.

(1) Please do not post images of code, post actual code text properly so we can scroll it and copy and paste it, edit it etc. (Please also use carriage returns so that we do not need to scroll horizontally.)

(2) When you have a test model that fails, please post the test model.


Really, because that 1 instance is within 1 definition (Shower Tray) that has 3 instances.

Which is correct.

I think you’ve had incorrect expectations.

In reality, there is only 1 instance of the drain. If you double-click into any of the 3 Shower Tray instances (you’ll then be in the definition’s entities context,) and you can examine the PID of the drain. Repeat for the other tray instances, comparing the PID, and you’ll realize that the drain is the same exact object in all 3.

What is being done here for count_used_instances is a hierarchy walking product that multiples the real uses with wrapper definition’s uses.

This kind of unreal expectation is why Thomas is questioning (in the GitHub issue tracker) whether this method should really return an array of InstancePath arrays, because if you expected an array of 3 drain objects, they’d all 3 be the same instance object. Which really will not help all that much.


Conclusion. The method I gave above is actually working correctly.
There is only one drain group instance, even though it is “used” 3 times by virtue of it’s parent definition having 3 instances.

Your mistake is assuming that the “used” array will have the same number of members as the “used” count. This is not true in all cases, but may be true in some.


However, Thomas’ example above has a error.
In his edition, the call to the model method needs to be: definition.model

1 Like

Thank you @DanRathbun for the extended response, now everything is pretty clear about the count_used_instances method.

@slbaumgartner, regading the coordinates issue, the next thread helped me to perform the conversion from local to global coordinates in nested elements.

Finally I could manage to retrieve the global coordinates of the centroids of every used instance in a model. Thank you all.

1 Like

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.