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 #2907

Merged

Conversation

Patrick12115
Copy link
Contributor

@Patrick12115 Patrick12115 commented May 21, 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.

Moved from develop PR #2903 to develop-spock

Build Artifacts

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.

not the biggest fan of UpdateDirtPathFixState and gSceneSpecificDirtPathFix as names but nothing good is coming to mind so i don't want to let that block this

:shipit:

@Patrick12115
Copy link
Contributor Author

Yeah, if anyone has any better names, I'm all for it. haha. Just went with what I thought would be easyish to understand what it was doing. And stole the other one from Proxy haha.

@briaguya-ai
Copy link
Contributor

my issue with gSceneSpecificDirtPathFix and gDirtPathFix is it feels like when gSceneSpecificDirtPathFix is true that should mean we want the fix for this scene, and when gDirtPathFix is true it means we want to fix dirt paths, but they're being used to mean the opposite

i know that's because changing gDirtPathFix would require LUS changes. i feel like if we could change both names it'd be easy to come up with good ones, but trying to come up with a good name when we're stuck with gDirtPathFix meaning "apply the dirt path fix in fast3d without checking what scene we're in" it makes it tough

as for UpdateDirtPathFixState - i feel like maybe ConditionallyApplyDirtPathFix could work?

@garrettjoecox
Copy link
Contributor

not the biggest fan of UpdateDirtPathFixState and gSceneSpecificDirtPathFix as names but nothing good is coming to mind so i don't want to let that block this

I think the CVAR used in LUS is probably the issue here but more annoying to change. CVAR in LUS should be gZFightingMode, and then within SoH this new cvar should be gDirtPathFix, and the update method could be something like UpdateZFightingModeForDirtPathFix, maybe a bit verbose but it says what it does.

@briaguya-ai
Copy link
Contributor

@garrettjoecox i like that plan, want to make an issue to update the LUS cvar so we can have decent names in sulu?

@Patrick12115
Copy link
Contributor Author

Want me to go ahead and change all that on my side, now? Or wait?

@garrettjoecox
Copy link
Contributor

@garrettjoecox i like that plan, want to make an issue to update the LUS cvar so we can have decent names in sulu?

Kenix3/libultraship#278

Want me to go ahead and change all that on my side, now? Or wait?

I think there's some stuff in flight with LUS so probably we stick with the weird names for now and change for sulu, I'll differ to Bria though

@briaguya-ai
Copy link
Contributor

yeah, let's get this one in as-is for spock and worry about names when we can change things everywhere

@leggettc18 leggettc18 merged commit 7962e0e into HarbourMasters:develop-spock May 21, 2023
7 checks passed
@Patrick12115 Patrick12115 deleted the UpdateDirtPathFixState branch August 25, 2023 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants