Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation overhaul #39

Draft
wants to merge 130 commits into
base: main
Choose a base branch
from
Draft

Implementation overhaul #39

wants to merge 130 commits into from

Conversation

Multirious
Copy link
Owner

@Multirious Multirious commented Jul 4, 2024

  1. Not flexible enough for curves

Problem:
This crate is not flexible enough for any curve more than a single f32, forcing user to create a custom system that's not within the crate's framework, if anyone even attempted it.
Solution:
The solution is to change how interpolator is implemented.
Don't store curve in an interpolator and make value generic.

From

struct Translation { ... }
impl Interpolator for Translation {
    type Item = Transform
    fn interpolate(&self, item: &mut Self::Item, value: f32) { ... }
}

To

struct Translation;
impl Set for Translation {
    type Item = Transform;
    type Value = f32;
    fn set(&self, item: &mut Self::Item, value: &Value) { ... }
}

Curve is now decoupled from interpolator and implementor should set the item with whatever it receive from the value argument.
To reflect this change, interpolator should be renamed to Set which directly describe what it should do.

Change TweenInterpolationValue(f32) to CurveValue<V>(V) to support the new design.

The original curve will be in another component called
AToB<V, C> { a: V, b: V, curve: C }
with curve that returns value within [0, 1]

  1. Registering new tween using systems is convoluted

Problem:
Users must extends this crate by registering systems directly to register new tween. I even add more trait just to smooth this up. It's also hard for this crate to make changes to systems without breaking changes. And system cannot access the App.
Solution:
Use a plugin. This causes a lot of code due to each systems needing its own plugin. I've avoided this in the early versions because of this reason, but after awhile I've realized the advantages of using plugin outweigh the boilerplate.

Some more changes...
todo!()

Add Setter trait
Add blanket impls like previously Interpolator trait
Add sprite module
Add SpriteColor setter
Remove Interpolation trait
Add ease_function_system
Add ease_closure_system
Remove ease_closure_system
Add AToB generic curve struct
Remove ease_function_system
Add a_to_b_ease_function_system
Register a_to_b_ease_function_system for
- f32
- Vec2
- Vec3
- Quat
- Color
Make value argument take a reference in Setter's set method
Make SpriteColor setter component
Add test plugin
Add ColorMaterial setter
Add SkipSetter component
Add set_component_from_curve_value_system
Add animation_target module to lib
Inside the module:
- Add AnimationTarget
- Add AnimationTargetResolver
- Add resolve_animation_target_system

Old animation target is not changed yet
…_system

And complete the system implementation
Add apply_resource_tween_system
Add apply_asset_tween_system
Add apply_handle_component_tween_system
Content:
- `ComponentTweenPlugin` with shortcut function `component`
- `ResourceTweenPlugin` with shortcut function `resource`
- `AssetTweenPlugin` with shortcut function `asset`
- `HandleComponentTweenPlugin` with shortcut function `handle_component`

Add DefaultTweenPlugins and has
- component::<SpriteColor, _, _>()
- asset::<ColorMaterial, _, _>()
- handle_component::<ColorMaterial, _, _>();
Remove `marker()` method
Change `TargetComponent`'s Default to `TargetComponent::None`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Even more generalized curve API
1 participant