Mikhail Rachinskiy MikhailRachinskiy
  • Joined on 2010-02-17
Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:13 +01:00
3D Print Toolbox: Add hollow out

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).

Mikhail Rachinskiy suggested changes for blender/blender-addons#105194 2024-02-27 21:37:13 +01:00
3D Print Toolbox: Add hollow out

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.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:13 +01:00
3D Print Toolbox: Add hollow out

The name of the panel should be changed from Transform to Edit, Hollow tool should be placed first (no additional label needed).

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:13 +01:00
3D Print Toolbox: Add hollow out

Double newline only allowed in-between class and function definitions, everything inside function body should be separated by 1 line max.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:13 +01:00
3D Print Toolbox: Add hollow out
  • Operator should be called Hollow, it is short and widely accepted terminology.
Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:13 +01:00
3D Print Toolbox: Add hollow out

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.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:12 +01:00
3D Print Toolbox: Add hollow out

Duplicating operator settings in the scene is redundant, it is much better to invoke popup dialog with operator settings:

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:12 +01:00
3D Print Toolbox: Add hollow out

If you find matrix stuff confusing, then you could write it as:

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:12 +01:00
3D Print Toolbox: Add hollow out

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.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:12 +01:00
3D Print Toolbox: Add hollow out

This code block is useless, just do if self.offset > 0.0

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:12 +01:00
3D Print Toolbox: Add hollow out

It's better to avoid negation, if possible, it just makes harder to follow the logic.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 21:37:12 +01:00
3D Print Toolbox: Add hollow out

Offset

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-27 17:14:01 +01:00
3D Print Toolbox: Add hollow out

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.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-25 17:58:02 +01:00
3D Print Toolbox: Add hollow out

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,
Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-25 17:45:54 +01:00
3D Print Toolbox: Add hollow out

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…

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-25 11:53:58 +01:00
3D Print Toolbox: Add hollow out

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…

Mikhail Rachinskiy suggested changes for blender/blender-addons#105194 2024-02-24 17:57:30 +01:00
3D Print Toolbox: Add hollow out

This one is execution only, next well do UI.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-24 17:57:29 +01:00
3D Print Toolbox: Add hollow out

It's better to not create unnecessary local variables, especially when they are rarely used.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-24 17:57:29 +01:00
3D Print Toolbox: Add hollow out

Ther should be "gate" conditions at the beginning of the execute method checking for invalid values.

Mikhail Rachinskiy commented on pull request blender/blender-addons#105194 2024-02-24 17:57:29 +01:00
3D Print Toolbox: Add hollow out

Trailing whitespace and hard to read. Either do a newline for each attribute or keep it single line, it's not that long anyway.