Differential D11263
Fix T87854: Add clamp option to Path Animation Authored by Sebastian Parborg (zeddb) on May 14 2021, 7:31 PM.
Details Previously, the "follow path constraint" and "follow parented curve" were clamped. Add back an option to get the old behavior in the "Path animation" settings. @Sybren A. Stüvel (sybren) Should I just bump the file version or should I leave it in the "put stuff here until the next version bump" area?
Diff Detail
Event TimelineComment Actions The code looks good to me. As for the versioning, I don't think it's a good idea to keep things in the "until the next version bump", so let's bump the file version. I don't know how this reflects on the files that have already been saved with some 3.0-alpha version, and what the policy is here. Maybe @Dalai Felinto (dfelinto) or @Campbell Barton (campbellbarton) can give us some guidance here? Comment Actions Regardless of 2.93/3.0 branches, the code as it is means it will run for every file, so any change you make will be lost. You need to do version bump. (increase BLENDER_FILE_SUBVERSION to 17). Am I missing something here? Comment Actions It was mostly just to check if we had some bulk rule for bumping file version now. I'll do that before I commit. Is there any other things that I need to adress? Comment Actions My doubt was about what happens when:
I think this means that the file won't be versioned (because it's newer than 2.93), so the CU_PATH_CLAMP won't be set. Maybe we want to have the versioning code in both the 2.93 and the 3.0 versioning functions? Comment Actions You could have the reverse issue now as well that you have a file where the user didn't want it to be clamped. We can't possibly make the right call for all the cases here. My vote would just to have this in the 2.93 version. Comment Actions I'm confused. I didn't expect this to be in the agenda for 2.93 since it doesn't seem to be addressing a critical issue. If you do need this in 2.93 you just need to be careful about version control when merging to master (for 2.93 itself is just a regular version bump). We do something similar in versioning_290.c: Comment Actions I just had a face-to-face chat with @Sergey Sharybin (sergey) about this, and he suggests:
This keeps things simple and predictable, so I agree with him here. Update: I think we can just move the versioning code to the if (!MAIN_VERSION_ATLEAST(bmain, 293, 19)) { block; I don't think we need to bump the file version for this particular change, given that the actual change in behaviour was introduced earlier anyway. Comment Actions Sounds good! I agree as well. Do you want me to update the patch with those changes before you accept? |