kbeckmann / game-and-watch-retro-go

Emulator collection for Nintendo® Game & Watch™
GNU General Public License v2.0
440 stars 133 forks source link

Zopfli (DEFLATE) NES Support #106

Closed BrianPugh closed 3 years ago

BrianPugh commented 3 years ago

Note: the other parse_roms.py related PRs should probably be merged in first, which will make this PR look much smaller.

Summary:

Current downside of this PR as is:

For completeness, here are some comparisons on NES games:

Name          Size (Uncompressed)      Size (lz4)    Size (zopfli)   Ratio (lz4)  Ratio (zopfli)
StarTropics   524,304                  291,322       251,186         55.6%        47.9%
SMB           40,976                   36,248        30,432          88.5%        74.3%
SMB3          393,232                  250,828       220,336         63.8%        56.0%
kbeckmann commented 3 years ago

Awesome! I've merged #103 now so you should be able to rebase this branch to get rid of the extra commits.

I've pulled in the commits to the main branch of https://github.com/kbeckmann/retro-go-stm32/commits/main

BrianPugh commented 3 years ago

PR has been rebased! I added back in ignoring linting rule E203 because it conflicts with black

BrianPugh commented 3 years ago

GB/GBC compression support is basically ready (it just requires this PR as well as some additional stuff in retro-go-stm32). You'll see a PR there soon, so we should merge that, update the submodule reference here, then merge (so at that point, both NES and GB/GBC work with zopfli)

BrianPugh commented 3 years ago

see https://github.com/kbeckmann/retro-go-stm32/pull/10

kbeckmann commented 3 years ago

Awesome, thanks! I have merged the pr in retro-go-stm32.

Could you update the help string too? Something like

help=f"Compression method. One of {', '.join(filter(lambda x: not x.startswith('.'), COMPRESSIONS.keys()))}. Defaults to no compression.",

or so. You can probably write it in a nicer way than me :smile: .

kbeckmann commented 3 years ago

Before finalizing this PR, could you remove the commit 14474fbce8296e3cf57de1ffe32424c38da204b2 and then make a commit that updates the submodule to https://github.com/kbeckmann/retro-go-stm32/commit/4c3264831137d653c67358c614416bc82089530f instead? That way I don't have to cherry-pick from this PR and can just rebase and merge it later.

BrianPugh commented 3 years ago

updated, should we mention something in the readme about setting COMPRESS=zopfli during building if people want to use it?

Also, needing to run python3 -m pip install -r requirements.txt if they choose to use this.

kbeckmann commented 3 years ago

Great! Yeah, a comment in the readme is probably a good idea.

BrianPugh commented 3 years ago

README updated, let me know if its too much.

kbeckmann commented 3 years ago

Thanks! No it’s great.