loodakrawa / SpriterDotNet

A pure C# Spriter implementation
zlib License
220 stars 75 forks source link

Simplify public interface. #118

Closed Martenfur closed 4 years ago

Martenfur commented 4 years ago

This is what you need to do to set up a single animation.

public static readonly Config Config = new Config
{
    MetadataEnabled = true,
    EventsEnabled = true,
    PoolingEnabled = true,
    TagsEnabled = true,
    VarsEnabled = true,
    SoundsEnabled = false
};
DefaultProviderFactory<ISprite, SoundEffect> factory = new DefaultProviderFactory<ISprite, SoundEffect>(Constants.Config, true);

Stack<SpriteDrawInfo> drawInfoPool = new Stack<SpriteDrawInfo>();

SpriterContentLoader loader = new SpriterContentLoader(Content, scmlPath);
loader.Fill(factory);
foreach (var entity in loader.Spriter.Entities)
{
    var animator = new MonoGameAnimator(entity, factory, drawInfoPool);
    animators.Add(animator);
}

This is horribly overcomplicated. Ideally, it should look roughly like this:

var template = new SpriterAnimatorTemplate(Content, scmlPath);
var animator = template.MakeAnimator();

Additional configs could be moved away to a separate controller class.

loodakrawa commented 4 years ago

Not really. You can do something as simple as:

var factory = new DefaultProviderFactory<ISprite, SoundEffect>(new Config());
var loader = new SpriterContentLoader(Content, scmlPath);
loader.Fill(factory);
var animator = new MonoGameAnimator(loader.Spriter.Entities.First(), factory);
Martenfur commented 4 years ago

My version is twice as simple, tho. : - )

Also this way you'll create a separate pool for every single animation, which is a waste. n fact, I don't understand why you even need to expose the pool and pass it separately. You can just make is a private static field.

loodakrawa commented 4 years ago

I agree, it is twice as simple.

I exposed the pool because:

As I mention for other things as well, version 2 SHOULD handle all of this better, without really the need for pooling in the first place.