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

Add missing ExternalFile tags to object_dmask and object_osn #1770

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

Thar0
Copy link
Contributor

@Thar0 Thar0 commented Dec 17, 2024

Skeletons in these objects have references to display lists in gameplay_keep, so they should rely on gameplay_keep as an ExternalFile.

Unfortunately though, adding these to the xmls do not appear to actually replace the hardcoded gameplay_keep pointers with symbols.. a ZAPD bug?

@hensldm hensldm removed the Needs-first-approval First approval label Dec 18, 2024
Copy link
Collaborator

@AngheloAlf AngheloAlf left a comment

Choose a reason for hiding this comment

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

I don't think this PR actually does anything at all since we list gameplay_keep as a dependency for every other object by listing it on the global config (https://github.com/zeldaret/mm/blob/main/tools/ZAPDConfigs/MM/Config.xml#L7). This needs to be fixed on ZAPD's side

@Thar0
Copy link
Contributor Author

Thar0 commented Dec 18, 2024

Lots of other objects explicitly list gameplay_keep as an ExternalFile, so if nothing else it makes these objects consistent with the rest. But yes, a ZAPD fix is needed.

Copy link
Collaborator

@AngheloAlf AngheloAlf left a comment

Choose a reason for hiding this comment

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

In that case I think we should probably clean up the xmls. I guess we could remove gameplay_keep from the global config and just be explicit about it. Either way, that decision doesn't affect this PR.

@AngheloAlf AngheloAlf added Merge-ready All reviewers satisfied, just waiting for CI and removed Needs-second-approval Second approval labels Dec 18, 2024
@engineer124 engineer124 merged commit 5d49317 into zeldaret:main Dec 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Asset Merge-ready All reviewers satisfied, just waiting for CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants