-
Notifications
You must be signed in to change notification settings - Fork 521
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
Tweak: Refactor 3D drops handling #3071
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Great refactor and nice addition to the bombhus drop!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ultimately this minimizes the original code changes while restoring authentic code/behavior
Sign me up. Would be neat if I could somehow see a diff of just the new changes on top of the original source but I'll hunt that down later. Great work
@@ -1406,11 +1327,11 @@ void EnItem00_DrawRupee(EnItem00* this, PlayState* play) { | |||
rupeeColor = CVarGetColor24("gCosmetics.Consumable_RedRupee.Value", (Color_RGB8){ 255, 255, 255 }); | |||
shouldColor = CVarGetInteger("gCosmetics.Consumable_RedRupee.Changed", 0); | |||
break; | |||
case 3: | |||
case 4: // orange rupee texture corresponds to the purple rupee (authentic bug) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we like authentic bugs around here
This PR aims to refactor and improve the 3D drops handling to address bugs when toggling on/off and simplify the logic.
The way 3d drops worked before was applying a different actor scale, yOffset and sometimes shadowScale either during actor init or actor update/draw. This resulted in having to constantly apply the original or 3d drop specific values which lead to the drops flashing or ending up in a stuck state when toggled on/off.
Instead, this PR uses the pattern that the authentic game does for making recovery hearts 3D, by applying a
Matrix_Scale
. Doing this I was able to remove all the actor scale yOffset changes such that all the drops uses the authentic game values now, and only when 3d drops is only do we apply theMatrix_Scale
to make the GID appear as the right size. This resolves any lingering issues with toggling on the fly and ensures actor properties aren't left in a partially modified state.Ultimately this minimizes the original code changes while restoring authentic code/behavior. I have verified all drops have an appropriate scale with these changes.
Notable bug fixes in these changes:
ITEM00_BOMB_SPECIAL
was using the arrow GID instead of the bomb GIDNotable additions:
Notable change:
Previously when 3d drops are on, the "red" rupees in the Shooting gallery tracker section where being switched from purple (authentic) to red. This was due to the GID for gold rupees matching the color. The problem is that the game authentically incorrectly labels gold rupees as pink, and purple rupees as orange, and so this "adjustment" was used to make the rupee match the red ones in the mini-game. After swapping the GID to match this behavior (pink using purple GID), I have removed this change as it was only applied to 3d drops and was changing authentic behavior beyond what the toggle implies.
So now the progress tracker rupees are correctly their "pink" variant (purple GID when 3d drops are on).
Admittedly, this PR is doing a bit, so I am targeting develop as it is more of a refactor than just "bug fixes" alone. I can rebase if we prefer it for Sulu instead.
Fixes #1343
Build Artifacts