From 8a4fdd2e7ebdaa310e9fadc1c221b0235cc8070d Mon Sep 17 00:00:00 2001 From: Titanium Brain Date: Sun, 3 May 2026 13:59:00 +0100 Subject: [PATCH] fix(DurFile): resolve issues from review Removes unnecessary checks, improves naming and syntax. --- src/animations/DurFile.zig | 120 ++++++++++++++++++++++++------------- 1 file changed, 77 insertions(+), 43 deletions(-) diff --git a/src/animations/DurFile.zig b/src/animations/DurFile.zig index b180880..d17ec29 100644 --- a/src/animations/DurFile.zig +++ b/src/animations/DurFile.zig @@ -75,48 +75,42 @@ const DurFormatRaw = struct { // Validate data and return a valid DurFormat // Consumes `self`, making it unusable after pub fn validate(self: *DurFormatRaw) !DurFormat { - if (self.formatVersion != null and - self.colorFormat != null and - self.encoding != null and - self.framerate != null and - self.columns != null and - self.lines != null and - self.frames.items.len >= 1) - { - // v8 may have breaking changes like changing the colormap xy direction - // (https://github.com/cmang/durdraw/issues/24) - const formatVersion = self.formatVersion orelse return error.NotValidFile; - if (formatVersion != 7) return error.NotValidFile; + // v8 may have breaking changes like changing the colormap xy direction + // (https://github.com/cmang/durdraw/issues/24) + const formatVersion = self.formatVersion orelse return error.MissingFieldVersion; + if (formatVersion != 7) return error.UnsupportedVersion; - const colorFormatStr = self.colorFormat orelse return error.NotValidFile; - // Code currently only supports 16 and 256 color format only - const colorFormat: DurColorFormat = - if (eql(u8, colorFormatStr, "16")) .sixteen else if (eql(u8, colorFormatStr, "256")) .two_fifty_six else return error.NotValidFile; + const colorFormatStr = self.colorFormat orelse return error.MissingFieldColorFormat; + // Code currently only supports 16 and 256 color format only + const colorFormat: DurColorFormat = + if (eql(u8, colorFormatStr, "16")) .@"16" else if (eql(u8, colorFormatStr, "256")) .@"256" else return error.UnsupportedColorFormat; - // Code currently supports only utf-8 encoding - const encoding: DurEncoding = if (eql(u8, self.encoding.?, "utf-8")) .utf_8 else return error.NotValidFile; + const encodingStr = self.encoding orelse return error.MissingFieldEncoding; + // Code currently supports only utf-8 encoding + const encoding: DurEncoding = if (eql(u8, encodingStr, "utf-8")) .utf_8 else return error.UnsupportedEncoding; - // Sanity check on file - const columns = std.math.cast(u32, self.columns.?) orelse return error.NotValidFile; - const lines = std.math.cast(u32, self.lines.?) orelse return error.NotValidFile; - if (self.framerate.? <= 0) return error.NotValidFile; + if (self.framerate == null) return error.MissingFieldFramerate; + if (self.framerate.? <= 0) return error.InvalidFramerate; + const framerate: f64 = self.framerate.?; - // transfer ownership - const frames = self.frames; + // Sanity check on file + if (self.columns == null or self.lines == null) return error.MissingDimensions; + const columns = std.math.cast(u32, self.columns.?) orelse return error.InvalidColumnCount; + const lines = std.math.cast(u32, self.lines.?) orelse return error.InvalidLineCount; - return .{ - .allocator = self.allocator, - .formatVersion = formatVersion, - .colorFormat = colorFormat, - .encoding = encoding, - .framerate = self.framerate.?, - .columns = columns, - .lines = lines, - .frames = frames, - }; - } + // transfer ownership + const frames = self.frames; - return error.NotValidFile; + return .{ + .allocator = self.allocator, + .formatVersion = formatVersion, + .colorFormat = colorFormat, + .encoding = encoding, + .framerate = framerate, + .columns = columns, + .lines = lines, + .frames = frames, + }; } fn parse_dur_from_json(self: *DurFormatRaw, allocator: Allocator, dur_json_root: Json.Value) !void { @@ -187,8 +181,8 @@ const DurFormatRaw = struct { }; const DurColorFormat = enum { - sixteen, - two_fifty_six, + @"16", + @"256", }; const DurEncoding = enum { utf_8 }; @@ -414,6 +408,7 @@ pub fn init( frame_delay: u16, ) !DurFile { var dur_movie_raw: DurFormatRaw = .init(allocator); + defer dur_movie_raw.deinit(); dur_movie_raw.create_from_file(allocator, io, file_path) catch |err| switch (err) { error.FileNotFound => { @@ -428,15 +423,54 @@ pub fn init( }; var dur_movie = dur_movie_raw.validate() catch |err| switch (err) { - error.NotValidFile => { - try log_file.err(io, "tui", "dur_file loaded was invalid or not a dur file!", .{}); + error.MissingFieldVersion => { + try log_file.err(io, "tui", "dur_file loaded was invalid: missing field formatVersion!", .{}); + return err; + }, + error.UnsupportedVersion => { + try log_file.err(io, "tui", "dur_file loaded was invalid: unsupported version!", .{}); + return err; + }, + error.MissingFieldColorFormat => { + try log_file.err(io, "tui", "dur_file loaded was invalid: missing field formatVersion!", .{}); + return err; + }, + error.UnsupportedColorFormat => { + try log_file.err(io, "tui", "dur_file loaded was invalid: unsupported colorFormat!", .{}); + return err; + }, + error.MissingFieldEncoding => { + try log_file.err(io, "tui", "dur_file loaded was invalid: missing field encoding!", .{}); + return err; + }, + error.UnsupportedEncoding => { + try log_file.err(io, "tui", "dur_file loaded was invalid: unsupported encoding!", .{}); + return err; + }, + error.MissingFieldFramerate => { + try log_file.err(io, "tui", "dur_file loaded was invalid: missing field framerate!", .{}); + return err; + }, + error.InvalidFramerate => { + try log_file.err(io, "tui", "dur_file loaded was invalid: negative framerate value found!", .{}); + return err; + }, + error.MissingDimensions => { + try log_file.err(io, "tui", "dur_file loaded was invalid: missing field(s) lines and/or columns!", .{}); + return err; + }, + error.InvalidColumnCount => { + try log_file.err(io, "tui", "dur_file loaded was invalid: columns value falls outside of supported range!", .{}); + return err; + }, + error.InvalidLineCount => { + try log_file.err(io, "tui", "dur_file loaded was invalid: lines value falls outside of supported range!", .{}); return err; }, }; - dur_movie_raw.deinit(); // 4 bit mode with 256 color is unsupported - if (!full_color and dur_movie.colorFormat == .two_fifty_six) { + if (!full_color and dur_movie.colorFormat == .@"256") { try log_file.err(io, "tui", "dur_file can not be 256 color encoded when not using full_color option!", .{}); dur_movie.deinit(); return error.NotFullColor; @@ -466,7 +500,7 @@ pub fn init( .frame_delay = frame_delay, .dur_movie = dur_movie, .frame_time = frame_time, - .is_color_format_16 = dur_movie.colorFormat == .sixteen, + .is_color_format_16 = dur_movie.colorFormat == .@"16", .offset_alignment = offset_alignment, .offset = offset, };