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 Scene Specific Checks to Dirt Path Fixes #2903

Conversation

Patrick12115
Copy link
Contributor

@Patrick12115 Patrick12115 commented May 20, 2023

This makes it to where it only activates in the specific scenes with paths. (Hyrule Field, Hyrule Castle, and Kokiri Forest)

It will then be disabled in any other scenes, so that other decals, like scrub shadows, are not affected.

Build Artifacts

This makes it to where it only activates in the specific scenes with paths. (Hyrule Field, Hyrule Castle, and Kokiri Forest)

It will then be disabled in any other scenes, so that other decals, like scrub shadows, are not affected.
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.

overall this feels like a reasonable solution, couple little suggestions to clean stuff up

also, this feels like it could probably be pointed to develop-spock instead of develop

Comment on lines 867 to 876
switch (gPlayState->sceneNum) {
case SCENE_SPOT00:
case SCENE_SPOT04:
case SCENE_SPOT15:
CVarSetInteger("gDirtPathFix", CVarGetInteger("gSceneSpecificDirtPathFix", 0));
break;
default:
CVarClear("gDirtPathFix");
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if this was moved out into a method then that could be registered on the scene transition hook and called here too (i'm assuming this code is duplicated so you don't need to transition scenes when changing the dropdown value)

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'm not sure what you're suggesting here.

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm suggesting making a function that has this logic in it, so something like

void DoDirtPathStuff(int32_t sceneNum) {
    switch (sceneNum) {
        case SCENE_SPOT00:
        case SCENE_SPOT04:
        case SCENE_SPOT15:
            CVarSetInteger("gDirtPathFix", CVarGetInteger("gSceneSpecificDirtPathFix", 0));
            return;
        default:
            CVarClear("gDirtPathFix");
    }
}

then you could have

if (UIWidgets::EnhancementCombobox("gSceneSpecificDirtPathFix", zFightingOptions, 0) && gPlayState != NULL) {
    DoDirtPathStuff(gPlayState->sceneNum);
}

and

void RegisterPathFix() {
    GameInteractor::Instance->RegisterGameHook<GameInteractor::OnTransitionEnd>([](int32_t sceneNum) {
        DoDirtPathStuff(sceneNum);
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DoDirtPathStuff becomes undefined in gamemenubar

Nevermind, I have to set the define hook somewhere

I'm confused again

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to either define the function in a header that's included in both spots, or just use externs and declare it

Copy link
Contributor

Choose a reason for hiding this comment

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

Does the extra cvar/code need to exist in the first place? Is it not possible to place the scene specific checks within wherever the dirt path fix is applied?

yea like pat said the original fix is low level gfx code in LUS, which already feels icky but doesn't have access to higher level zelda stuff https://github.com/Kenix3/libultraship/blob/097ffc7d0e2b1a24a796de6cb299869c0f3030a2/src/graphic/Fast3D/gfx_gx2.cpp#L380

Copy link
Contributor

Choose a reason for hiding this comment

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

it kept yelling at me for trying to use int32_t as it wasn't defined.

which file did it get mad at you in?

edit: i assume it's in mods.h because there are no types being included there, probably just include stdint.h in mods.h

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 did it by adding it to GameInteractor.h. Not sure if that's better or not, but it was included in both already.

Copy link
Contributor

Choose a reason for hiding this comment

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

it shouldn't go in GameInteractor. it's defined in mods.cpp so it should be declared in mods.h. Also GameInteractor is a class so it's strange having extra non-class method declarations just kind of chillin in that file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes pushed at #2907

case SCENE_SPOT00:
case SCENE_SPOT04:
case SCENE_SPOT15:
CVarSetInteger("gDirtPathFix", CVarGetInteger("gSceneSpecificDirtPathFix", 0));
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not the biggest fan of gSceneSpecificDirtPathFix as a name here, i think it'd be more clear as gMenuOptionDirtPathFix

that way it's communicating that in these specific scenes, we apply the dirt path fix based on the option for the menu, but in all other scenes we ignore it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me. I wasn't sure about the name so I just went with that.

@Patrick12115
Copy link
Contributor Author

Closing to move to develop-spock #2907

@Patrick12115 Patrick12115 deleted the Path-Fix-in-Specific-Areas-Only branch May 24, 2023 05:32
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