search

OutpostUniverse / OP2Utility

C++ library for working with Outpost 2 related files and tasks.
MIT License
4 stars 0 forks source link

Read tileset #366

Closed Brett208 closed 3 years ago

Brett208 commented 3 years ago

@DanRStevens,

Currently, BItmapFile uses static functions to Write bitmaps. I'm trying to decide why I structured it that way. What do you think about removing the static functions with member functions? Maybe just:

BitmapFile::WriteIndexed(Stream::BidirectionalWriter& writer);
BitmapFile::WriteIndexed(std::string& filename); // Helper to automatically create a FileWriter stream

Instead of:

// Current Function Stack
static void WriteIndexed(std::string filename, uint16_t bitCount, int32_t width, int32_t height, std::vector<Color> palette, const std::vector<uint8_t>& indexedPixels);

// I think the use case below is that you could collect all the bitmap information from different sources 
// instead of first forming a BitmapFile structure in memory. We could leave this one in the overloads, 
// but I'd probably just delete it for our use cases.
static void WriteIndexed(Stream::BidirectionalWriter& seekableWriter, uint16_t bitCount, int32_t width, int32_t height, std::vector<Color> palette, const std::vector<uint8_t>& indexedPixels);

static void WriteIndexed(std::string filename, const BitmapFile& bitmapFile);
void WriteIndexed(Stream::BidirectionalWriter& writer) const;

Probably actually update code base in a different branch if this looks good.

DanRStevens commented 3 years ago

I think I agree with using member functions for write. Having a static factory method for read makes sense, though I would generally use a member function for write.