[Info] Allowable Classes for "set_attribute"

Dan Rathbun

It's explicitly coded, though the code doesn't mention the historical reason..

OK now I understand what Christina was saying.

I agree with Jim. I can think of a situation, whee I'd save some point in the model (which is relative to the world origin, and I would be rather ticked if SketchUp applied a transform to MY attribute without MY permission.

How would / should SketchUp know what the value of my attribute was for ?

tt_su

If you want to ensure the data isn't transformed, convert the Point3d or Vector3d to an array.

Dan Rathbun

I know this. It does not negate what I said. If I store a point that is inside the group, relative to the group's origin, and move the group, ... I would not want the point to be transformed.

WHO decided that this should happen, WHEN and WHY ? (If this was for DCs, then it should only happen to attributes in the "dynamic_attributes" dictionary.)

This is the first I am hearing of this. Is it documented anywhere ?

It would be nice to specify things as absolute (world co-ords,) or relative (local co-ords.)
Thinking, what if we had subclasses: LocalPoint & WorldPoint ?

tt_su

@dan rathbun said:

I know this. It does not negate what I said. If I store a point that is inside the group, relative to the group's origin, and move the group, ... I would not want the point to be transformed.

Just saying, that is the existing behaviour. It caught me off guard as well and initially I wondered if it was a bug, but when looking at the code it was made deliberately. There are use cases for and against this.

@dan rathbun said:

WHO decided that this should happen, WHEN and WHY ? (If this was for DCs, then it should only happen to attributes in the "dynamic_attributes" dictionary.)

I don't know when, why or who. As far as I can tell this code goes way back and it doesn't mention these details.

@dan rathbun said:

This is the first I am hearing of this. Is it documented anywhere ?

It came to the attention to us (the Extensibility Team) just recently. I'm not sure if it's mentioned anywhere.

For all intent and purposes, it is what it is. We cannot change the behaviour now without probably breaking things. We do need to document it though, make it clear that Point3d and Vector3d will be transformed, but arrays will not.

Dan Rathbun

... ya know how much we hate these "secret behaviors"!

tt_su

@dan rathbun said:

... ya know how much we hate these "secret behaviors"!

I sure do. Unfortunately some of this is very old code going way back before what the source control system has any info about. Occasionally someone who's been around long enough from the early days will recall why something was made the way it was - other times it's one of them mysteries.

eneroth3

My railroad plugin relies heavily on these data being transformed. When a track is manually moved the bezier control points are moved with it and read by the extension so it knows where the track is located and trains can run on it. I was quite worried how to code this until I accidentally ran into this behavior. It can be really useful and if you don't want it it's easy to store the coordinates in an array instead of Point3d or Vector3d (which is what I do in another part of the plugin).

However this really really really should be documented!

tt_su

@eneroth3 said:

However this really really really should be documented!

Indeed. I'll look into this next week to check out what you indicated to be inconsistencies. That worries me.

Aerilius

If I understand it right, an attribute with the same Point3d object that an entity uses (for example directly obtained from the entity face.vertices.first.position) will move with the entity.
A clone/copy of such a Point3d (like point + [0,0,0]) might have initially the same position, but does not show this behavior.

If this behavior is documented, then it's fine. If it is decided to cleanup/simplify it by removing that behavior, we could (?) still use a Point3d relative to the entity/group's boundingbox origin, which will move with the entity. Does that make sense, Christina?

eneroth3

All points and vectors change, even if you add Geom::Point3d.new to the attribute, not just those that refers to a moved vertex.

I suppose a change in the behavior would affect more plugins than just mine. Also it would break backward compability unless you write separate code for older versions which will be much harder to maintain. Also the bounding box is always aligned with the coordinate axes so rotating entity would break the relation between the bounding box origin and a point relative to the entity.

emotionally wounded beast

I'm very new at this, so pardon if I'm terribly mistaken, but....

Small, additional quirks worth mentioning:

Directly storing / recovering Point3ds and Vector3ds in hashes will fail via Dan's method, as the inspection string will not have proper constructors for them. This is Ruby-specific, not Sketchup. That, and I'm assuming the recently noted entity-attributes-auto-transformation will not apply, as Sketchup will only see a string stored as the attribute.

The hash creation failure:

point = Geom;;Point3d.new(0,0,0)
hash = {;origin => point}
hash_str = hash.inspect
new_hash = eval(hash_str) # Will fail

hash.inspect will return => {:origin=>Point3d(0, 0, 0)}

If you eval that it will throw the error: #<NoMethodError: undefined method `Point3d' for main:Object>

Simple, quick solution? Store Point3ds and Vector3ds as arrays, of course.

hash = {;origin => point.to_a}

Or, alternatively, if you were dead set on keeping them as Geoms...

hash_str = hash.inspect
hash_str.gsub!("Point3d", "Geom;;Point3d.new")
# Same for vectors if needed

This will successfully eval to a new hash containing a Point3d (though, as previously mentioned, the point will not auto-transform with the entity it is attached to... I assume).

Dan Rathbun

Good point(s) (pun intended!)

hash_str.gsub!("Point3d", "Geom::Point3d.new")
is effectively fixing flawed inspect methods for API classes. (The hash's method calls inspect upon it's members.)

inspect strings should be evaluable!

.. but someone got lazy, and decided to use inspect for specially formatted output.

eneroth3

I just discovered empty arrays cannot be saved as attributes between sessions in SU 6-2015 (tested on windows 7). However it works just as expected within sessions o.0. Bug report has been submitted.

TIG

@eneroth3 said:

I just discovered empty arrays cannot be saved as attributes between sessions in SU 6-2015 (tested on windows 7). However it works just as expected within sessions o.0. Bug report has been submitted.
But of course your code can simply sidestep that with:
...get_attribute(dict, key, **[]**)
which will either return any array which the key has as its value, OR with your own [] as its default, e.g. when it is not setup OR empty, and you get nil otherwise...

eneroth3

@tig said:

@eneroth3 said:

I just discovered empty arrays cannot be saved as attributes between sessions in SU 6-2015 (tested on windows 7). However it works just as expected within sessions o.0. Bug report has been submitted.
But of course your code can simply sidestep that with:
...get_attribute(dict, key, **[]**)
which will either return any array which the key has as its value, OR with your own [] as its default, e.g. when it is not setup OR empty, and you get nil otherwise...

Yea, it's really easy to work around it once you know about it.

slbaumgartner

@eneroth3 said:

@tig said:

@eneroth3 said:

I just discovered empty arrays cannot be saved as attributes between sessions in SU 6-2015 (tested on windows 7). However it works just as expected within sessions o.0. Bug report has been submitted.
But of course your code can simply sidestep that with:
...get_attribute(dict, key, **[]**)
which will either return any array which the key has as its value, OR with your own [] as its default, e.g. when it is not setup OR empty, and you get nil otherwise...

Yea, it's really easy to work around it once you know about it.

I had run into the empty array problem earlier and I agree 100% with eneroth3. It is not hard to allow for this once you know about it, but there is no documentation anywhere that tells you that you need to. I wonder how many others have independently discovered this...

TIG

BUT the whole idea is that it always returns 'nil' if it's not set.
Doesn't [] === nil
so then that's as expected ?
When you 'get_' an attribute you should always default it to be the base level result: nil, 0, 0.0, "", [], false and so on...
Then test for the 'class' of what's returned...
If you don't, then how do you know what's been returned ?

slbaumgartner

@tig said:

BUT the whole idea is that it always returns 'nil' if it's not set.
Doesn't [] === nil
so then that's as expected ?
When you 'get_' an attribute you should always default it to be the base level result: nil, 0, 0.0, "", [], false and so on...
Then test for the 'class' of what's returned...
If you don't, then how do you know what's been returned ?

I believe in defensive programming, so don't get me wrong: you should always verify and/or default the return from get_.

nil is what is returned by get_ if you never assigned a value to a particular key and provided no default, but it should not be returned when SketchUp couldn't handle the value you happened to set! Transmuting [] into nil is wrong for multiple reasons. No, []===nil is false. Performing a test such as #empty? in case the Array wasn't populated will throw an Exception because nil is not an instance of the Array class.

Instead of converting [] into nil (or converting any other unsupported class instance to nil) the dictionary should raise an exception when you try to set the illegal value.

Dan Rathbun

Storage:
dict["keyname"]= ary.inspect

Retrieval:
ary = eval(dict["keyname"])

It has no problem with an empty array.

There is something in the code of the set_attribute method, that will ignore even a string representation such as "[]" ?

[Info] Allowable Classes for &quot;set_attribute&quot;

[Info] Allowable Classes for "set_attribute"