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

Improve helpers module #297

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

isHarryh
Copy link
Contributor

@isHarryh isHarryh commented Feb 6, 2025

Summary

This PR does some minor improvements on the helpers module. These improvements will not affect the existing features.

Common Changes

  1. Type hints.

Individual Changes

  1. ArchiveStorageDecryptor: Uses an inline approach to replace the function to_uint4_array, which can improve the performance and simplify the code.
  2. ResourceReader: Refactors the get_resource_data function, because this function used an implicit argument overloading trick (mixed the 4-args invocation and the 3-args invocation, but only 4-args invocation is used externally), which is bad for maintaining.
  3. UnityVersion: Fixes the error in the method fromString.

UnityPy/helpers/MeshHelper.py Outdated Show resolved Hide resolved
UnityPy/helpers/Tpk.py Outdated Show resolved Hide resolved

from .TypeTreeHelper import TypeTreeNode

TPKTYPETREE: TpkTypeTreeBlob = None
TPKTYPETREE: Optional[TpkTypeTreeBlob] = None
Copy link
Owner

Choose a reason for hiding this comment

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

TPKTYPETREE is always a TpkTypeTreeBlob due to the init call at the end of the file.
So the Optional would add more trouble, as the other functions here would then need unnecessary None checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.
I guess it would be better if we use something like TPKTYPETREE: TpkTypeTreeBlob = init(), which is also an execute-at-import behavior but result in more clear intention and better type checking experience which avoid separating the init() call to the end of the file.

Copy link
Owner

Choose a reason for hiding this comment

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

Sadly that doesn't work, as the parser classes aren't defined yet at that point.
It simply errors.

Exception has occurred: NameError
name 'TpkFile' is not defined

@K0lb3 K0lb3 force-pushed the master branch 2 times, most recently from 5ab6d25 to c695346 Compare February 6, 2025 21:28
@isHarryh
Copy link
Contributor Author

isHarryh commented Feb 7, 2025

According to your 4 reviews. I have changed these things and force-pushed these changes.

UnityPy/helpers/Tpk.py Outdated Show resolved Hide resolved
@isHarryh
Copy link
Contributor Author

Okay. 2 fixes were applyed since the last push:

  1. Revert the changes in Tpk.py which caused NameError.
  2. Use forward reference "SerializedFile" in ResourceReader.

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.

2 participants