Skip to content

fix: explicitly define more nullable types - #39

Merged
Krzyhau merged 3 commits into
FEZModding:mainfrom
zrckr:fix-more-nullable-types
Aug 3, 2026
Merged

fix: explicitly define more nullable types#39
Krzyhau merged 3 commits into
FEZModding:mainfrom
zrckr:fix-more-nullable-types

Conversation

@zrckr

@zrckr zrckr commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Some fields were already marked as nullable in b019045 and 97c5531. This PR marks additional fields as nullable and adds default values for the fields, in accordance with the class definitions in the original game. This also adds the corresponding tests.

I’m not entirely sure this will completely eliminate ALL footguns for FEZEditor or HAT mod loader, but at least it makes the contract more explicit, given that the game predates nullable-reference annotations.

If there’s a better way to resolve the root discrepancy, feel free to reject this PR.

@zrckr
zrckr requested a review from Krzyhau July 27, 2026 06:02
@Krzyhau

Krzyhau commented Jul 27, 2026

Copy link
Copy Markdown
Member

If you're at least 90% sure your manual checks for creating those unit tests are correct, I'm perfectly fine with merging it. However, you've shown interest in producing unit tests directly from stripped game libraries. So I'll let you choose what's more appealing to you - wouldn't want to take away fun from you :)

@zrckr

zrckr commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

I used the updated FezEngine.dll containing readers' logic from the FezEngine.Readers namespace. I created a new test that reads this IL, which we use to verify the Repacker's types. This was done because, ultimately, these are the readers that will be used to read the assets, whether they're in XNB or JSON format.

@Krzyhau

Krzyhau commented Aug 3, 2026

Copy link
Copy Markdown
Member

lgtm

@Krzyhau
Krzyhau merged commit 9ca59ce into FEZModding:main Aug 3, 2026
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