PuckAnimator animates with `duration=0` when called with an empty `options` lambda.

[tomaszrybakiewicz]

tl;dr; Android SDK (31) ValueAnimator.clone() method doesn't copy duration value, causing PuckAnimator.userConfiguredAnimator.duration = 0.

Environment

• Android OS version: Android 12 (API 31)
• Devices affected: Pixel 6
• Maps SDK Version: 10.6.0

Observed behavior and steps to reproduce

We detected this issue when working on a patch for a Location puck's velocity.

Puck Velocity Fix relies on a mechanism that allows customization of ValueAnimator via LocationConsumer.onLocationUpdated() callback.

When NavigationLocationProvider calls LocationConsumer.onLocationUpdated(vararg location: Point, options: (ValueAnimator.() -> Unit)? = null) with non-null options, the PuckAnimator uses a separate animator instance (userConfiguredAnimator) to drive the animation (PuckAnimator.animate()).

// PuckAnimator.kt
class PuckAnimator(...) : ValueAnimator() {
  ... 
  init {
    setObjectValues(emptyArray<Any>())
    setEvaluator(evaluator)
    addUpdateListener {
      @Suppress("UNCHECKED_CAST")
      val updatedValue = it.animatedValue as T
      updateLayer(it.animatedFraction, updatedValue)
      updateListener?.invoke(updatedValue)
    }
    duration = LocationComponentConstants.DEFAULT_INTERVAL_MILLIS
    interpolator = DEFAULT_INTERPOLATOR
    userConfiguredAnimator = clone() // <---------- THIS doesn't copy duration value
  }
  ...
  fun animate(
    vararg targets: T,
    options: (ValueAnimator.() -> Unit)? = null
  ) {
    cancelRunning()
    if (options == null) {
      setObjectValues(*targets)
      start()
    } else {  // <------------ 
      options.invoke(userConfiguredAnimator)
      userConfiguredAnimator.setObjectValues(*targets)
      userConfiguredAnimator.start()
    }
  }
  ...
}

The userConfiguredAnimator instance is created via ValueAnimator.clone() method in the PuckAnimator constructor. It appears that cloned version doesn't copy duration field, leaving userConfiguredAnimator.duration = 0.

(screenshots of debbuger session showing PuckAnimator duration fields)

Expand for glitch recording

https://user-images.githubusercontent.com/2678039/174207047-6719eeb9-5d77-422f-bcb3-529ae8bb6cfc.mp4

Expected behaviour

Calling PuckPositionAnimator.animate() with an empty options block should animate Puck position with default duration (LocationComponentConstants.DEFAULT_INTERVAL_MILLIS).

Both animate calls below should result in the same animator behaviour:

var positionAnimator = PuckPositionAnimator(...)
val point = Point.fromLngLat(..., ...)

// First call without options
positionAnimator.animate(point)    

// Second call with empty options block
positionAnimator.animate(point, options = {})

Notes / preliminary analysis

After further investigation, we identified that in Android 12 SDK (API 31) neither the ValueAnimator.clone(), not its super class Animator.clone() clones the duration value, leaving it 0.

Proposed solution

PuckAnimator should set userConfiguredAnimator.duration default value to match PuckAnimator.duration.

// PuckAnimator.kt
class PuckAnimator(...) : ValueAnimator() {
  ... 
  init {
    setObjectValues(emptyArray<Any>())
    setEvaluator(evaluator)
    addUpdateListener {
      @Suppress("UNCHECKED_CAST")
      val updatedValue = it.animatedValue as T
      updateLayer(it.animatedFraction, updatedValue)
      updateListener?.invoke(updatedValue)
    }
    duration = LocationComponentConstants.DEFAULT_INTERVAL_MILLIS
    interpolator = DEFAULT_INTERPOLATOR
    userConfiguredAnimator = clone() // <---------- THIS doesn't copy duration value
    userConfiguredAnimator.duration = duration  // <---- MANUALLY copy duration value
  }
...
}

[kiryldz]

@tomaszrybakiewicz I assume it's also connected with https://github.com/mapbox/mapbox-maps-android/issues/1411 - I did also mention workaround there, could you please check if that will help in your case?

[tomaszrybakiewicz]

@kiryldz, Thanks. This issue is not a blocker and we already implemented a workaround in NavSDK. However this issue might still affect developers that decide to use their LocationProvider implementation with LocationComponentPlugin, instead suggested NavigationLocationProvider.