Check (attributes) file when using --update flag

[sjoblomj]

The --update flag for the add subcommand intends to only add the files if the local file is deemed "updated" compared to the file in the archive. A file is currently considered "updated" when the file sizes aren't equal.

However, as issue #174 points out, files can be updated yet still have the same file size. This is not uncommon - some of the files that mods for StarCraft I and WarCraft II change most often always have fixed sizes.

While not mandatory, some MPQ archives contains an (attributes) file, which can contain MD5 sums, CRC32 checksums and timestamps of the files inside the archives. I believe that starcraft1, warcraft2, diablo1, diablo2 and lordsofmagic does not contain (attributes) but that newer games do.

This PR will check the file sizes, and if they are the same it proceeds to check against the values of (attributes) if present. It compares the MD5 if present, if not it checks the CRC32 if present, if not it checks the timestamps if present. It compares the timestamps if present, if not or if not matching, it checks the MD5 if present, if not it checks the CRC32 if present. So if file sizes and timestamps, MD5 or CRC32 are matching, the file is not considered "updated"; if file sizes, MD5 or CRC32 are not matching, the file is considered "updated" (but timestamps are allowed to be different).

If (attributes) is not present in the archive (which again, it would not be for starcraft1, warcraft2, diablo1, diablo2 and lordsofmagic), passing in --update will basically do nothing with this PR.

---

The code for CRC32 and MD5 was written by AI and is by its nature quite messy to read. We need to ask ourselves if this is complexity we want in mpqcli. I myself am somewhat leaning towards answering No to that question - and I'd then say that the --update flag should be removed altogether. In my opinion, it is not unreasonable to delegate the decision of whether to add a file to the archive to the user itself.

[sjoblomj]

It seems to me that MD5 is the most reliable metric but also the most expensive. CRC32 seems a little less reliable but a bit faster than MD5. Timestamps seem reliable for the matching case, i.e. if the timestamps and the file sizes match, it ought to be a strong sign of equality, but as #174 shows, timestamps that don't match don't need to mean that the files are not equal.

I'm thinking that we should thus start by comparing file sizes + timestamps. If the timestamps don't match, we look at MD5 if present, then resort to CRC32. So I'm proposing something like this:

flowchart TD
    A[Start] --> FS{File size equals}
    FS -->|No| U[Update file]
    FS -->|Yes| TE{Timestamps exist and equals} -->|Yes| DU
    TE -->|No| MX
    DU[Don't update file]
    MX{MD5 exists} -->|Yes| FB
    MX -->|No| CX
    FB{MD5 equals} -->|Yes| DU
    FB -->|No| U
    CX{CRC32 exists} -->|Yes| CE
    CX -->|No| U
    CE{CRC32 equals} -->|Yes| DU
    CE -->|No| U

Loading

I'll push a new commit with that implementation.

[sjoblomj]

I made a benchmark (code). The baseline is when we don't pass in --update at all:

Scenario	Min	Avg	Max	vs baseline
No --update (baseline)	3.30s	3.43s	3.78s	—
--update, no attributes	3.36s	3.50s	3.73s	—
--update, Timestamp matches	0.01s	0.01s	0.01s	430× faster
--update, MD5 (TS mismatch)	1.01s	1.04s	1.29s	3.3× faster
--update, CRC32 (TS mismatch, no MD5)	0.70s	0.71s	0.76s	4.8× faster

500 files · 512 KB each · 100 runs

---

So this implementation offers a clear speedup compared to dropping the --update flag. If the timestamps match, that speeds things up 430 times. Comparing MD5 is more reliable than CRC32 I believe, but a bit more expensive. It still offers 3.3 times speedup compared to no --update though.