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

[Vanilla Fix] Slow Down Darunia's Dance #3438

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

Malkierian
Copy link
Contributor

@Malkierian Malkierian commented Nov 24, 2023

This adds a fix enhancement to slow down Darunia's dance during Saria's Song. Defaults to on.

Known issue: while the values used make the dance virtually perfectly synced during the cutscene, as soon as you get to the last textbox and let it sit instead of closing the box, the dance desyncs within 2 loops. Honestly don't know how that's happening. Figure most people won't be looking past that box very long, so it might still be mergeable.

Next PR work: timing of camera and textbox changes (I tried for 2 hours and just couldn't figure it out XD).

Build Artifacts

@Malkierian Malkierian changed the base branch from develop to develop-macready November 24, 2023 06:25
@Malkierian Malkierian changed the title [Vanilla Fix] Slow Darunia's Dance [Vanilla Fix] Slow Down Darunia's Dance Nov 24, 2023
Comment on lines +107 to +113
{ &gDaruniaDancingLoop1Anim, 0.78f, 0.0f, -1.0f, ANIMMODE_ONCE, -10.0f }, //
{ &gDaruniaDancingLoop1Anim, 0.77f, 0.0f, -1.0f, ANIMMODE_ONCE, 0.0f }, // hop
{ &gDaruniaDancingLoop2Anim, 0.78f, 0.0f, -1.0f, ANIMMODE_ONCE, 0.0f }, // from hop to spin
{ &gDaruniaDancingLoop3Anim, 0.77f, 0.0f, -1.0f, ANIMMODE_ONCE, 0.0f }, // spin
{ NULL },
{ NULL },
{ &gDaruniaDancingLoop4Anim, 0.78f, 0.0f, -1.0f, ANIMMODE_ONCE, 0.0f }, // from spin to hop
Copy link
Contributor

@Archez Archez Nov 27, 2023

Choose a reason for hiding this comment

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

These 0.77 and 0.78 feel arbitrary.
I believe someone said that the animation key frames were adjusted on PAL to account for the 50hz vs 60hz difference so that it stayed in sync with the music,
From my understanding PAL ran at 17 fps instead of the true 20 (16.67 rounded up from 20 * (50/60))

Based on that, I believe these values should all either be:
0.85 (derived from 17/20), or
0.8333... (derived from 50/60)

Copy link
Contributor

Choose a reason for hiding this comment

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

But could the n64 run the cutscene at full speed? I know the frog game timing is a hardware slowdown not existing on the port issue

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, this is working against both the PAL adjustment and the lag timing, I believe. I had tried .85 ish before and it was still too fast.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair, I wasn't aware of any lag due to original hardware. In my testing it is hard to really tell the exact speed it should be anyways, so it seemed fine to me 🤷‍♂️

Then maybe just add a TODO comment to come back when NTSC is added so we don't forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a little easier for someone like me who might have had more extensive experience and training in music XD

@@ -255,7 +275,13 @@ void func_809FE040(EnDu* this) {
if (this->unk_1E6 >= 8) {
this->unk_1E6 = 0;
}
Animation_ChangeByInfo(&this->skelAnime, sAnimationInfo, animationIndices[this->unk_1E6]);
// #region SOH[Enhancement]
if (CVarGetInteger("gEnhancements.FixDaruniaDanceSpeed", 1)) {
Copy link
Contributor

@Archez Archez Nov 27, 2023

Choose a reason for hiding this comment

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

We only support PAL, but we should probably add a PAL check here so that in the future when NTSC is added, this fix wont be applied.

You can do that with the following (ref z_file_nameset_PAL.c:707)
ResourceMgr_GetGameRegion(ResourceMgr_GameHasMasterQuest() && ResourceMgr_GameHasOriginal()) == GAME_REGION_PAL

Where it will read the region value of either the first rom if only one rom is applied, or the second rom if two roms are applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considering the rest of the convo, NTSC will likely need a different set of adjustments, because of lag still existing, but yeah, the check could still be there. Honestly, I'm hoping for a better method by the time NTSC support comes around XD

@@ -271,7 +297,13 @@ void func_809FE104(EnDu* this) {
if (Animation_OnFrame(&this->skelAnime, this->skelAnime.endFrame)) {
this->unk_1E6++;
if (this->unk_1E6 < 4) {
Animation_ChangeByInfo(&this->skelAnime, sAnimationInfo, animationIndices[this->unk_1E6]);
// #region SOH[Enhancement]
if (CVarGetInteger("gEnhancements.FixDaruniaDanceSpeed", 1) && this->unk_1E6 <= 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@@ -1064,6 +1064,9 @@ void DrawEnhancementsMenu() {
UIWidgets::Tooltip("Prevents immediately falling off climbable surfaces if climbing on the edges.");
UIWidgets::PaddedEnhancementCheckbox("Fix Link's eyes open while sleeping", "gFixEyesOpenWhileSleeping", true, false);
UIWidgets::Tooltip("Fixes Link's eyes being open in the opening cutscene when he is supposed to be sleeping.");
UIWidgets::PaddedEnhancementCheckbox("Fix Darunia dancing too fast", "gEnhancements.FixDaruniaDanceSpeed",
true, false, false, "", UIWidgets::CheckboxGraphics::Cross, true);
UIWidgets::Tooltip("Fixes Darunia's dancing speed so he dances to the beat of Saria's Song, like in vanilla.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should mention that this would only apply to PAL.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that would only lead to confusion as long as we don't support NTSC.

@briaguya-ai
Copy link
Contributor

briaguya-ai commented Nov 29, 2023

hardware

https://youtu.be/HRY_9u2CTO0?t=11148

SoH 8.0.2

charliedancescaled.mp4

This PR

malkdancescaled.mp4

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.

love it :shipit:

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Approving as is. Any NTSC feedback can be resolved later when we actually support NTSC 👍

@briaguya-ai briaguya-ai merged commit f14c390 into HarbourMasters:develop-macready Nov 29, 2023
8 checks passed
A-Green-Spoon pushed a commit to A-Green-Spoon/Shipwright that referenced this pull request Nov 30, 2023
* Fix Darunia's dancing animation speed (static for now; needs toggle later).

* Moved previous code to toggleable fix in enhancements. Tweaked speed factors, partially successful.
@Malkierian Malkierian deleted the darunia-anim branch January 2, 2024 05:49
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