alpine-alpaca / asefile

Library for loading Aseprite files. Directly reads binary Aseprite files and does not require you to export files to JSON.
MIT License
43 stars 15 forks source link

Refactoring types and invariants #4

Open B-Reif opened 3 years ago

B-Reif commented 3 years ago

I'm opening this issue to explore how we can encode some existing panic/assert invariants in static typing. This is one design I'm considering:

pub enum ImageContent {
    Rgba {
        palette: Option<ColorPalette>,
        cels: CelsData<pixel::Rgba>,
        tilesets: TilesetsById<pixel::Rgba>,
    },
    Grayscale {
        palette: Option<ColorPalette>,
        cels: CelsData<pixel::Grayscale>,
        tilesets: TilesetsById<pixel::Grayscale>,
    },
    Indexed {
        palette: ColorPalette,
        transparent_color_index: u8,
        cels: CelsData<pixel::Indexed>,
        tilesets: TilesetsById<pixel::Indexed>,
    },
}

Any invariant behavior around the pixel format (a palette must be present, etc) would be encoded in this single ImageContent enum. This would replace the PixelFormat and Pixels enums, and additionally contain the dependent data. Cels and Tilesets would become generic over a pixel type.

Cels and Tilesets also contain a lot of non-pixel metadata. Making these types generic would make it harder to work with all of the non-image content. I also considered a design where the pixel data would live in a separate map from the non-pixel metadata:

// Only these pixel tables are generic.
// The rest of the data remains non-generic, in the existing top-level maps.
// This would prevent the generic types from "leaking" into other code that touches cels or tilesets.
pub struct CelPixelsById<T>(HashMap<CelId, Vec<T>>);
pub struct TilesetPixelsById<T>(HashMap<TilesetId, Vec<T>>);

pub enum ImagePixelContent {
    Rgba {
        palette: Option<ColorPalette>,
        cels: CelPixelsById<pixel::Rgba>,
        tilesets: TilesetPixelsById<pixel::Rgba>,
    },
    // etc
}

This indirection would allow users to use Tilesets and Cels from multiple files without regard for their pixel format.

alpine-alpaca commented 3 years ago

Hm, I haven't found a good time to fully wrap my head around how the tileset stuff fits in.

But I have a few quick thoughts on the handling of indexed data.

The validator should still check for out of bounds indices. But if it has a bug there won't be a crash, just some transparent pixels.

The one ugly spot would be the handling of background-layer or not. I think that info shouldn't be moved into the cel, so it must be handled somewhere else.