New Material Methods! (With bugs :( )

Dan Rathbun · 15 Jan 2011, 01:20

http://code.google.com/apis/sketchup/docs/releases.html

@unknownuser said:

Expanded the Material Class
New methods:
Material.remove
Material.name=
Material.write_thumbnail

NOTE: There's a typo here: it's Materials.remove(material)

You mean it's an instance method of the Materials collection.

Pixero · 15 Jan 2011, 07:31

@thomthom said:

There's a type here: it's Materials.remove(material)

That's a great addition. I have some ideas that would greatly benefit from this...

simon le bon · 15 Jan 2011, 09:15

but no retro compatibility

*s

thomthom · 15 Jan 2011, 11:10

@thomthom said:

NOTE: There's a type here: it's Materials.remove(material)

Oh great, a typo in my typo notice.

@simon le bon said:

but no retro compatibility

No. That's evolution for yah. With the changes and additions in SU8 there might be some plugins that will require SU8 as a minimum.

thomthom · 15 Jan 2011, 11:13

Just hope that no one had extended Material.name= anywhere.
Dan: did you say there was a way to detect if a method had been overwritten?

TIG · 15 Jan 2011, 11:37

@thomthom said:

Just hope that no one had extended Material.name= anywhere.
Dan: did you say there was a way to detect if a method had been overwritten?

This is 'detected' in my additional class methods and the code only makes the new method if it doesn't already exist...
Amongst others there are material.name='newname', material.delete and material.save_thumbnail('filepath') that mimic these new methods in this v8 only... so they get made in earlier versions as needed - see http://forums.sketchucation.com/viewtopic.php?p=301258#p301258
So


class Sketchup;;Material
  if not Sketchup;;Material.method_defined?(;name=)
   def name=(new_name=nil)
     ### etc code to rename material
   end#def
  end#if
end#class

etc etc

Dan Rathbun · 15 Jan 2011, 17:11

SO TIG you may wish to update your extension then to alias the method names write_thumbnail() and remove() ??

thomthom · 15 Jan 2011, 17:16

But there is no knowing of which other plugins has done this. There's a number of them that extended Group, Image and Array. It would be interesting to make a script that could scan the base classes for modification and map them. To get a feel of how the general state of the SU Ruby environment.

Dan Rathbun · 15 Jan 2011, 17:56

@thomthom said:

But there is no knowing of which other plugins has done this.

Dont say there is no way, just because you don't know how (..YET,) ... remember Ruby is cool! Ruby will find a way, or allow a way through it's immaculate dynamistic glory.

TIG · 15 Jan 2011, 18:39

@dan rathbun said:

SO TIG you may wish to update your extension then to alias the method names write_thumbnail() and remove() ??

I could - trust Google not to copy my naming conventions!

Dan Rathbun · 15 Jan 2011, 19:18

@tig said:

@dan rathbun said:

SO TIG you may wish to update your extension then to alias the method names write_thumbnail() and remove() ??

I could - trust Google not to copy my naming conventions!

They could be following the conventions they have established already in the API .. although they didn't in this case (which really irks me to no end.. a quick check of the API methods index is all it takes.. before 'inventing' a new method..)

All the other thumbnail methods use " save_thumbnail()"
" write" was used for images, textures, and IO operations, such as defaults to plist or registry.

" remove()" was used when you 'dettached' an item from something else, but did not actually delete or dipose of it, as in an observer, or removing entity references from the selection set.

" delete()" was used when you actually wanted to dispose of something, such as deleting an attribute, dictionary, or a set.

So the question is.. did they follow their OWN conventions with these new methods??

thomthom · 15 Jan 2011, 22:02

And why would the delete method be part of Materials instead of Material ?

Dan Rathbun · 15 Jan 2011, 22:10

@thomthom said:

And why would the delete method be part of Materials instead of Material ?

Because self.kill is suicidal, socially unacceptable, and often religiously repugnant.

Seriously, an object does not often dispose of itself, especially when it's a member of a collection. The collection class has a method(s) to dispose of one (or more,) of it's members. Ie Array instance methods: .shift, .pop, .slice etc....

thomthom · 15 Jan 2011, 22:13

entity.erase! ...

Dan Rathbun · 15 Jan 2011, 22:44

@thomthom said:

entity.erase! ...

(1) that's not a Ruby method.. it's a Google written method (that doesn't really follow convention.)

(2) It uses the **!** ("in place",) suffix to denote the operation is immediate against the receiver... BUT it is actually just an alias for Sketchup::Entities#erase_entities(receiver)

.. yes there are bound to be variances ... likely caused by human error.

thomthom · 17 Jan 2011, 15:21

Cursed! Materials.remove is bugged! http://forums.sketchucation.com/viewtopic.php?f=180&t=34217&p=301158#p301158

TIG · 17 Jan 2011, 15:44

So material.remove doesn't exist BUT is in the API docs ???
Then materials.remove(material) exists but doesn't work as expected !!!

What is going on ???

At least my 'clunky' old material.delete works... and it reverts all objects that were using that material to have 'nil'! [but it does have to traverse all model.entities and definition.entities too to achieve it !]

thomthom · 17 Jan 2011, 15:47

@tig said:

So material.remove doesn't exist BUT is in the API docs ???

Yes - the docs where wrong.

@tig said:

At least my 'clunky' old material.delete works... and it reverts all objects that were using that material to have 'nil'! [but it does have to traverse all model.entities and definition.entities too to achieve it !]

Yea. When using Materials.remove you still need to iterate the whole model, but at least one doesn't ahve to create temp groups and use purge.
Too bad this method is bugged when it finally appeared.

But material.name= seem to work fine. Which is very good as one doesn't need to clone a new variant of the material.

jhauswirth · 20 Jan 2011, 14:54

@tig said:

Then materials.remove(material) exists but doesn't work as expected !!!

What is going on ???

It is working as intended. It would all make sense if you guys could see how the Ruby API is implemented in SU.
The Ruby API is a very thin wrapper around our C++ code. So in this case we have a CMaterialManager class
in SU and the Ruby Materials class wraps CMaterialManager. When you get Materials from the model you get the
Ruby wrapper around CMaterialManager. CMaterialManager knows nothing about the model. Its just
a std::vector<CMaterial*> CMaterialVec; and all Materials.remove does is call
CMaterialManager.RemoveMaterial(CMaterial*), which just removes the given material from the CMaterialVec.
What everyone is asking for is probably a new method in model called model.remove_material (you guys can start
another thread on what it should be called). This new method would live in model because
model owns Materials and the entities that are assigned the materials. So model has all the info
needed to implement what you are asking for. Sorry for messing this up, I sneaked these new methods
in right before release which didn't get properly reviewed by the beta group because there were no
release notes made for the new methods.

thomthom · 20 Jan 2011, 15:33

@jhauswirth said:

It is working as intended. It would all make sense if you guys could see how the Ruby API is implemented in SU.
The Ruby API is a very thin wrapper around our C++ code.

In this case I think it might have been too thin. I can't think of a case where you want to remove a material just from the manager and not the model entities. As it is now it needs to be clearly documented that it orphans the material and the developer that uses it needs to takes care of removing it from the model. Because once it's been orphaned one can't locate it in the material collection.

@jhauswirth said:

What everyone is asking for is probably a new method in model called model.remove_material (you guys can start
another thread on what it should be called). This new method would live in model because
model owns Materials and the entities that are assigned the materials. So model has all the info
needed to implement what you are asking for.

But wouldn't that be inconsistent Pages.erase ? Pages internally contain a reference to it's owner Model via .parent. Couldn't materials.remove also do this? Just seems more neater organized that way. And it's where I'd be starting to look if I was looking for such a method.

@jhauswirth said:

Sorry for messing this up, I sneaked these new methods
in right before release which didn't get properly reviewed by the beta group because there were no
release notes made for the new methods.

These method has been high on the request list, so it's good to see it. But yes, would have been good if they had gone through at least one round of testing.
The Material.name= appear to work as expected though. It's just this Materials.remove which has potential for causing issues. But at least it allows us to remove materials from the list without purging and creating temp geometry etc. I already made a wrapper for the function that remove the material from the entities before removing it from the material manager, but I wonder if it would have been faster if done natively from SU instead of via the Ruby API?