The store_original_bezt_arrays function previously lived in graphkeys_decimate_invoke
Future graph slider operators will need to use this code as well, so it is extracted here
No functional changes
Depends on D12486
Details
Details
- Reviewers
Sybren A. Stüvel (sybren) - Maniphest Tasks
- T81785: Implementation: Modal Key Manipulation Operators
- Commits
- rBb2e9f35c5eb2: Cleanup: Extract function to store bezt arrays
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
The documentation changed from:
Construct a list with the original bezt arrays so we can restore them during modal operation.
to:
Construct a list with the original bezt arrays so we can use them during modal operation.
It would seem that the purpose of the code has changed, from being able to restore the data to being able to use the data. Of course restoring is a use, but a "no functional changes" patch shouldn't really change the semantics either. I think "restore" would be better to use here, not just because it was used before, but also because to me "restore" implies that the only use of the data is to put it back where it came from. Using the word "use" here would imply (at least to me) that the data would be used directly, so for other purposes than just putting it back where it was found, as well.
The comment should IMO also describe where that backup is stored. The function doesn't return anything, so it's not obvious how it works from just reading the comment + the function declaration.
How would you feel about also refactoring the restoration code into a separate function? That way there are two symmetrical functions; one to make the backup, and one to restore it.
Comment Actions
- Change comment back and describe where data is stored
The reason why it changed, was because future operators will use it in different ways than just resetting curves.
I think the change came from when I first split up my work.
Good catch
Regarding the separate restore function: it already exists as decimate_reset_bezts
In a future patch I will move and rename this function as well