These comments are redundant, it's obvious why you flip normals, why you move new object to a location of current object, or why you remove mesh (you could name it mesh_temp
to emphasize intent).
It's a lot, but after that it all done for the exception of one issue which we'll discuss later after UI part is handled.
The name of the panel should be changed from Transform
to Edit
, Hollow tool should be placed first (no additional label needed).
Double newline only allowed in-between class and function definitions, everything inside function body should be separated by 1 line max.
- Operator should be called
Hollow
, it is short and widely accepted terminology.
Part of this code should be outside if
block, currently selection changes only when hollow duplicate is created. It should change regardless of that, new object is added to the scene - it should be selected and active.
Duplicating operator settings in the scene is redundant, it is much better to invoke popup dialog with operator settings:
If you find matrix stuff confusing, then you could write it as:
Again, you don't need to comment on every line, there is a check above if create_hollow
and you are using bpy.data.objects.new
right after it, the code is self-explanatory.
This code block is useless, just do if self.offset > 0.0
It's better to avoid negation, if possible, it just makes harder to follow the logic.
Offset
Do you have any more feedback/suggestions/comments about this part?
Tool execution is fine, now we need to tackle UI. I'll do it in a separate thread.
hollow object stay on the same position of the target object
Or alternatively strip world matrix of translation component.
mat = ob.matrix_world.copy()
mat.translation = 0, 0,…
What do you think about this approach?
Your approach is fine, the intent is not as clear with new_from_object
, but that is semantics, functionality is exactly the same.
mesh it produces…
Yeah, let's do Create Hollow Copy
instead.
I'm not getting what you mean by instantiate in bpy.data.meshes
, we still have to use obj.to_mesh()
to get evaluated mesh.
I'm not aware of native…
It's better to not create unnecessary local variables, especially when they are rarely used.
Ther should be "gate" conditions at the beginning of the execute method checking for invalid values.
Trailing whitespace and hard to read. Either do a newline for each attribute or keep it single line, it's not that long anyway.