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

Enhancement: Show Gauntlets in First Person #2918

Merged
merged 2 commits into from
Jun 2, 2023

Conversation

MoriyaFaith
Copy link
Contributor

@MoriyaFaith MoriyaFaith commented May 24, 2023

A simple enhancement that allows the player to see Link's Gauntlets while using the Bow and Hookshot, mimicking behavior in Ocarina of Time 3D.

image
image

Build Artifacts

Copy link
Contributor

@leggettc18 leggettc18 left a comment

Choose a reason for hiding this comment

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

Gotta say, don't love how it looks, but that's just the way the model is, nothing to do with your code. Plus a custom model pack might be able to make it look better so I'd say this is good to merge.

I left one comment about a thing I'd like to see/discuss a bit but it's not a blocker to actually merging.

@@ -803,7 +803,9 @@ void func_8008F470(PlayState* play, void** skeleton, Vec3s* jointTable, s32 dLis

SkelAnime_DrawFlexLod(play, skeleton, jointTable, dListCount, overrideLimbDraw, postLimbDraw, data, lod);

if ((overrideLimbDraw != func_800902F0) && (overrideLimbDraw != func_80090440) && (gSaveContext.gameMode != 3)) {
if (((CVarGetInteger("gFPSGauntlets", 0) && LINK_IS_ADULT) || (overrideLimbDraw != func_800902F0)) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this LINK_IS_ADULT check be removed to allow this to work with the Timeless Equipment cheat and equip swap? Or would that break something else or result in a really nasty if statement? Or do we think it's more consistent to not have the gauntlets when using Hookshot as child? I'm totally fine with either option, just wanted to make sure this question was considered.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it can check for the held item instead and check for the adult first person items?

Copy link
Contributor

Choose a reason for hiding this comment

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

Further discussion with some other devs has revealed that there's actually another Adult Link check further down in the authentic code. So even removing the above LINK_IS_ADULT check would still cause the gauntlets to not render when equip swapping as child, which is fine and imo should be left alone for now. That being said, the LINK_IS_ADULT check here should be unnecessary. Can it be removed @MoriyaFaith or is there a specific reason you have it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's removed, It renders the goron bracelet on the otherwise invisible child link right arm.

@leggettc18 leggettc18 added the merge conflicts PR has conflicts that need to be resolved before it can be merged label May 30, 2023
@leggettc18
Copy link
Contributor

Looks like a merge conflict popped up here. Probably nothing difficult, @MoriyaFaith are you able to see about resolving it?

@MoriyaFaith
Copy link
Contributor Author

should be fixed now. As for the way they look, I know it doesn't look great with default gauntlets. If anything, this was made for custom gauntlets, like my own work and the OoT3D port.

@leggettc18 leggettc18 added merge ready and removed merge conflicts PR has conflicts that need to be resolved before it can be merged labels Jun 2, 2023
@leggettc18 leggettc18 merged commit 26d9345 into HarbourMasters:develop Jun 2, 2023
@garrettjoecox garrettjoecox added this to the Sulu (7.1.x) milestone Jun 13, 2023
Patrick12115 pushed a commit to Patrick12115/Shipwright that referenced this pull request Sep 2, 2023
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