SpritesheetLibrary::new_clip

[UkoeHB]

Had some issues with this method.

• You cannot make marker IDs in the method because the library is borrowed as mutable to call new_clip. A reference to the library from within the closure would solve that.
• You cannot access anything mutably in the closure since it's a Fn method. It should only need to be a FnOnce.

[merwaaan]

@UkoeHB Thanks for the feedback!

• I'm afraid that adding the library as a parameter would make things more verbose and would not add much in terms of ergonomics. What's your use-case for creating markers inside of new_clip instead of before the call?

• I'm not too familiar with the subtleties of closures. Does using FnMut instead of Fn/FnOnce seem reasonnable? I have this working in a branch, but I just want some input since you seem more confortable with the topic.

[UkoeHB]

Hi @merwaaan

• The main problem is if you have multiple markers, then defining them outside the closure requires allocating which isn't ideal (not a huge problem).
• A FnOnce should be used when you only call a closure at most one time. A Fn and FnMut are for closures that can be called multiple times. If it's Fn then you can't mutate any captured variables.

My use-case was solved by defining markers outside the closure. It was just a small headache when I wanted to make marker ids at the same spot I call builder.add_marker().

fn setup_animation(
    image_map: &ImageMap,
    library: &mut SpritesheetLibrary,
    atlas_layouts: &mut Assets<TextureAtlasLayout>,
    animations: &mut SpriteAnimations,
    spritesheet_markers: &mut SpritesheetMarkers,
    mut animation: SpriteAnimation,
)
{
    // Animation clips.
    let clips: Vec<(SpriteAnimationClip, Vec<(AnimationMarkerId, usize)>)> = animation
        .clips
        .drain(..)
        .map(|mut clip| {
            let markers: Vec<(AnimationMarkerId, usize)> = clip
                .markers
                .drain(..)
                .map(|(name, frame)| {
                    let marker_id = library.new_marker();
                    spritesheet_markers.insert(marker_id, name);
                    (marker_id, frame)
                })
                .collect();

            (clip, markers)
        })
        .collect();

    let clip_ids: Vec<AnimationClipId> = clips
        .iter()
        .map(|(clip, markers)| {
            library.new_clip(|builder| {
                match clip.frames.clone() {
                    AnimationFrames::Row(row) => {
                        builder.push_frame_indices(Spritesheet::new(animation.columns, animation.rows).row(row));
                    }
                    AnimationFrames::Column(column) => {
                        builder.push_frame_indices(
                            Spritesheet::new(animation.columns, animation.rows).column(column),
                        );
                    }
                    AnimationFrames::Frame(frame) => {
                        builder.push_frame_indices(vec![frame]);
                    }
                    AnimationFrames::Frames(frames) => {
                        builder.push_frame_indices(frames);
                    }
                }

                for (marker_id, marker_frame) in markers.iter() {
                    builder.add_marker(*marker_id, *marker_frame);
                }
            })
        })
        .collect();

    // Animation.
    let anim_id = library.new_animation(|builder| {
        // Set frame duration.
        builder.set_duration(AnimationDuration::PerFrame(animation.frame_time));

        // Set cycles.
        if let Some(cycles) = animation.loops {
            builder.set_repeat(AnimationRepeat::Cycles(cycles as u32));
        }

        // Add clips.
        for clip_id in clip_ids.iter() {
            builder.add_stage((*clip_id).into());
        }
    });

    // Save the animation's sprite info.
    let image = image_map.get(animation.image);
    let layout = atlas_layouts.add(TextureAtlasLayout::from_grid(
        animation.size,
        animation.columns,
        animation.rows,
        animation.padding,
        animation.offset,
    ));
    animations.insert(anim_id, animation.name, image, layout);
}

[merwaaan]

@UkoeHB Creating animations has been made way more straightforward with the latest commits so things should be easier for you now.