Just a few further comments to discuss:

• I am not too happy about the fact that materials is an attribute of ba.bond.settings for Bond but ba.isosurface.materials for Isosurface. I did this simply because of where the existing code for build_materials etc was. I don't know if there is a good reason why build_materials is in a different place for each class, but long-term maybe refactoring this so that it is consistent would be a good idea
• I am also not happy that you can only set ba.isosurface.materials after ba.isosurface.draw() has been called. More generally, I have noticed in batoms that the order in which you do things really matters. Is this a deliberate design decision, or should I modify the isosurface materials setter to also work if draw() has not yet been called?
• more generally, I don't have a good sense for how many other classes we might want to implement a materials propery for. If there are more, I am not sure that having bespoke materials property implementation for each class is the right approach. Do we want to introduce an ABC or a Protocol to make sure that the materials property follows a consistent pattern?

Hi @elinscott , Thanks for the PR.

In batoms, the bond, isosurface, and other plugins follow a similar design. Each of them has a bpy_data.py file containing a Setting class that stores the setting data. This class includes parameters like color and materials_style (optional). All data in this class should have a clearly defined structure, such as string or float, meaning they are static data. The node_inputs you want to add is dynamic and cannot be added to this class.

Limited Support

The simplest solution would be to add some common material parameters, such as Metallic and Roughness, to the Setting class, e.g., in bpy_data.IsosurfaceSetting.

Benefits:

• Users can define all setting parameters (e.g., bond width, color, etc.) in one class before drawing the bond.

Full Support

Full support would involve users interacting directly with the materials. Users would draw the objects (bond, isosurface), then create and set the materials to these objects. This approach would require refactoring the code to handle this interaction.

For now, I recommend the Limited Support approach.

Responses to Your Comments:

I am not too happy about the fact that materials is an attribute of ba.bond.settings for Bond but ba.isosurface.materials for Isosurface.

There are no materials data directly in bond.settings; only color and material_style are there. The color is also in isosurface.settings, and you can add material_style similarly.

I am also not happy that you can only set ba.isosurface.materials after ba.isosurface.draw() has been called.

You can set the color and other parameters before drawing the isosurface.

More generally, I don't have a good sense for how many other classes we might want to implement a materials property for. If there are more...

All classes share the same pattern and have a bpy_data.py file.

I hope this answers your concerns.

Hi @superstar54 -- thanks for the feedback. However, I don't actually think that these objects follow a similar design. Bond is not a plugin, and build_materials() is a method of BondSettings (cf. Isosurface).

For this reason it was possible for me to add node_inputs to the BondSettings class, contrary to what you say above:

The node_inputs you want to add is dynamic and cannot be added to this class

Bond is not a plugin, and build_materials() is a method of BondSettings (cf. Isosurface).

I see. bond is special in this case because the BondSettings creates its own objects (cylinders) with materials, then copy these objects into different positions for the bond. Meanwhile, isosurface and other plugins don't need to copy step, so the materials are bound to the isosurface directly.

I would suggest to merge your PR first, and then we can try to refactor the code later.