[feature/kotlin] Kotlin code in animator module

[ivybae]

Hi, @tarek360 :)
As you see in this PR, I added kotlin source set in animator module.
I think we can remove java source set after we discuss about changes.

1. RepeatMode
   I changed RepeatMode type to sealed class.

2. @JvmStatic fun animate
   I think animator module can be called from Java.
   And, I tested in app module MainActivity.

3. apply
   I used many apply inline function in AnimationBuilder class.
   So, I want to change more like :

RichPathAnimator.create { paths = allPaths; duration = 800; .... }

I need your advice.
Thanks!

[tarek360]

we can do like so:

val evaluatedNodes = this.evaluatedNodes?.let { it } ?: PathParserCompat.deepCopyNodes(startPathDataNodes)
this.evaluatedNodes = evaluatedNodes

then we can remove !! from evaluatedNodes!!

[ivybae]

Yes, I agree with that. You can see in 7bb9d6a.

[tarek360]

we can move applyAnimatorProperties(valueAnimator, path) inside apply{} block then can remove val valueAnimator

like below:

ValueAnimator.ofFloat(*values).apply {
    addUpdateListener { animation ->
        listener?.update(path, animation.animatedValue as Float)
        path.onRichPathUpdatedListener?.onPathUpdated()
    }
    applyAnimatorProperties(this, path)
}

[ivybae]

Thanks. You can see in b9ce4a1.

[tarek360]

First of all, This is a great PR and thank you for your efforts!

I agree with the three points you mentioned in the PR description, especially the second and the third points, I always prefer to have backward compatibility as you did and also I would like to see that Kotlin style you suggested :)

Thanks!

[ivybae]

First of all, This is a great PR and thank you for your efforts!

I agree with the three points you mentioned in the PR description, especially the second and the third points, I always prefer to have backward compatibility as you did and also I would like to see that Kotlin style you suggested :)

Thanks!

@tarek360 Hi, Thank you for saying so.
I'm concerned three things about Kotlin Builder style.

First, If we are going to support backward compatibility and add new Kotlin style, then I think we should modify like below in AnimationBuilder.

[ AnimationBuilder.kt ]

    var trimPathEnd = floatArrayOf(0f)
        set(value) {
            property("trimPathEnd", *value)
            field = value
        }

    fun trimPathEnd(vararg values: Float) = apply {
        trimPathEnd = values
    }

Second, If we add animate method in RichPathAnimator, then we can keep similar with Java.
[ RichPathAnimator.kt ]

companion object {
        private val animationBuilders = arrayListOf<AnimationBuilder>()
        @JvmStatic fun animate(vararg paths: RichPath): AnimationBuilder {
            val viewAnimator = RichPathAnimator()
            return viewAnimator.addAnimationBuilder(*paths)
        }

        fun animate(vararg paths: RichPath, block:AnimationBuilder.()->Unit){
            AnimationBuilder(RichPathAnimator(), *paths).apply(block).run {
                animationBuilders.add(this)
                start()
            }
        }
    }

Last, we create animation in Kotlin like this :
I moved start() method to AnimationBuilder block before.

      RichPathAnimator.animate(*allPaths) {
            trimPathEnd = floatArrayOf(0f, 1f)
            // ..
        }

What about that?
I'm looking forward your advice.
Thank you. Have a good day!

[tarek360]

Hi, for the 1st and 2nd points, if we keep it, for now, I see it still works with Java correctly and but not Kotlin style, so I think later after we merge this PR, we can have another version of the builder with Kotlin DSL.

The 3rd point, I'd get rid of the asterisk!

What do you think?

[ivybae]

@tarek360
I agree with that. paths is vararg type, so we should change to List type.

And, I also think we can develop Kotlin DSL after merging this :)

[ivybae]

@tarek360
Hi, I modified vararg parameter paths to Array type.
And I also added thenAnimate method overloading like below.
Because I thought it could be better that not to change client code in java.
fun thenAnimate(path: RichPath): AnimationBuilder { return thenAnimate(arrayOf(path)) }
please check this out 3747986.

[tarek360]

I think the code like
animate(path1, path2, path3) or thenAnimate(path1, path2, path3)
will be broken.
do you think we should overload this as well?
or maybe something like:

animate(path1)
animate(path1, path2, path3)
animateAll(paths)
or maybe you you have a better idea, what do you think?

[ivybae]

@tarek360
I thought animate and thenAnimate have procedural order.
I guess that's why you said that it will be broken, right?

Then you means that we are going to replace thenAnimate to animate and animateAll?

[tarek360]

@step4me this PR is getting old :)
I'd merge it and fix any backward compatibility issue later.
so don't worry about that, just remove the java source and add back com. to the Kotlin source package, please!