WIP: Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle" #110637

Draft

Damien Picard wants to merge 1 commits from pioverfour/blender:dp_rename_degr_angle into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.

Damien Picard commented

2023-07-31 00:08:20 +02:00

Member

The "Degr" socket of the Rotate compositing node is used to store the
angle of rotation. It is needlessly shortened, but it is also wrong, as
the unit depends on the scene settings and can also be radians.

Similarly to !108234, this commit renames it to "Angle", with a
versioning for existing nodes.

Before	After

The "Degr" socket of the Rotate compositing node is used to store the angle of rotation. It is needlessly shortened, but it is also wrong, as the unit depends on the scene settings and can also be radians. Similarly to !108234, this commit renames it to "Angle", with a versioning for existing nodes. Ref #96219 ----- | Before | After | |-|-| | ![image](/attachments/f1873243-67cd-43cb-a287-5ef81b936138) | ![image](/attachments/a750d389-64c7-4629-af01-876d05baa4a2) |

56 KiB

55 KiB

Damien Picard added the

Nodes & Physics

labels 2023-07-31 00:08:20 +02:00

Damien Picard added 1 commit 2023-07-31 00:08:33 +02:00

08a1c4aa9e UI: Rename "Degr" socket of the Rotate compositing node to "Angle"

The "Degr" socket of the Rotate compositing node is used to store the
angle of rotation. It is needlessly shortened, but it is also wrong, as
the unit depends on the scene settings and can also be radians.

Similarly to !108234, this commit renames it to "Angle", with a
versioning for existing nodes.

Damien Picard added this to the Compositing project 2023-07-31 00:08:42 +02:00

Yevgeny Makarov commented

2023-07-31 15:58:17 +02:00

Member

UI: Composting nodes Rotate socket rename #53574

UI: Composting nodes Rotate socket rename #53574

Damien Picard commented

2023-07-31 16:21:25 +02:00

Author

Member

UI: Composting nodes Rotate socket rename #53574

Oh I didn’t see this one, thanks. There was a PR for it a few months ago, but I can’t see its contents.

> UI: Composting nodes Rotate socket rename #53574 Oh I didn’t see this one, thanks. There was a PR for it a few months ago, but I can’t see its contents.

Damien Picard changed title from ~~UI: Rename "Degr" socket of the Rotate compositing node to "Angle"~~ to Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle"

2023-07-31 16:22:52 +02:00

Pratik Borhade requested review from Brecht Van Lommel 2023-08-04 10:58:43 +02:00

Brecht Van Lommel commented

2023-08-04 16:39:44 +02:00

Owner

Note that any socket renaming is still blocked on a solution to the identifier/name problem in the Python API. I do not plan to accept any renaming patches until that, and it's unclear if there will be time for it before 4.0.

Note that any socket renaming is still blocked on a solution to the identifier/name problem in the Python API. I do not plan to accept any renaming patches until that, and it's unclear if there will be time for it before 4.0.

Damien Picard commented

2023-08-04 17:28:47 +02:00

Author

Member

@brecht Should !108234 be reverted then?

@brecht Should !108234 be reverted then?

Brecht Van Lommel refused to review 2023-08-04 17:39:03 +02:00

Brecht Van Lommel commented

2023-08-04 17:39:07 +02:00

Owner

Breaking API compatibility for these compositing nodes is not as bad as for shader nodes, so I think that could stay. In that context maybe this change is also ok. But for shader nodes at least there are various exporters relying on socket names, so there it is blocked.

But I'm not a maintainer of the compositor, so not really the appropriate person to review.

Breaking API compatibility for these compositing nodes is not as bad as for shader nodes, so I think that could stay. In that context maybe this change is also ok. But for shader nodes at least there are various exporters relying on socket names, so there it is blocked. But I'm not a maintainer of the compositor, so not really the appropriate person to review.

Sergey Sharybin commented

2023-08-11 16:26:53 +02:00

Owner

We did accept some renames in the compositor recently, but it was also followed with the discussion of compatibility.

I do find it important to keep compatibility with exporters, and I don't really know why we should treat different node systems differently.

We had similar discussion (in the chat) about the previous rename, and we decided all things considered it was a low risk change. Mainly because the node was unlikely to be used (the motion tracking one is much more commonly used, as it has better interop with actual calibration data). The rotation node is probably more commonly used by scripts.

I didn't mind doing the risk assessment once, but doing it on a regular basis is rather time and energy consuming. Think it would be easier for everyone if we could delay this rename until python identifier and name are decoupled.

We did accept some renames in the compositor recently, but it was also followed with the discussion of compatibility. I do find it important to keep compatibility with exporters, and I don't really know why we should treat different node systems differently. We had similar discussion (in the chat) about the previous rename, and we decided all things considered it was a low risk change. Mainly because the node was unlikely to be used (the motion tracking one is much more commonly used, as it has better interop with actual calibration data). The rotation node is probably more commonly used by scripts. I didn't mind doing the risk assessment once, but doing it on a regular basis is rather time and energy consuming. Think it would be easier for everyone if we could delay this rename until python identifier and name are decoupled.

Damien Picard commented

2023-08-11 16:42:22 +02:00

Author

Member

That’s fine, I’ll mark this as WIP: in the mean time. Thanks.

That’s fine, I’ll mark this as WIP: in the mean time. Thanks.

Damien Picard changed title from ~~Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle"~~ to WIP: Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle"

2023-08-11 16:42:27 +02:00

This pull request has changes conflicting with the target branch.

source/blender/blenloader/intern/versioning_300.cc
source/blender/nodes/composite/nodes/node_composite_rotate.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u dp_rename_degr_angle:pioverfour-dp_rename_degr_angle

git checkout pioverfour-dp_rename_degr_angle

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

4 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#110637

No description provided.