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

Version checking #232

Merged
merged 10 commits into from
Oct 16, 2024
Merged

Conversation

steven11sjf
Copy link
Contributor

  • added a new GameVersion enum that tracks game, version, and md5 checksums
  • added functions to identify version, verify files and verify data for a FileTreeEditor
  • MSR and Dread now only load files that exist for your version
  • resource json's store dicts instead of values, with a crc field and an optional versions field of which versions the file is for
  • updated the binary format to include a 2byte bitfield for which versions each file is in

Copy link

codecov bot commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 78.57143% with 27 lines in your changes missing coverage. Please review.

Project coverage is 75.54%. Comparing base (d41e7e7) to head (65e8671).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...ry_engine_data_structures/_dread_data_construct.py 66.66% 8 Missing ⚠️
src/mercury_engine_data_structures/dread_data.py 65.21% 8 Missing ⚠️
...rcury_engine_data_structures/samus_returns_data.py 61.90% 8 Missing ⚠️
...rcury_engine_data_structures/version_validation.py 87.50% 2 Missing ⚠️
src/mercury_engine_data_structures/game_check.py 96.87% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #232      +/-   ##
==========================================
+ Coverage   75.53%   75.54%   +0.01%     
==========================================
  Files          77       78       +1     
  Lines        3670     3766      +96     
==========================================
+ Hits         2772     2845      +73     
- Misses        898      921      +23     

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


def identify_version(editor: FileTreeEditor) -> GameVersion:
romfs = editor.romfs
print(romfs)
Copy link
Member

Choose a reason for hiding this comment

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

print


MSR = (0, "1.0.0", Game.SAMUS_RETURNS, 1, "E9F1963CCCD5002CF6DE6E844528DF46", "83E382FB3E95061185184CE7FCB45AF8")
DREAD_1_0_0 = (1, "1.0.0", Game.DREAD, 1, "8DEC0C18622C6DAC370F84CF3A3AC0B4", "862EB0111C28082F5730FACF583CEF7B")
DREAD_1_0_1 = (2, "1.0.1", Game.DREAD, 2, "35309081AF05C60CEBC476F78F3609B6", "00")
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather these two versions have all_files_hash be None instead.

DREAD_1_0_1 = (2, "1.0.1", Game.DREAD, 2, "35309081AF05C60CEBC476F78F3609B6", "00")
DREAD_2_0_0 = (3, "2.0.0", Game.DREAD, 4, "B36FB05261F2E4EAF0408760E1B983FD", "00")
DREAD_2_1_0 = (4, "2.1.0", Game.DREAD, 8, "782820635AC434A18DF11DE3D4052DD1", "F1A3ABE49305A16F4671E9E64EBCA119")
UNDEFINED = (-1, "UNDEFINED", None, 2**15, "", "")
Copy link
Member

Choose a reason for hiding this comment

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

Remove this entry. Anywhere that'd use it should use None

return member

@classmethod
def get_value(cls, game: Game, version: str):
Copy link
Member

Choose a reason for hiding this comment

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

Function is unused and seems weird overall. I'd say remove it

steven11sjf and others added 7 commits October 15, 2024 13:52
- added a new GameVersion enum that tracks game, version, and md5
  checksums
- added functions to identify version, verify files and verify data for
  a FileTreeEditor
- MSR and Dread now only load files that exist for your version
- resource json's store dicts instead of values, with a crc field and an
  optional versions field of which versions the file is for
- updated the binary format to include a 2byte bitfield for which
  versions each file is in
- added generator method to iterate over all asset names in folder
- replaced all_asset_names with a generator comprehension
"""
returns an iterator of all known asset names in a folder
"""
return (
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test for this?

raise ValueError("Not a valid version!")


def verify_file_structure(editor: FileTreeEditor) -> GameVersion:
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this function and verify_file_integrity? I'm not happy with how they work and I'd rather have not block this PR because of these.

return md5(data, usedforsecurity=False).digest()


def identify_version(editor: FileTreeEditor) -> GameVersion:
Copy link
Member

Choose a reason for hiding this comment

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

This function should receive the RomFS directly, to not receive a FileTreeEditor in mid-constructor (a thing you should avoid!)

@henriquegemignani henriquegemignani added this pull request to the merge queue Oct 16, 2024
Merged via the queue into randovania:main with commit b4e1076 Oct 16, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants