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

[OTR Archive] Move shared scenes out of nonmq/mq folders #3191

Merged

Conversation

Archez
Copy link
Contributor

@Archez Archez commented Sep 14, 2023

This is a PoC and request for comment on the possibility of moving shared scenes between mq and nonmq to a new common folder in the OTR called shared.

After the work from #3161, all roms should have the same exact "name" for the same textures (aligned with MQ debug names), so moving the common scenes to a shared location will cut down the amount of duplicate textures that a texture pack maker has to implement.

Although texture names are synced, other resource types do not share names yet (DList, VTX, etc). Currently this is not an issue as OTR patching would just place the mq ones side by side the nonmq ones, and nonmq will have the higher priority so scenes will load fine. Synchronizing these names will only really matter once scene editing support via mod OTRs becomes available, so I don't consider that a blocker for this proposal.

The way this is handled is the xml definitions already has a breakdown of dungeons, indoor, overworld and misc scenes. So rather than applying the nonmq/mq divide to "all" themes, I have moved the split to apply only to the scenes defined in the dungeons folder, and the rest now are placed under the shared folder.

Something to note is that the dungeons folder has more than just the actual MQ dungeons, it also includes Thieves' Hideout, Boss rooms, Outside Ganon's Castle, and Ganons Tower/Castle Collapse. We could technically move these xml definitions out to another folder to further reduce the common scenes. Alternatively we could build a map of the scenes we want split, but this map would need to exist both in ZAPD and OTRExporter.
EDIT: I've decided to use regex to search for the explicit dungeons (as seen in OTRExporter/ZAPDTR PRs)

image

Depends on HarbourMasters/ZAPDTR#16
Depends on HarbourMasters/OTRExporter#11

Build Artifacts

@briaguya-ai
Copy link
Contributor

i vote yes we do it it sounds like a really good idea

@PurpleHato
Copy link
Contributor

I also vote yes, the alignment PR now merged and this PR would indeed make things way easier for textures packs makers

@Archez Archez changed the title [RFC] [OTR Archive] Move shared scenes out of nonmq/mq folders [OTR Archive] Move shared scenes out of nonmq/mq folders Nov 5, 2023
@Archez Archez marked this pull request as ready for review November 5, 2023 14:28
@Archez
Copy link
Contributor Author

Archez commented Nov 5, 2023

I have elected to use a regex pattern in OTRExporter and ZAPDTR to filter for only explicit scenes that will have MQ variants (this makes it so theives hideout and ganons collapse considered shared scenes).

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

one non-blocking comment

:shipit:

Comment on lines +33 to +34
int16_t inNonSharedScene = (sceneNum >= SCENE_DEKU_TREE && sceneNum <= SCENE_ICE_CAVERN) ||
sceneNum == SCENE_GERUDO_TRAINING_GROUND || sceneNum == SCENE_INSIDE_GANONS_CASTLE;
Copy link
Contributor

Choose a reason for hiding this comment

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

i feel like we do something similar in quite a few places throughout the codebase. i know changing the scene order would break things, but i think it'd be nice to come up with a reusable reliable source of truth for stuff like "is this scene a dungeon" and "is this scene the same in mq and vanilla"

that feels like code cleanup and out of scope for this PR though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, some of them are dungeons + bosses, or all of ganons scenes instead of just the trials part. So those aren't useful here. But agree overall that we can try to clean up and move those to a similar location.

@garrettjoecox garrettjoecox merged commit 460b3d0 into HarbourMasters:develop Nov 6, 2023
8 checks passed
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.

None yet

4 participants