dr.sybren/rignodes
11
14
Fork 2
Code Issues Pull Requests 5 Projects Releases Wiki Activity

Motion transfer setup #1

Manually merged
Sybren A. Stüvel merged 18 commits from cgtinker/powership:motion_transfer into main 2023-06-05 12:06:15 +02:00
Conversation 6 Commits 18 Files Changed 4 +106 -142
2 changed files with 106 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a024f31128 - Show all commits

3
execute.py
View File

@ -80,8 +80,7 @@ def _on_frame_changed_post(
scene: bpy.types.Scene, depsgraph: bpy.types.Depsgraph scene: bpy.types.Scene, depsgraph: bpy.types.Depsgraph
) -> None: ) -> None:
global _skip_next_autorun global _skip_next_autorun
if not _skip_next_autorun: _skip_next_autorun = True
_skip_next_autorun = True
cgtinker marked this conversation as resolved
Sybren A. Stüvel commented 2023-06-01 11:05:15 +02:00
Review
Copy Link

Since _skip_next_autorun is a boolean, the if condition can be removed.

Since `_skip_next_autorun` is a boolean, the `if` condition can be removed.
run_node_tree(scene, depsgraph) run_node_tree(scene, depsgraph)

245
nodes.py
View File

@ -3,8 +3,8 @@
import functools import functools
from collections import deque from collections import deque
from copy import copy from copy import copy
from math import degrees, radians, sqrt from math import degrees, radians
from typing import TypeVar, Callable, Optional, Iterable, Any from typing import TypeVar, Callable, Optional, Iterable
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:08:17 +02:00
Outdated
Review
Copy Link

Both sqrt and Any are unused.

Both `sqrt` and `Any` are unused.
import bpy import bpy
import nodeitems_utils import nodeitems_utils
@ -271,7 +271,7 @@ class AbstractPowerShipNode(bpy.types.Node):
def _first_input_to_output(self) -> None: def _first_input_to_output(self) -> None:
"""Copy the first input's default value to the output, if the sockets are compatible.""" """Copy the first input's default value to the output, if the sockets are compatible."""
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:06:43 +02:00
Outdated
Review
Copy Link

Keep formatting changes out of the patch. You can use git gui or some other tool to cherry-pick which lines you do (not) want to include in a commit. That way you can exclude such changes, commit the rest, then revert the unwanted formatting changes to get rid of them.

Keep formatting changes out of the patch. You can use `git gui` or some other tool to cherry-pick which lines you do (not) want to include in a commit. That way you can exclude such changes, commit the rest, then revert the unwanted formatting changes to get rid of them.
if not self.inputs or not self.outputs: if not self.inputs or not self.outputs:
return return
@ -580,25 +580,16 @@ class ToVector(AbstractPowerShipNode):
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketFloat", "X") self.inputs.new("NodeSocketFloat", "X")
self.add_optional_input_socket("NodeSocketFloat", "Y") self.inputs.new("NodeSocketFloat", "Y")
self.add_optional_input_socket("NodeSocketFloat", "Z") self.inputs.new("NodeSocketFloat", "Z")
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:17:38 +02:00
Outdated
Review
Copy Link

Here I think a regular input socket would be better. The "optional" ones (and I should document this better) are intended for sockets that will be ignored when they are unconnected. In this case I feel regular sockets would be better, as those allow you to enter their values manually when the socket is unconnected.

Here I think a regular input socket would be better. The "optional" ones (and I should document this better) are intended for sockets that will be ignored when they are unconnected. In this case I feel regular sockets would be better, as those allow you to enter their values manually when the socket is unconnected.
self.outputs.new("NodeSocketVector", "Vector") self.outputs.new("NodeSocketVector", "Vector")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
x = self._get_optional_input_value("X", float) x = self.inputs["X"].default_value or 0.0
y = self._get_optional_input_value("Y", float) y = self.inputs["Y"].default_value or 0.0
z = self._get_optional_input_value("Z", float) z = self.inputs["Z"].default_value or 0.0
self.outputs["Vector"].default_value = Vector((x, y, z))
v = Vector((0.0, 0.0, 0.0))
if x is not None:
v.x = x
if y is not None:
v.y = y
if z is not None:
v.z = z
self.outputs["Vector"].default_value = v
class SplitVector(AbstractPowerShipNode): class SplitVector(AbstractPowerShipNode):
@ -607,93 +598,88 @@ class SplitVector(AbstractPowerShipNode):
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketVector", "Vector") self.inputs.new("NodeSocketVector", "Vector")
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:18:40 +02:00
Outdated
Review
Copy Link

This can be simplified by using shortcutting (a or b evaluates to b when a is falsey).

        x = self._get_optional_input_value("X", float) or 0.0
        y = self._get_optional_input_value("Y", float) or 0.0
        z = self._get_optional_input_value("Z", float) or 0.0

        v = Vector((x, y, z))
This can be simplified by using shortcutting (`a or b` evaluates to `b` when `a` is falsey). ```py x = self._get_optional_input_value("X", float) or 0.0 y = self._get_optional_input_value("Y", float) or 0.0 z = self._get_optional_input_value("Z", float) or 0.0 v = Vector((x, y, z)) ```
self.outputs.new("NodeSocketFloat", "X") self.outputs.new("NodeSocketFloat", "X")
self.outputs.new("NodeSocketFloat", "Y") self.outputs.new("NodeSocketFloat", "Y")
self.outputs.new("NodeSocketFloat", "Z") self.outputs.new("NodeSocketFloat", "Z")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
v = self._get_optional_input_value("Vector", Vector) v = self.inputs["Vector"].default_value
self.outputs["X"].default_value = v.x self.outputs["X"].default_value = v.x
self.outputs["Y"].default_value = v.y self.outputs["Y"].default_value = v.y
self.outputs["Z"].default_value = v.z self.outputs["Z"].default_value = v.z
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:18:58 +02:00
Outdated
Review
Copy Link

Same as above, this could be a regular vector input socket.

Same as above, this could be a regular vector input socket.
class Distance(AbstractPowerShipNode): class Distance(AbstractPowerShipNode):
""" Calculates distance between two points. """ """Calculates distance between two points."""
bl_idname = "Distance" bl_idname = "Distance"
bl_label = "Distance" bl_label = "Distance"
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketVector", "U") self.inputs.new("NodeSocketVector", "U")
self.add_optional_input_socket("NodeSocketVector", "V") self.inputs.new("NodeSocketVector", "V")
self.outputs.new("NodeSocketFloat", "Float") self.outputs.new("NodeSocketFloat", "Float")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
u = self._get_optional_input_value("U", Vector) u = self.inputs["U"].default_value
v = self._get_optional_input_value("V", Vector) v = self.inputs["V"].default_value
self.outputs["Float"].default_value = (u-v).length self.outputs["Float"].default_value = (u - v).length
cgtinker marked this conversation as resolved
Sybren A. Stüvel commented 2023-05-26 17:23:28 +02:00
Review
Copy Link

Does this calculate a normal vector? Or its length?

Does this calculate a normal vector? Or its length?
Denys Hsu commented 2023-05-26 19:33:44 +02:00
Review
Copy Link

The length.
I went through the comments and updated them (some were unrelated and happened due to closed eyes copy pasting...)

The length. I went through the comments and updated them (some were unrelated and happened due to closed eyes copy pasting...)
class NormalFromPoints(AbstractPowerShipNode): class NormalFromPoints(AbstractPowerShipNode):
""" Calculates normal from three points (plane). """ """Calculates normal from three points (plane)."""
bl_idname = "NormalFromPoints" bl_idname = "NormalFromPoints"
bl_label = "Normal from Points" bl_label = "Normal from Points"
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketVector", "U") self.inputs.new("NodeSocketVector", "U")
self.add_optional_input_socket("NodeSocketVector", "V") self.inputs.new("NodeSocketVector", "V")
self.add_optional_input_socket("NodeSocketVector", "W") self.inputs.new("NodeSocketVector", "W")
self.outputs.new("NodeSocketVector", "Result") self.outputs.new("NodeSocketVector", "Result")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
u = self._get_optional_input_value("U", Vector) u = self.inputs["U"].default_value
v = self._get_optional_input_value("V", Vector) v = self.inputs["V"].default_value
w = self._get_optional_input_value("W", Vector) w = self.inputs["W"].default_value
a = v - u a = v - u
b = w - u b = w - u
normal = a.cross(b) normal = a.cross(b).normalized()
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:20:10 +02:00
Outdated
Review
Copy Link

These should again be regular input sockets, as it makes no sense to do the computation without a full set of three points.

These should again be regular input sockets, as it makes no sense to do the computation without a full set of three points.
self.outputs["Result"].default_value = normal self.outputs["Result"].default_value = normal
_enum_up_axis_items = ( _enum_up_axis_items = (
('X', "X", ""), ("X", "X", ""),
('Y', "Y", ""), ("Y", "Y", ""),
('Z', "Z", ""), ("Z", "Z", ""),
) )
_enum_track_axis_items = ( _enum_track_axis_items = (
('X', "X", ""), ("X", "X", ""),
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:21:49 +02:00
Outdated
Review
Copy Link

This only outputs a normal vector when a and b are perpendicular. Better to use a.cross(b).normalized()

This only outputs a normal vector when `a` and `b` are perpendicular. Better to use `a.cross(b).normalized()`
👍 1
('Y', "Y", ""), ("Y", "Y", ""),
('Z', "Z", ""), ("Z", "Z", ""),
('-X', "-X", ""), ("-X", "-X", ""),
('-Y', "-Y", ""), ("-Y", "-Y", ""),
('-Z', "-Z", ""), ("-Z", "-Z", ""),
) )
class RotateTowards(AbstractPowerShipNode): class RotateTowards(AbstractPowerShipNode):
""" Calculate the rotation from a vector with a track and up axis """Calculate the rotation from a vector with a track and up axis."""
based on the given origin, destination. """
bl_idname = "RotateTowards" bl_idname = "RotateTowards"
bl_label = "Rotate Towards" bl_label = "Rotate Towards"
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
track: bpy.props.EnumProperty( # type: ignore track: bpy.props.EnumProperty( # type: ignore
name="Track", name="Track", items=_enum_track_axis_items, default="X"
items=_enum_track_axis_items,
default='X'
) )
up: bpy.props.EnumProperty( # type: ignore up: bpy.props.EnumProperty( # type: ignore
name="Up", name="Up", items=_enum_up_axis_items, default="Y"
items=_enum_up_axis_items,
default='Y'
) )
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:23:20 +02:00
Outdated
Review
Copy Link

Follow PEP 257 (in this case, the initial line should be shorter, and the leading/trailing spaces should be removed). Same for other docstrings.

Wellll.... try to follow PEP 257 but also keep in mind that Blender uses those docstrings for the tooltips, and thus they shouldn't end in a period. Stupid, I know...

Follow PEP 257 (in this case, the initial line should be shorter, and the leading/trailing spaces should be removed). Same for other docstrings. Wellll.... try to follow PEP 257 but also keep in mind that Blender uses those docstrings for the tooltips, and thus they shouldn't end in a period. Stupid, I know...
def draw_buttons( def draw_buttons(
@ -704,34 +690,33 @@ class RotateTowards(AbstractPowerShipNode):
layout.prop(self, "track") layout.prop(self, "track")
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketVector", "Origin") self.inputs.new("NodeSocketVector", "Vector")
self.add_optional_input_socket("NodeSocketVector", "Destination") self.inputs.new("NodeSocketVector", "RotateTo")
self.outputs.new("NodeSocketQuaternion", "Rotation") self.outputs.new("NodeSocketQuaternion", "Rotation")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
origin = self._get_optional_input_value("Origin", Vector) origin = self.inputs["Vector"].default_value
Sybren A. Stüvel commented 2023-06-01 11:24:10 +02:00
Outdated
Review
Copy Link

Why is Y the default up-vector, and not Z?

Why is `Y` the default up-vector, and not `Z`?
Denys Hsu commented 2023-06-04 16:58:55 +02:00
Outdated
Review
Copy Link

I thought it was like that in the python interface (and I was wrong).
Just checked in mathutils_Vector.c, there the up axis is Y and the track axis is Z. Should we follow that convention or go for Z as up axis and X as track axis?

I thought it was like that in the python interface (and I was wrong). Just checked in _mathutils_Vector.c_, there the up axis is **Y** and the track axis is **Z**. Should we follow that convention or go for **Z** as up axis and **X** as track axis?
destination = self._get_optional_input_value("Destination", Vector) destination = self.inputs["RotateTo"].default_value
vec = Vector((destination - origin)) vec = Vector((destination - origin))
rot = vec.to_track_quat(self.track, self.up) rot = vec.to_track_quat(self.track, self.up)
self.outputs["Rotation"].default_value = rot.normalized() self.outputs["Rotation"].default_value = rot.normalized()
class OffsetRotation(AbstractPowerShipNode): class OffsetRotation(AbstractPowerShipNode):
""" Offset a rotation. """ """Offset a rotation."""
bl_idname = "OffsetRotation" bl_idname = "OffsetRotation"
bl_label = "Offset Rotation" bl_label = "Offset Rotation"
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:25:32 +02:00
Outdated
Review
Copy Link

I think these names could be improved for clarity. How about "Vector" and "Rotate To"?

I think these names could be improved for clarity. How about "Vector" and "Rotate To"?
default_value = Quaternion()
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketQuaternion", "Base") self.inputs.new("NodeSocketQuaternion", "Base")
self.add_optional_input_socket("NodeSocketQuaternion", "Offset") self.inputs.new("NodeSocketQuaternion", "Offset")
self.outputs.new("NodeSocketQuaternion", "Rotation") self.outputs.new("NodeSocketQuaternion", "Rotation")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
base = self._get_optional_input_value("Base", Quaternion) base = self.inputs["Base"].default_value
offset = self._get_optional_input_value("Offset", Quaternion) offset = self.inputs["Offset"].default_value
self.outputs["Rotation"].default_value = base @ offset self.outputs["Rotation"].default_value = base @ offset
@ -741,19 +726,19 @@ class MapRange(AbstractPowerShipNode):
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:26:11 +02:00
Outdated
Review
Copy Link

default_value doesn't seem used.

`default_value` doesn't seem used.
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketFloat", "Value") self.inputs.new("NodeSocketFloat", "Value")
self.add_optional_input_socket("NodeSocketFloat", "From Min") self.inputs.new("NodeSocketFloat", "From Min")
self.add_optional_input_socket("NodeSocketFloat", "From Max") self.inputs.new("NodeSocketFloat", "From Max")
self.add_optional_input_socket("NodeSocketFloat", "To Min") self.inputs.new("NodeSocketFloat", "To Min")
self.add_optional_input_socket("NodeSocketFloat", "To Max") self.inputs.new("NodeSocketFloat", "To Max")
self.outputs.new("NodeSocketFloat", "Result") self.outputs.new("NodeSocketFloat", "Result")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
val = self._get_optional_input_value("Value", float) val = self.inputs["Value"].default_value
fmin = self._get_optional_input_value("From Min", float) fmin = self.inputs["From Min"].default_value
fmax = self._get_optional_input_value("From Max", float) fmax = self.inputs["From Max"].default_value
tmin = self._get_optional_input_value("To Min", float) tmin = self.inputs["To Min"].default_value
tmax = self._get_optional_input_value("To Max", float) tmax = self.inputs["To Max"].default_value
factor = (tmax - tmin) / (fmax - fmin) factor = (tmax - tmin) / (fmax - fmin)
offset = tmin - factor * fmin offset = tmin - factor * fmin
@ -761,52 +746,34 @@ class MapRange(AbstractPowerShipNode):
class AngleFromVectors(AbstractPowerShipNode): class AngleFromVectors(AbstractPowerShipNode):
""" Calculate the angle between two vectors. Output in degrees. """ """Calculate the angle between two vectors. Output in degrees."""
bl_idname = "AngleFromVectors" bl_idname = "AngleFromVectors"
bl_label = "Angle From Vectors" bl_label = "Angle From Vectors"
bl_icon = "EMPTY_ARROWS" bl_icon = "EMPTY_ARROWS"
angle_type: bpy.props.EnumProperty( # type: ignore
name="Type",
items=[
("DEFAULT", "Unsigned", ""),
("SIGNED", "Signed", ""),
],
)
def draw_buttons(
self, context: bpy.types.Context, layout: bpy.types.UILayout
) -> None:
super().draw_buttons(context, layout)
layout.prop(self, "angle_type")
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketVector", "U") self.inputs.new("NodeSocketVector", "U")
self.add_optional_input_socket("NodeSocketVector", "V") self.inputs.new("NodeSocketVector", "V")
self.outputs.new("NodeSocketFloat", "Angle") self.outputs.new("NodeSocketFloat", "Angle")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
u = self._get_optional_input_value("U", Vector) u = self.inputs["U"].default_value
v = self._get_optional_input_value("V", Vector) v = self.inputs["V"].default_value
signed = self.angle_type == 'SIGNED'
angle = 0 angle = 0
if not (u.length == 0 or v.length == 0): if not (u.length_squared == 0 or v.length_squared == 0):
if signed: angle = u.angle(v)
angle = u.angle_signed(v)
else:
angle = u.angle(v)
self.outputs["Angle"].default_value = degrees(angle) self.outputs["Angle"].default_value = degrees(angle)
_enum_vector_math_operations = [ _enum_vector_math_operations = [
('ADD', "Add", ""), ("ADD", "Add", ""),
('SUBSTRACT', "Substract", ""), ("SUBSTRACT", "Substract", ""),
('MULTIPLY', "Mutliply", ""), ("MULTIPLY", "Mutliply", ""),
('DIVIDE', "Divide", ""), ("DIVIDE", "Divide", ""),
Sybren A. Stüvel commented 2023-06-01 11:27:56 +02:00
Outdated
Review
Copy Link

Why name it DEFAULT instead of UNSIGNED? And why is UNSIGNED the default value? Not saying that it should be the other one, just wondering about your thought process.

Why name it `DEFAULT` instead of `UNSIGNED`? And why is `UNSIGNED` the default value? Not saying that it should be the other one, just wondering about your thought process.
Denys Hsu commented 2023-06-04 21:38:30 +02:00
Outdated
Review
Copy Link

Usually an angle is not signed, you got to define something to sign it. That's why I thought unsigned angles should be the default.

I rechecked and to be honest, I neither knew that the "signed" vector method is only for 2D vectors nor how it actually signed an angle. So I guess I removed it and kept only the "unsigned" angle for now - my bad sorry. Kinda liked the idea of an easy to use signed angle.

So far, when I needed a signed angle, I ended up using a plane. Basically I've used the signed distance (based on the normal) from the plane to the destination of the vector and used the sign for the angle. This is possible with the current system without changes so I guess it's fine.

Usually an angle is not signed, you got to define something to sign it. That's why I thought unsigned angles should be the default. I rechecked and to be honest, I neither knew that the "signed" vector method is only for 2D vectors nor how it actually signed an angle. So I guess I removed it and kept only the "unsigned" angle for now - my bad sorry. Kinda liked the idea of an easy to use signed angle. So far, when I needed a signed angle, I ended up using a plane. Basically I've used the signed distance (based on the normal) from the plane to the destination of the vector and used the sign for the angle. This is possible with the current system without changes so I guess it's fine.
('CROSS', "Cross", ""), ("CROSS", "Cross", ""),
] ]
@ -827,38 +794,36 @@ class VectorMath(AbstractPowerShipNode):
layout.prop(self, "operation") layout.prop(self, "operation")
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketVector", "U") self.inputs.new("NodeSocketVector", "U")
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:30:14 +02:00
Outdated
Review
Copy Link

Don't compute length when you can use length_squared as well.

Don't compute `length` when you can use `length_squared` as well.
self.add_optional_input_socket("NodeSocketVector", "V") self.inputs.new("NodeSocketVector", "V")
self.outputs.new("NodeSocketVector", "Result") self.outputs.new("NodeSocketVector", "Result")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
u = self._get_optional_input_value("U", Vector) u = self.inputs["U"].default_value
v = self._get_optional_input_value("V", Vector) v = self.inputs["V"].default_value
match self.operation: match self.operation:
case 'ADD': case "ADD":
res = u+v res = u + v
case 'MULTIPLY': case "MULTIPLY":
res = u*v res = u * v
case 'SUBSTRACT': case "SUBSTRACT":
res = u-v res = u - v
case 'DIVIDE': case "DIVIDE":
res = Vector([x/y if y != 0.0 else float("nan") res = Vector((x / y if y != 0.0 else 0.0 for x, y in zip(u, v)))
for x, y in zip(u, v)]) case "CROSS":
case 'CROSS':
res = u.cross(v) res = u.cross(v)
case _: case _:
print("Vector math operation not found:", self.operation) raise ValueError(f"Vector math operation not found: {self.operation}\n")
raise ValueError
self.outputs["Result"].default_value = res self.outputs["Result"].default_value = res
_enum_math_operations = [ _enum_math_operations = [
('ADD', "Add", ""), ("ADD", "Add", ""),
('SUBSTRACT', "Substract", ""), ("SUBSTRACT", "Substract", ""),
('MULTIPLY', "Mutliply", ""), ("MULTIPLY", "Mutliply", ""),
('DIVIDE', "Divide", ""), ("DIVIDE", "Divide", ""),
] ]
@ -879,26 +844,25 @@ class Math(AbstractPowerShipNode):
layout.prop(self, "operation") layout.prop(self, "operation")
def init(self, context): def init(self, context):
self.add_optional_input_socket("NodeSocketFloat", "U") self.inputs.new("NodeSocketFloat", "U")
self.add_optional_input_socket("NodeSocketFloat", "V") self.inputs.new("NodeSocketFloat", "V")
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:34:40 +02:00
Outdated
Review
Copy Link

This could be an interesting design discussion. Not something to do here -- this code is fine. But in general, we have to decide how Powership is going to handle erroneous values. For example,. the Vector.normalized() function I suggested above will simply return (0, 0, 0) for zero vectors. I feel that maybe here it could also make sense to define x / 0 → 0 and return as many usable values as possible.

I actually had a little design discussion with @nathanvegdahl about this, and until we have a 'debug mode' that can visualise where NaNs are produced, it's better to stick to regular floats. So in this case x/y if y != 0.0 else 0.0

And another minor thing: the list comprehension can be replaced by a generator expression for a little bit of added performance and readability:

res = Vector(x/y if y != 0.0 else 0.0 for x, y in zip(u, v))
This could be an interesting design discussion. ~~Not something to do here -- this code is fine.~~ But in general, we have to decide how Powership is going to handle erroneous values. For example,. the `Vector.normalized()` function I suggested above will simply return `(0, 0, 0)` for zero vectors. I feel that maybe here it could also make sense to define `x / 0 → 0` and return as many usable values as possible. I actually had a little design discussion with @nathanvegdahl about this, and until we have a 'debug mode' that can visualise where NaNs are produced, it's better to stick to regular floats. So in this case `x/y if y != 0.0 else 0.0` And another minor thing: the list comprehension can be replaced by a generator expression for a little bit of added performance and readability: ```py res = Vector(x/y if y != 0.0 else 0.0 for x, y in zip(u, v)) ```
self.outputs.new("NodeSocketFloat", "Result") self.outputs.new("NodeSocketFloat", "Result")
def execute(self, depsgraph: bpy.types.Depsgraph) -> None: def execute(self, depsgraph: bpy.types.Depsgraph) -> None:
u = self._get_optional_input_value("U", float) u = self.inputs["U"].default_value
v = self._get_optional_input_value("V", float) v = self.inputs["V"].default_value
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:32:16 +02:00
Outdated
Review
Copy Link

Either log/print, or raise an exception. Don't do both, as one issue will be reported twice.

Either log/print, or raise an exception. Don't do both, as one issue will be reported twice.
cgtinker marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2023-06-01 11:31:40 +02:00
Outdated
Review
Copy Link

Don't use a class as exception, always instantiate it.

raise ValueError(f"Vector math operation not found: {self.operation!r}")
Don't use a class as exception, always instantiate it. ```py raise ValueError(f"Vector math operation not found: {self.operation!r}") ```
match self.operation: match self.operation:
case 'ADD': case "ADD":
res = u+v res = u + v
case 'MULTIPLY': case "MULTIPLY":
res = u*v res = u * v
case 'SUBSTRACT': case "SUBSTRACT":
res = u-v res = u - v
case 'DIVIDE': case "DIVIDE":
res = u/v if v != 0 else float("nan") res = u / v if v != 0 else 0
case _: case _:
print("Math operation not found:", self.operation) raise ValueError(f"Math operation not found: {self.operation}\n")
raise ValueError
self.outputs["Result"].default_value = res self.outputs["Result"].default_value = res
@ -994,12 +958,13 @@ class SetBoneNode(AbstractPowerShipNode):
match self.space: match self.space:
case "WORLD": case "WORLD":
bone_mat_world = Matrix.LocRotScale( bone_mat_world = Matrix.LocRotScale(loc, rot, scale)
loc, rot.normalized(), scale)
loc, rot, scale = bone_mat_world.decompose() loc, rot, scale = bone_mat_world.decompose()
case "CHANNELS": case "CHANNELS":
bone_rest_rot_scale.translation = v_nil bone_rest_rot_scale.translation = v_nil
mat_rot_scale = Matrix.LocRotScale(v_nil, rot, scale) @ bone_rest_rot_scale mat_rot_scale = (
Matrix.LocRotScale(v_nil, rot, scale) @ bone_rest_rot_scale
)
mat_loc = Matrix.Translation(loc) mat_loc = Matrix.Translation(loc)
bone_mat_world = mat_loc @ mat_rot_scale bone_mat_world = mat_loc @ mat_rot_scale
@ -1318,7 +1283,7 @@ node_categories = [
nodeitems_utils.NodeItem("ClampNode"), nodeitems_utils.NodeItem("ClampNode"),
nodeitems_utils.NodeItem("ToEulerNode"), nodeitems_utils.NodeItem("ToEulerNode"),
nodeitems_utils.NodeItem("FromEulerNode"), nodeitems_utils.NodeItem("FromEulerNode"),
] ],
), ),
PowerShipNodeCategory( PowerShipNodeCategory(
"DEBUG", "DEBUG",