Added: PathValueProvider

[kalugny]

Hi,
Thank you for the great work on Lottie.

This PR adds the ability to replace a Path in the lottie, similarly to PointValueProvider.

In order to accomplish this I had to make BezierPath public.

Thanks,
Yuval

[calda]

How have you been testing this? Can you add a snapshot test case for this functionality?

You'll need to add a case for this to SnapshotConfiguration.swift, you can follow the examples of the existing value provider test cases. If you add a new sample animation file, you can add it at Tests/Samples/Issues/pr_2376.json.

[kalugny]

I encountered an issue with the tests, but I think it may be a problem with how the tests work.

recordHierarchyKeypath, which is used to log all the keypaths in use by the Core Animation engine, logs whatever is used in a ValueProvider. This now includes a Path property.

The test fails because the Path property seems to be missing in the Main Thread engine.

But the PathValueProvider works for both, so I think the test is at fault.

I will add Snapshot tests as you suggested.

[calda]

Was this to work around the failure of testCoreAnimationEngineKeypathLogging?

Instead, please re-record the snapshots. You can add isRecording = true locally, and then re-run the test:

  func testCoreAnimationEngineKeypathLogging() async {
+   isRecording = true
    for animationKeypathTestAnimation in animationKeypathTestAnimations {
      await snapshotHierarchyKeypaths(
        animationName: animationKeypathTestAnimation,
        configuration: LottieConfiguration(renderingEngine: .coreAnimation))
    }
  }

We'll just want to make sure the updated snapshot output (which you'll have to commit) is reasonable.

[calda]

We shouldn't change the code to work around a test failure, unless the test failure is highlighting a genuine bug or compatibility issue. Instead we should adjust the test cases as necessary.

[calda]

The test fails because the Path property seems to be missing in the Main Thread engine.

If this was the issue, then we need to fix how the Path property is set up. Although I didn't see that issue when I pulled your code and ran the tests (I only saw a failure in testCoreAnimationEngineKeypathLogging).

[kalugny]

OK, then it seems I misunderstood how the test works.
I'll record the tests and make sure they pass as well as add tests as necessary.

[calda]

Hi @kalugny, since there hasn't been activity on this PR recently I'm going to go ahead and close it. If you're still interested in working on this please feel free to! We can reopen it if/when you find chance to continue looking at it.