Conversation
Port of DolphinIsoLib into SabreTools.Serialization architecture: - Data.Models: GCZ/, WIA/, NintendoDisc/ model subdirectories (15 files) - Serialization.Readers: GCZ, WIA, NintendoDisc readers - Serialization.Writers: GCZ, WIA writers (structural metadata; full round-trip TODO) - Wrappers: NintendoDisc, GCZ, WIA wrappers with Encryption partial class - Wrappers: WiaRvzCompressionHelper (BZip2/LZMA/LZMA2/Zstd, net462+ guarded) - WrapperType + WrapperFactory: GCZ, WIA, NintendoDisc entries added GetInnerWrapper() decompression and NintendoDisc.Extraction FST extraction are stubbed with TODO comments pending full implementation.
|
There are a few things missing that I forgot to include. Working on these now, so not as ready for review yet. |
There was a problem hiding this comment.
Pull request overview
Adds preliminary parsing/wrapper support for Dolphin disc image formats (GCZ, WIA/RVZ) and a new NintendoDisc inner wrapper to enable extraction from these images.
Changes:
- Introduce new wrapper types and factory creation paths for GCZ, WIA/RVZ, and Nintendo disc images.
- Add new serialization models + readers (and metadata-only writers) for GCZ, WIA/RVZ, and NintendoDisc.
- Implement extraction for NintendoDisc plus “delegate extraction” for GCZ/WIA via full decompression to an inner wrapper.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| SabreTools.Wrappers/WrapperType.cs | Adds new wrapper enum values (GCZ, NintendoDisc, WIA). |
| SabreTools.Wrappers/WrapperFactory.cs | Registers new wrapper types in CreateWrapper. |
| SabreTools.Wrappers/WiaRvzCompressionHelper.cs | Adds helper routines for WIA/RVZ group compression/decompression. |
| SabreTools.Wrappers/WIA.cs | Adds WIA/RVZ wrapper with full-image reconstruction + inner NintendoDisc creation. |
| SabreTools.Wrappers/WIA.Extraction.cs | Implements extraction by delegating to the inner NintendoDisc wrapper. |
| SabreTools.Wrappers/NintendoDisc.cs | Adds NintendoDisc wrapper (GC/Wii disc image). |
| SabreTools.Wrappers/NintendoDisc.Extraction.cs | Adds GameCube + Wii partition extraction logic. |
| SabreTools.Wrappers/NintendoDisc.Encryption.cs | Adds Wii partition AES helpers (ticket title-key + block decryption). |
| SabreTools.Wrappers/GCZ.cs | Adds GCZ wrapper with full decompression + inner NintendoDisc creation. |
| SabreTools.Wrappers/GCZ.Extraction.cs | Implements extraction by delegating to the inner NintendoDisc wrapper. |
| SabreTools.Serialization.Writers/WIA.cs | Adds metadata-only WIA/RVZ writer (headers + tables). |
| SabreTools.Serialization.Writers/GCZ.cs | Adds metadata-only GCZ writer (header + tables). |
| SabreTools.Serialization.Readers/WIA.cs | Adds WIA/RVZ reader (headers + tables). |
| SabreTools.Serialization.Readers/NintendoDisc.cs | Adds NintendoDisc reader (boot header + Wii structures). |
| SabreTools.Serialization.Readers/GCZ.cs | Adds GCZ reader (header + pointer/hash tables). |
| SabreTools.Data.Models/WIA/WiaHeader2.cs | Adds WIA/RVZ header2 model. |
| SabreTools.Data.Models/WIA/WiaHeader1.cs | Adds WIA/RVZ header1 model. |
| SabreTools.Data.Models/WIA/RawDataEntry.cs | Adds WIA raw-region entry model. |
| SabreTools.Data.Models/WIA/PartitionEntry.cs | Adds WIA partition entry models. |
| SabreTools.Data.Models/WIA/HashExceptionEntry.cs | Adds WIA hash exception entry model. |
| SabreTools.Data.Models/WIA/GroupEntries.cs | Adds WIA + RVZ group entry models. |
| SabreTools.Data.Models/WIA/Enums.cs | Adds WIA/RVZ enums (disc type, compression type). |
| SabreTools.Data.Models/WIA/Constants.cs | Adds WIA/RVZ format constants. |
| SabreTools.Data.Models/WIA/Archive.cs | Adds WIA/RVZ archive model container. |
| SabreTools.Data.Models/NintendoDisc/WiiRegionData.cs | Adds Wii region data model. |
| SabreTools.Data.Models/NintendoDisc/WiiPartitionTableEntry.cs | Adds Wii partition table entry model. |
| SabreTools.Data.Models/NintendoDisc/Enums.cs | Adds NintendoDisc enums (platform, partition type). |
| SabreTools.Data.Models/NintendoDisc/DiscHeader.cs | Adds disc boot header model. |
| SabreTools.Data.Models/NintendoDisc/Disc.cs | Adds NintendoDisc model container. |
| SabreTools.Data.Models/NintendoDisc/Constants.cs | Adds NintendoDisc layout constants. |
| SabreTools.Data.Models/GCZ/GczHeader.cs | Adds GCZ header model. |
| SabreTools.Data.Models/GCZ/Constants.cs | Adds GCZ format constants. |
| SabreTools.Data.Models/GCZ/Archive.cs | Adds GCZ archive model container. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…isc detection for .iso files
…r to WIA/GCZ printing - Fix GCMagicWord: 0xC23D3C1F -> 0xC2339F3D (confirmed from Dolphin DiscUtils.h) - Fix disc header field layout to match Dolphin's confirmed offsets: MakerCode is bytes 4-5 of the 6-char GameId (no separate field at 0x006), DiscNumber at 0x006, DiscVersion at 0x007, unused region is 14 bytes (0x00A-0x017) - Update NintendoDisc reader: derive MakerCode from GameId[4..5], fix skip count - Add ParseDiscHeaderOnly() to reader for partial (short) stream parsing - Guard DisableHash/DisableEnc reads at the 0x080 boundary for 128-byte embedded headers - Guard DOL/FST skip for streams shorter than full 0x440 boot block - Fix WrapperFactory: NintendoDisc magic detection now precedes .iso -> ISO9660 fallback - Add GameId-prefix heuristic in WrapperFactory for GC discs lacking magic word - Add GameId-prefix platform fallback in reader for GC discs without GCMagicWord - Add DiscHeader property to WIA wrapper (parsed from Header2.DiscHeader bytes) - Add DiscHeader property to GCZ wrapper (decompresses first block only) - Add ReadDiscHeader() helper to GCZ for lightweight first-block decompression - Print embedded disc header (Game ID, Maker, Disc/Rev, Title) in WIA.Printing.cs and GCZ.Printing.cs
…aming
- Block decryption: IV is at raw block offset 0x3D0 (still-encrypted),
matching Dolphin/DolphinIsoLib WiiPartitionDecryptor.DecryptBlock exactly.
- FST size field at boot.bin 0x428 is also stored >>2 on Wii; apply <<2
to get true byte size.
- Partition folder naming now matches DolphinIsoLib WiiDiscExtractor exactly:
type 0->GM+n, 1->UP+n, 2->CH+n, printable ASCII unknown->raw 4-char string,
non-printable->P{index}. SSBB VC channels extract as HA8E, HA9E, etc.
- ExtractionTool peek buffer increased from 16 to 32 bytes.
Verified: SSBB GM0 extracts 5524 files, boot.bin/fst.bin byte-identical
to Dolphin reference extraction.
Files with fileSize=0 in the FST were silently skipped. Now they are created as empty files, matching Dolphin/DolphinIsoLib behavior. Verified: SSBB now extracts 5958 files with 0 missing, 0 extra, 0 size mismatches, and 0 hash mismatches vs DolphinIsoLib reference.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 48 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 48 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Some overall comments:
- There are a lot of places that you are having to deal with endian-specific reads and writes. In my
SabreTools.Numerics.Extensionsnamespace, you will find all of these sort of helpers ready to be used for arrays and streams alike. Even if I don't have a comment on a specific place, please replace all of these reads and writes with those methods instead. I'd even further suggest replacing all such reads and writes with those helpers as they have guards in them for additional safety.ReadByteis the odd one out that gets replaced byReadByteValue. - Many of the comments in this and the previous review apply to more than just the files they're in. I didn't want to clog the review with the same comment in every single place that it applies. Please take a look at the comments I have here and see what other places those might apply.
- I will strongly prefer that all methods have a summary, even the helpers. This helps maintainability into the future and allows them to be merged, moved, or even removed when possible.
|
|
||
| #region Little-endian write helpers | ||
|
|
||
| private static void WriteUInt32LE(Stream s, uint value) |
There was a problem hiding this comment.
Similar to the reading helpers mentioned in a previous comment, there are also endian-specific writing helpers in SabreTools.Numerics.Extensions. Please use those instead.
| /// Mirrors Dolphin's <c>FileSystemGCWii</c> offset-to-file-info cache | ||
| /// (<c>m_offset_file_info_cache</c>). | ||
| /// </summary> | ||
| internal sealed class GcFst |
There was a problem hiding this comment.
There's nothing really actionable about this comment, but I think that this may be able to live elsewhere, but I do not have a good idea where quite yet.
| #if NET20 || NET35 | ||
| using (var sha1 = SHA1.Create()) | ||
| #else | ||
| using (var sha1 = SHA1.Create()) | ||
| #endif |
There was a problem hiding this comment.
I will strongly prefer that you use my Hashing library for these sort of things. It wraps a lot of the specifics for hashing data that you shouldn't have to manage manually
There was a problem hiding this comment.
Also, side note, both of these lines are identical so I'm not sure why it was framework-gated.
| } | ||
|
|
||
| // --------------------------------------------------------------- | ||
| // Public API |
| /// <summary> | ||
| /// Compress a <see cref="NintendoDisc"/> wrapper to a WIA or RVZ file. | ||
| /// </summary> | ||
| public static bool ConvertFromDisc(NintendoDisc source, string outputPath, |
There was a problem hiding this comment.
This doesn't really have much of a precedent, so I'll have to figure out whether this living in the extraction or writing partial class makes more sense. It can stay here and I'll likely deal with that after.
| // GameCube/Wii disc by GameId prefix: first byte is a known title type code, | ||
| // bytes 1-2 are ASCII letters (region + developer), bytes 3-4 are ASCII digits or letters (title code), | ||
| // byte 5 is an ASCII digit (disc number). Covers redump ISOs that lack magic words. | ||
| if (magic.Length > 5 |
There was a problem hiding this comment.
I know there's likely no better way of handling this, but this is a very heavy if statement...
| #region WIA | ||
|
|
||
| // WIA magic ("WIA\x01" stored little-endian: 0x01414957) | ||
| if (magic.StartsWith([0x57, 0x49, 0x41, 0x01])) |
There was a problem hiding this comment.
Applies to other places too, but if you have constants defined, you can use them here instead of duplicating the array. See the XboxExecutable check below for an example
Preliminary support for reading Dolphin Disc Images (GCZ, WIA, RVZ)