Creating Keyframes from Box<dyn EasingFunction> directly

[PSteinhaus]

I've finally come around to actually creating an animation example for ggez, showcasing frame-by-frame animation using keyframe.

During this effort I spent a bit of time working around the fact that both Keyframe::new and the keyframes! macro need to know the type of EasingFunction which they'll get fed at compile time, even though it'll just get saved as a Box internally anyway. Not being able to pass a Box<dyn EasingFunction> led to massive amounts of code duplication relative to the size of the example.

After a while I gave up, realising that the proper way to address this could just be adding another constructor taking a boxed EasingFunction directly. This would lead to code like this:

match ease_enum {
    EasingEnum::Linear => keyframes![
        (ball_pos_start, 0.0, Linear),
        (ball_pos_end, duration, Linear)
    ],
    EasingEnum::EaseIn => keyframes![
        (ball_pos_start, 0.0, EaseIn),
        (ball_pos_end, duration, EaseIn)
    ],
    EasingEnum::EaseInOut => keyframes![
        (ball_pos_start, 0.0, EaseInOut),
        (ball_pos_end, duration, EaseInOut)
    ],
    EasingEnum::EaseOut => keyframes![
        (ball_pos_start, 0.0, EaseOut),
        (ball_pos_end, duration, EaseOut)
    ],
    EasingEnum::EaseInCubic => keyframes![
        (ball_pos_start, 0.0, EaseInCubic),
        (ball_pos_end, duration, EaseInCubic)
    ],
    EasingEnum::EaseOutCubic => keyframes![
        (ball_pos_start, 0.0, EaseOutCubic),
        (ball_pos_end, duration, EaseOutCubic)
    ],
    EasingEnum::EaseInOutCubic => keyframes![
        (ball_pos_start, 0.0, EaseInOutCubic),
        (ball_pos_end, duration, EaseInOutCubic)
    ],
    EasingEnum::Bezier => {
        let bezier_function = BezierCurve::from([0.6, 0.04].into(), [0.98, 0.335].into());
        keyframes![
            (ball_pos_start, 0.0, bezier_function),
            (ball_pos_end, duration, bezier_function)
        ]
    }
    _ => panic!(),
    }
}

being shortened to something like this:

fn easing_function(easing_enum: &EasingEnum) -> Box<dyn EasingFunction> {
    Box::new(match ease_enum {
        EasingEnum::Linear => Linear,
        EasingEnum::EaseIn => EaseIn,
        EasingEnum::EaseInOut => EaseInOut,
        EasingEnum::EaseOut => EaseOut ,
        EasingEnum::EaseInCubic => EaseInCubic,
        EasingEnum::EaseOutCubic => EaseOutCubic,
        EasingEnum::EaseInOutCubic => EaseInOutCubic,
        EasingEnum::Bezier => BezierCurve::from([0.6, 0.04].into(), [0.98, 0.335].into()),
        _ => panic!(),
    })
}

keyframes![
    (ball_pos_start, 0.0, easing_function(ease_enum)),
    (ball_pos_end, duration, easing_function(ease_enum))
]

This would be immensely helpful for me, as there's a second portion of code (even worse than the first one), where I need to make basically the same match, but with each case being a bit more complicated (though practically the, same apart from the easing function used).

I considered just starting a PR, but I thought I'd start this issue first to ask how you feel about it.

[hannesmann]

Having a second constructor that takes Box<dyn EasingFunction + Send + Sync> makes sense since that's what the function is stored as internally.

I poked around the code a little and while creating a second constructor is easy (something like Keyframe::new_dynamic(value, time, box)) I couldn't get the macro working while keeping the same semantics as before. I would still like if sequences could be created with the macro without having to create a Box explicitly.

If you already have something implemented create a PR and I'll merge it. I pushed a few small fixes so once that's done I'll bump the version to 1.0.4.

[PSteinhaus]

I couldn't get the macro working while keeping the same semantics as before. I would still like if sequences could be created with the macro without having to create a Box explicitly.

That's the nontrivial part for me as well, as I'm still very much a newbie when it comes to writing macros. I guess I'll just try it out. In case I can't get it to work either, would you also accept a PR only adding Keyframe::new_dynamic(value, time, box) as well?

[hannesmann]

In case I can't get it to work either, would you also accept a PR only adding Keyframe::new_dynamic(value, time, box) as well?

Sure, that seems like the easiest option. :)