feat: the log names a mod's sprite shape that is cut short

docs/formats/spr.md

@@ -45,8 +45,9 @@ The placeholders hold bounds near `maxInt(i32)` and no pixel data. They occur in
 `ifhard.spr` and `ifsoft.spr`.
 
 A block with a shape's header but rows that run past the block, as a broken set from a mod can
-have, isn't a shape either, so OpenReliant draws nothing for it. The log doesn't say so yet
-([#1079](https://github.com/OpenReliant/openreliant/issues/1079)).
+have, isn't a shape either, so OpenReliant draws nothing for it. Its header's bounds are sane,
+unlike the placeholders', which tells it apart (`spr.Sprite.cutShort`), and as OpenReliant opens a
+set, the log names each such shape (`hud.warnCutShort`). None of the shipped sets has one.
 
 A remap table maps each palette index to another. The HUD sets open with a run of them, the first
 being the identity, `00 01 02 ... FF`.

docs/guide/modding.md

@@ -1454,6 +1454,7 @@ A script's messages and errors follow its mod's name, as above;
 | `skipping the mod <archive>: checking <file> failed: ...` | The archive doesn't match its checksum: download it again, or pack it again with `--checksum` ([Checksums](#checksums)) |
 | `<mod>: mod.ini: the <kind> '<name>' ...` | The entry the message names, and what it says is wrong ([New ships, guns, missiles and pilots](#new-ships-guns-missiles-and-pilots), [Game variables](#game-variables)) |
 | `skipping the picture that replaces <name>: it's <size>, and the original is <size>` | That picture must keep the original's size ([Pictures](#pictures)) |
+| `<set>: shape <n> is cut short: its rows run past the end of its block, so it draws nothing` | The set's file is broken at that shape: make the set again, or give a picture in its place ([Shapes](#shapes)) |
 | `the <map> of <texture> is left out: it is <size> and its picture <size>` | A map must be the same size as its texture ([Material maps](#material-maps)) |
 | `the <map> of <texture> is left out: it has <n> mipmap levels and its picture <n>` | Save the DDS or KTX2 map with all its mipmaps, as many as the picture has ([Compression](#compression)) |
 | `<file> is left out: it is compressed in <format>, which the device doesn't take` | The GPU doesn't take that format; use a PNG file instead ([Compression](#compression)) |

src/engine/game/create/library.zig

@@ -136,7 +136,9 @@ pub const TypeCache = struct {
                 log.warn("the schematic {s} is left out: {s}", .{ files.layout, @errorName(err) });
                 break :found null;
             };
-            break :found try .init(gpa, try spr.Sprite.parse(bytes), cache.global_palette, .of(cache.resources.mods, files.pictures));
+            const set = try spr.Sprite.parse(bytes);
+            hud.warnCutShort(set, files.layout);
+            break :found try .init(gpa, set, cache.global_palette, .of(cache.resources.mods, files.pictures));
         } else null;
         cached.wire_frame = try cache.ownShapes(gpa, ship_type, .wire_frame);
         cached.wing_icon = try cache.ownShapes(gpa, ship_type, .wing_icon);

src/engine/game/hud.zig

@@ -555,6 +555,15 @@ test globalPalette {
     try std.testing.expectEqual(null, globalPalette(try .parse(&empty)));
 }
 
+/// Logs each shape of the set `name` whose rows run past its block (`spr.Sprite.cutShort`), as a
+/// broken set from a mod can hold: it draws nothing
+/// ([#1079](https://github.com/OpenReliant/openreliant/issues/1079)).
+pub fn warnCutShort(set: spr.Sprite, name: []const u8) void {
+    for (0..set.count()) |index| {
+        if (set.cutShort(index)) log.warn("{s}: shape {d} is cut short: its rows run past the end of its block, so it draws nothing", .{ name, index });
+    }
+}
+
 /// A set of the display's shapes, with an image made of each shape when it's first drawn. No entry
 /// in a shipped set names a palette, so VFX draws each shape with the global palette, which
 /// `hud_draw` takes from the display's set. Without a global palette, a shape uses the nearest
@@ -2103,6 +2112,7 @@ pub const Resources = struct {
     /// and the power ball, which `hud_init` works out.
     pub fn load(gpa: Allocator, archive: bigfile.Hog, shapes: spr.Sprite, outlines: ?*outline.Outlines) !Resources {
         const global = globalPalette(shapes);
+        warnCutShort(shapes, hardware_shapes);
         var art: Art = try .init(gpa, shapes, global, .of(archive.mods, hardware_shapes));
         errdefer art.deinit(gpa);
         var font = try openFont(gpa, archive, font_name, global, outlines);

src/engine/game/interface/canvas.zig

@@ -355,6 +355,7 @@ pub const Shapes = struct {
             gpa.free(bytes);
             return null;
         };
+        hud.warnCutShort(set, name);
         const art = hud.Art.init(gpa, set, null, pictures) catch {
             gpa.free(bytes);
             return null;

src/engine/genilib/interf.zig

@@ -152,6 +152,14 @@ pub const Request = union(enum) {
     quit,
 };
 
+/// The sprite set `name` of `archive`, read as `file`, with the mods' pictures that replace its
+/// shapes, and its cut-short shapes logged (`hud.warnCutShort`).
+fn openShapes(gpa: Allocator, archive: bigfile.Hog, file: []const u8, name: []const u8) !hud.Art {
+    const set = try spr.Sprite.parse(file);
+    hud.warnCutShort(set, name);
+    return .init(gpa, set, null, .of(archive.mods, name));
+}
+
 /// What the front end draws with, which it opens as it starts and frees as it ends: the fonts its
 /// start-up opens (`interface_init`, `0x004288E0`), the developers' font the display opens
 /// (`font_01`), the dialog's shapes and ABOUT STARLANCER's box's, which `about_box` reads as it
@@ -204,11 +212,11 @@ pub const Resources = struct {
             .large = large,
             .small = small,
             .developer = developer,
-            .dialog = try .init(gpa, try spr.Sprite.parse(files.get(.dialog)), null, .of(archive.mods, names.get(.dialog))),
+            .dialog = try openShapes(gpa, archive, files.get(.dialog), names.get(.dialog)),
             .about = undefined,
         };
         errdefer resources.dialog.deinit(gpa);
-        resources.about = try .init(gpa, try spr.Sprite.parse(files.get(.about)), null, .of(archive.mods, names.get(.about)));
+        resources.about = try openShapes(gpa, archive, files.get(.about), names.get(.about));
         errdefer resources.about.deinit(gpa);
         try resources.show(.main_menu);
         return resources;
@@ -222,7 +230,7 @@ pub const Resources = struct {
         const gpa = resources.gpa;
         const file = try resources.archive.readFile(gpa, shown.shapes);
         errdefer gpa.free(file);
-        var art: hud.Art = try .init(gpa, try spr.Sprite.parse(file), null, .of(resources.archive.mods, shown.shapes));
+        var art = try openShapes(gpa, resources.archive, file, shown.shapes);
         errdefer art.deinit(gpa);
         try resources.background.set(gpa, resources.archive, shown.background);
         resources.dropShapes();

src/formats/spr.zig

@@ -207,6 +207,14 @@ pub const Sprite = struct {
         return .classify(sprite.data, sprite.entries[index].offset, sprite.blockEnd(index));
     }
 
+    /// Whether block `index` is a shape whose rows run past its block, as a broken set from a mod
+    /// can hold: a block `classify` takes for a placeholder, but whose header's bounds are sane,
+    /// which the game's own placeholders' aren't. One of 256 bytes reads as a remap table instead.
+    pub fn cutShort(sprite: Sprite, index: usize) bool {
+        if (sprite.block(index) != .placeholder) return false;
+        return saneHeader(sprite.data, sprite.entries[index].offset, sprite.blockEnd(index)) != null;
+    }
+
     /// The palette a shape draws with: the nearest one at or before it, since a file may carry
     /// several and each group of shapes follows its own.
     pub fn paletteFor(sprite: Sprite, index: usize) ?*const [palette_size]u8 {
@@ -230,24 +238,30 @@ pub const Sprite = struct {
 
 /// Reads a shape at `offset`, or null if the bytes there are not one.
 fn readShape(data: []const u8, offset: usize, limit: usize) ?Shape {
+    const header = saneHeader(data, offset, limit) orelse return null;
+    // The rows must fit in the block. A block whose opcodes run past it is not a shape.
+    const rows = data[offset + @sizeOf(ShapeHeader) .. limit];
+    var pos: usize = 0;
+    var scratch: [coordinate_limit]u8 = undefined;
+    for (0..@intCast(header.height())) |_| {
+        pos = decodeRow(rows, pos, scratch[0..@intCast(header.width())], null) catch return null;
+    }
+    return .{ .header = header.*, .rows = rows };
+}
+
+/// The shape header at `offset` in the block that ends at `limit`, where its bounds are sane:
+/// within `coordinate_limit`, around at least one pixel. Null otherwise, as for the game's own
+/// placeholders.
+fn saneHeader(data: []const u8, offset: usize, limit: usize) ?*align(1) const ShapeHeader {
     if (offset > limit or limit > data.len) return null;
     const header = layout.view(ShapeHeader, data[offset..limit]) catch return null;
-
     for ([_]i32{ header.x1, header.y1, header.x2, header.y2 }) |value| {
         if (value < -coordinate_limit or value > coordinate_limit) return null;
     }
     const w = header.width();
     const h = header.height();
     if (w <= 0 or h <= 0 or w > coordinate_limit or h > coordinate_limit) return null;
-
-    // The rows must fit in the block. A block whose opcodes run past it is not a shape.
-    const rows = data[offset + @sizeOf(ShapeHeader) .. limit];
-    var pos: usize = 0;
-    var scratch: [coordinate_limit]u8 = undefined;
-    for (0..@intCast(h)) |_| {
-        pos = decodeRow(rows, pos, scratch[0..@intCast(w)], null) catch return null;
-    }
-    return .{ .header = header.*, .rows = rows };
+    return header;
 }
 
 /// Expands one row.
@@ -449,6 +463,20 @@ test "parses a sprite with a palette and a shape" {
     try std.testing.expectEqualSlices(u8, &.{ 1, 1, 1, 0, 0, 1 }, pixels);
 }
 
+test "Sprite.cutShort" {
+    const gpa = std.testing.allocator;
+    const file = try testing.paletteAndShapes(gpa, 4);
+    defer gpa.free(file);
+    // The last shape's second row runs past the end of the set, as in a broken set from a mod.
+    const sprite: Sprite = try .parse(file[0 .. file.len - 2]);
+    try std.testing.expect(sprite.block(3) == .placeholder and sprite.cutShort(3));
+    try std.testing.expect(!sprite.cutShort(0) and !sprite.cutShort(1));
+    // A placeholder with nonsense bounds, as the game's own hold, isn't one.
+    const header = std.mem.bytesAsValue(ShapeHeader, file[sprite.entries[2].offset..][0..@sizeOf(ShapeHeader)]);
+    header.x2 = std.math.maxInt(i32);
+    try std.testing.expect(sprite.block(2) == .placeholder and !sprite.cutShort(2));
+}
+
 test "rejects files that are not sprites" {
     try std.testing.expectError(error.NotASprite, Sprite.parse(&.{}));
     try std.testing.expectError(error.NotASprite, Sprite.parse("2.00" ++ @as([4]u8, @splat(0))));

src/scripting/drawing.zig

@@ -206,7 +206,9 @@ pub const Assets = struct {
         errdefer gpa.free(copied);
         const art = try gpa.create(hud.Art);
         errdefer gpa.destroy(art);
-        art.* = try .init(gpa, try openreliant.spr.Sprite.parse(file), null, .of(archive.mods, copied));
+        const set = try openreliant.spr.Sprite.parse(file);
+        hud.warnCutShort(set, member);
+        art.* = try .init(gpa, set, null, .of(archive.mods, copied));
         errdefer art.deinit(gpa);
         try assets.sprite_sets.append(gpa, .{ .name = copied, .file = file, .art = art });
         assets.bytes += file.len;