Strange warning from Face.vertices

kwalkerman

I have the following code:


lowest_face = nil
lowest_z = nil
entities = group.entities
entities.each do |entity|
	if(entity.is_a?(Sketchup;;Face))
		z_avg = 0
		n = 0
		entity.vertices.each do |v|
			z_avg += v.position.z
			n+=1
		end
		z_avg = z_avg/n
		if(lowest_z == nil || lowest_z > z_avg)
			lowest_z = z_avg
			lowest_face = entity
		end
	end
end

I have run this code many times with no problems, but in the scope of a larger program (after this code had been run probably 50 times), I got the following error:

undefined method `position' for #Sketchup::Edge:0xdc52548>
C:/PROGRA~1/Google/GOOGLE~2/Plugins/work/entity_functions.rb:518

line 518 is the "z_avg += v.position.z

any ideas??? Face.vertices should never return an edge - right?

--
Karen

kwalkerman

in addition, I just figured out which group caused the error, and was able to run the code directly on the group, with no errors... I'm truly stumped

--
karen

thomthom

That truly is puzzling.

I've been working a lot with vertices these past six months and I've never seen vertices return anything else than vertices.

Does it occur with any model? Do you have a model/snippet where you can reliably reproduce this?

Only thing I'd dare to guess is mixup of reused variables? Hard to tell without more of the code.

Chris Fullmer

any chance another plugin is messing it up somehow?

kwalkerman

yeah, really puzzling. The thing is, we are starting with a face, because

if(ent.is_a?(Sketchup::Face))

returned true.

And no, I can't reliably produce it. It only happened once, which makes it difficult to diagnose. I'm thinking about just double checking to make sure that v is a vertex before asking for the position. But that shouldn't be necessary!

I can't imagine that a plugin is messing with things, but I haven't really discovered how they mess with each other yet.

--
Karen

thomthom

@kwalkerman said:

I'm thinking about just double checking to make sure that v is a vertex before asking for the position. But that shouldn't be necessary!

True, it's be treating the symptom and not the cause.

I'd recheck the variables, and the scope of the code. Might have been an obscure thing. I've sometimes been stuck for hours on an problem at resolved itself when I restarted SketchUp, as I was getting some bugs from old code still in memory after reloading.

kwalkerman

true. I was stuck for a few hours the other day because I wrote:

if(i = 0)
do this
else
do that
end

but this one... I have no idea.

Thanks for all the input. I'll restart the program.

--
Karen

Dan Rathbun

A few things:
(1)
z_avg = 0 n = 0

n is obviously a increment integer, but z_avg is likely to be a Float after the first loop.

I'd declare z_avg = 0.0 to start with, and also try starting with n = 0.0, and increment n within the loop as n+=1.0. Ie: keep every thing as Float.

Do some experiements as the console, dividing Integer and Float, and visa versa. You'll see some weirdness. Ex:
4/5

0
4.0/5.0
0.8

(2) entity.is_a?(Sketchup::Face)

The .is_a? method, is an alias for .kind_of?. Internally it is loop method, that checks if the object "is subclass of" OR "is the class of", the argument. But class Sketchup::Face does not have any subclasses. I'd change it to:
entity.class==(Sketchup::Face)

(3) I'll agree with thomthom, it's likely corruption. Your set got changed (behind your back so to speak,) while you were iterating. To prevent that, don't use the C++ collections directly; make Ruby Array copies, and iterate them instead. ie:
entities**.to_a**.each do |entity|
Face.vertices already returns an Array.
You could try to use a rescue clause:


    begin
      entity.vertices.each do |v|
         z_avg += v.position.z
         n+=1
      end
    rescue NoMethodError
      # recover
      retry
    end

Dan Rathbun

@dan rathbun said:

(2) entity.is_a?(Sketchup::Face)

Another option instead of using .is_a? or .kind_of?, (that may read nicer in the code, and make sense,) is:
entity**.instance_of?**(Sketchup::Face)

Warning however, don't use .instance_of? for Integer Numerics, because it will always be false. Integer (subclass of Numeric,) are kept in Ruby as either Integer subclasses: Fixnum or Bignum, and switched back and forth (between the 2 subclasses,) automatically by Ruby; depending on the platform. The line between the 2 is likely higher on a 64bit platform; definately lower on the old 8 and 16 bit platforms.
So for Integer always use .is_a? which returns true for both it's subclasses (Fixnum or Bignum.)

Float in standard Ruby shouldn't matter, as it has no subclasses, BUT it does matter in Sketchup Ruby, because Float has been subclassed (Length).
Sketchup::Vertex.position returns a Geom::Point3d class.
Geom::Point3d.z returns a Length class.
So for Float it's safer to always use .is_a?(Float) which will return true for it's subclass Length also.

TIG

Also you could add into the do loop, at the start
next if v.class!=Sketchup::Vertex
so that it only processes vertices and any spurious edges in the array are skipped...
can you add a temporary puts entity.vertices into the mix so you can read in the Ruby Console if the array has go more than vertices in it...

It's impossible to reproduce at my end...

kwalkerman

Dan,

Wow! Lots of useful information here. I'll definitely make the changes you suggested. I was using "is_a?" instead of "class" because I read somewhere that is_a? is faster to execute, is it not?.

TIG - also good advice. I'm trying not to debug this with "puts entity.vertices" because the error only occurred in the scope of much larger code. Printing to the console seems to slow things down considerably. I could print if v.class!=Sketchup::Vertex

Thanks!!

--
karen

Dan Rathbun

@kwalkerman said:

I was using "is_a?" instead of "class" because I read somewhere that is_a? is faster to execute, is it not?.

NO. A bunch of ==class comparisons can happen inside the "is_a?/kind_of?" method block because it 'walks' the class heirarchy checking the class argument and ALL it's subclasses.

IF you know the argument class has no subclasses, you might as well just use ==class.

I think you are confusing the DONT/DO item, in the Optimization thread.
DON'T: if entity.typename==('Face')
Because it's slow String comparison.
DO: if entity.is_a?(Sketchup::Face)
or if entity.class==(Sketchup::Face)
proven in test cases to be much faster.

I DO use is_a? (but I prefer the main name kind_of?,) because it reminds me that it checks a class family. If I want to filter only by things that can be 'drawn' I would use entity.kind_of?(Sketchup::Drawingelement) which would be false for say Layer, ShadowInfo and Behavior entities.

kwalkerman

cool. thanks Dan.

Dan Rathbun

@tig said:

Also you could add into the do loop, at the start
next if v.class!=Sketchup::Vertex

weirdly Vertex is shown in the API under the Geom module.
(I put a note at bottom of API page, noting it's **Sketchup::Vertex** and not %(#000000)[**Geom::Vertex**])

AdamB

You can guard against not getting what you expect all you like, but once you get this kind of result, the internal structure of SU is toast.

It always seems to comes down to Ruby coding errors on your part. I believe the issue is that (possibly for performance reasons?) SU isn't at all defensive in its API. You give it a bunch of crap and it dutifully carries on using it resulting in increasingly wrong behaviour.

So if you ever enumerate winged edge structres etc and get crazy objects, don't guard against it, go back and find your mistake - and it can be a real PITA to find..

Adam