systems/gc.ixx (1)

53-69: Consider extracting shared Nintendo BCA formatting into a common helper.

The GC/WII implementations are nearly identical; centralizing this avoids drift in offsets/size handling and output behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@systems/gc.ixx` around lines 53 - 69, The GC-specific BCA handling (bca_path
creation, existence check, read_vector, size compare against
_GC_BCA_STRUCTURE_SIZE, and hexdump output using
_GC_REPORTED_BCA_OFFSET/_GC_REPORTED_BCA_SIZE) should be extracted into a shared
helper (e.g., format_nintendo_bca or print_nintendo_bca) used by both GC and WII
modules; implement a single function that accepts the track path and an output
stream, encapsulates the path construction (track_extract_basename + ".bca"),
existence check, read_vector call, size validation, and hexdump formatting using
the shared constants (or move constants to a shared header), then replace the
duplicated block in the GC implementation with a call to that helper.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

systems/gc.ixx (1)

57-59: ⚠️ Potential issue | 🟡 Minor

Log malformed .bca before returning.

At Line 59, size mismatch exits silently, so malformed sidecars are indistinguishable from “no BCA output.” Please emit a clear INFO line (path + actual size) before returning.

Suggested patch

         auto bca = read_vector(bca_path);
-        if(bca.size() != _GC_BCA_STRUCTURE_SIZE)
-            return;
+        if(bca.size() != _GC_BCA_STRUCTURE_SIZE)
+        {
+            os << std::format("  BCA: unavailable (unexpected size: {} bytes in {}, expected: {})",
+                              bca.size(), bca_path.string(), _GC_BCA_STRUCTURE_SIZE)
+               << std::endl;
+            return;
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@systems/gc.ixx` around lines 57 - 59, The size-check on the BCA vector
silently returns when bca.size() != _GC_BCA_STRUCTURE_SIZE; before returning,
emit a clear INFO-level log containing the bca_path and the actual bca.size() so
malformed .bca files are distinguishable from missing output. Locate the check
around the read_vector(bca_path) call and replace the silent return with: log an
INFO message (using the project’s logging facility) that includes the bca_path
and bca.size(), then return; keep the same early-return control flow and
reference _GC_BCA_STRUCTURE_SIZE in the message for clarity.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

dvd/dvd_dump.ixx (1)

687-688: For multi-layer DVDs, has_bca reflects only the last layer's BCA flag.

The has_bca assignment inside the loop means it gets overwritten for each layer, and only the last layer's value is retained. While BCA presence is typically consistent across layers, consider assigning from the first layer (layer 0) which is authoritative, or using a logical OR to capture if any layer reports BCA.

♻️ Proposed fix to use first layer's BCA flag

                     // nintendo discs have first byte 0xFF
                     if(structure[sizeof(CMD_ParameterListHeader)] == 0xFF)
                         nintendo_key = 0;
 
-                    has_bca = layer_descriptor.bca;
+                    if(i == 0)
+                        has_bca = layer_descriptor.bca;
                 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dvd/dvd_dump.ixx` around lines 687 - 688, The current loop overwrites has_bca
with layer_descriptor.bca for each layer so only the last layer's value is kept;
change the assignment so has_bca reflects the authoritative BCA state by either
setting has_bca once from the first layer (e.g., when layer index == 0) or by
computing has_bca |= layer_descriptor.bca inside the loop to capture any layer
reporting BCA; update the code around the loop where has_bca and
layer_descriptor are used to follow one of these two approaches.

systems/wii.ixx (1)

68-70: Consider sharing BCA constants between GC and WII.

The BCA constants (_WII_BCA_STRUCTURE_SIZE, _WII_REPORTED_BCA_OFFSET, _WII_REPORTED_BCA_SIZE) are identical to those in gc.ixx. Since both GameCube and Wii discs share the same BCA structure format, consider extracting these to a shared location (e.g., a common Nintendo disc header or constants module) to reduce duplication and ensure consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@systems/wii.ixx` around lines 68 - 70, The three BCA constants
(_WII_BCA_STRUCTURE_SIZE, _WII_REPORTED_BCA_OFFSET, _WII_REPORTED_BCA_SIZE) are
duplicated in systems/wii.ixx and gc.ixx; extract them into a shared constants
header/module (e.g., nintendo_disc_constants or disc_bca) and replace the local
definitions in systems/wii.ixx and gc.ixx with references to the shared symbols
(import/include the new module and use its symbol names), update any namespaces
as needed so callers (e.g., code that used _WII_BCA_STRUCTURE_SIZE) now
reference the single shared constant to remove duplication and ensure
consistency.

utils/hex_bin.ixx (1)

109-131: Consider adding a trailing newline for consistent output formatting.

The function does not append a trailing newline after the last line of output. When used in gc.ixx and wii.ixx (e.g., os << std::format("{}", rawhexdump(...))), this means the BCA hex dump won't end with a newline, which may cause the next output to appear on the same line or result in inconsistent formatting.

♻️ Proposed fix to add trailing newline

         ss << std::setw(2) << (uint32_t)data[i];
     }
 
+    if(size > 0)
+        ss << std::endl;
+
     return ss.str();
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@utils/hex_bin.ixx` around lines 109 - 131, The rawhexdump function currently
builds hex lines into std::stringstream ss but does not append a trailing
newline, causing subsequent output to appear on the same line; update rawhexdump
(the function signature rawhexdump(const uint8_t *data, uint32_t size, uint32_t
group_size = 2, uint32_t groups_per_line = 8)) to ensure a trailing newline is
added to ss before return (e.g., append '\n' or use ss << std::endl) so output
always ends with a newline when using ss.str().

Configuration used: defaults

Review profile: CHILL

Plan: Pro

dvd/dvd_dump.ixx (1)

820-835: Consider using consistent std::set lookup pattern.

The BCA handling logic is correct. However, line 824 uses readable_formats.contains() while the rest of the file consistently uses the find(...) != end() pattern (e.g., lines 646, 736, 853). Consider aligning with the existing style for consistency.

♻️ Optional: Align with existing lookup pattern

-            if(!readable_formats.contains(READ_DISC_STRUCTURE_Format::BCA))
+            if(readable_formats.find(READ_DISC_STRUCTURE_Format::BCA) == readable_formats.end())

Alternatively, modernizing all lookups to use contains() would also be valid.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dvd/dvd_dump.ixx` around lines 820 - 835, The BCA branch uses
readable_formats.contains(...) which is inconsistent with the rest of the file;
change the lookup to the existing pattern using
readable_formats.find(READ_DISC_STRUCTURE_Format::BCA) != readable_formats.end()
(refer to has_bca, readable_formats, and READ_DISC_STRUCTURE_Format::BCA used in
this block) so the set lookup style matches other checks in this file.

Configuration used: defaults

Review profile: CHILL

Plan: Pro