Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only align if data section size contains files #112

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

ThanatosGit
Copy link
Contributor

Addresses #108

Is it different for dread? In SR there exists some empty BLA_discardables.pkg. If they have no files in the data section, they also have no 128 byte alignment after the header.

@duncathan
Copy link
Contributor

if you're unsure, add a test that rebuilds and compares every pak to make sure everything passes

Copy link

codecov bot commented Dec 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2fb3157) 73.94% compared to head (bb0a046) 73.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #112      +/-   ##
==========================================
+ Coverage   73.94%   73.95%   +0.01%     
==========================================
  Files          61       61              
  Lines        3116     3118       +2     
==========================================
+ Hits         2304     2306       +2     
  Misses        812      812              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ThanatosGit
Copy link
Contributor Author

ThanatosGit commented Dec 29, 2023

I don't even know if dread has empty packages but I added changed a test for dread.

parse_and_build_compare(
Pkg.construct_class(Game.DREAD), Game.DREAD, dread_path.joinpath("packs/system/system.pkg")
)
pkg_files = [f for f in dread_path.rglob("*.pkg")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preferable for having the list of files in the code, and then parametrizing the test.

Copy link
Member

@henriquegemignani henriquegemignani Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add test for SR files. If any file fails the test for SR, mark it as expected failure

Copy link
Contributor Author

@ThanatosGit ThanatosGit Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dread has 47 files and SR has 368 files.
For SR it is quite easy: As we never figured out how to rebuilt the pkgs, every test will fail except the empty pkgs. I can create a PR for my funny code which can parse + rebuild all files except 11 from SR if you like but it doesn't make sense in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno if you really want me to explicitly write down all these files but I did it now how it is done for most other tests with the following + parametrizing the test:

all_dread_pkg = [name for name in dread_data.all_name_to_asset_id().keys()
                   if name.endswith(".pkg")]

Added a test for SR but skipped for now because the already mentioned reason.

@ThanatosGit ThanatosGit added this pull request to the merge queue Jan 8, 2024
Merged via the queue into randovania:main with commit 94a3182 Jan 8, 2024
8 checks passed
@ThanatosGit ThanatosGit deleted the empty-pkg-parse branch January 8, 2024 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants