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

[Chart Enhancement] Fix Eggnog Erect ending notes + small adjustments #103

Closed
wants to merge 1 commit into from

Conversation

afreetoplaynoob
Copy link

@afreetoplaynoob afreetoplaynoob commented Nov 21, 2024

The ending of Eggnog Erect was using the wrong BPM (80, instead of 70, half of the song's main speed) and the four ending notes were off-sync, so I fixed those things and made some other changes to the chart.

Fixes this in both Erect and Nightmare difficulties.

Screenshot 2024-11-20 23 17 07

@afreetoplaynoob
Copy link
Author

I thought I had already gotten rid of the Hard difficulty but when tweaking the files again I forgot to get rid of it again. Also wasn't aware of the charter being removed. Thank you for pointing it out!

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 21, 2024

No problem!
I've been curious: What is the full list of changes you made to the chart?
Ideally, we would be able to go in and manually edit the notes and events without re-exporting the song and making comparison difficult. (See line changes for #96 as an example)

@afreetoplaynoob
Copy link
Author

afreetoplaynoob commented Nov 21, 2024

Excluding the aformentioned BPM change and ending notes:

Section 11 - Parents (Christmas)

Added a 32nd down note to accurately chart the grace.

Screenshot 2024-11-21 10 53 32

Section 15 - Parents (Christmas)

Added a 32nd right note to accurately chart the grace.

Screenshot 2024-11-21 10 54 27

Section 47 - Parents (Christmas)

Added an up note for the small note Daddy Dearest sings here.

Screenshot 2024-11-21 10 55 21

Section 56 - Boyfriend (Christmas)

Removed the down note grace here and moved the right note a bit earlier.

Screenshot 2024-11-21 11 01 02

Section 30 - Boyfriend (Christmas) [ERECT Exclusive]

Moved the three notes immediately after the event a bit lower to sync with the singing.

Screenshot 2024-11-21 11 04 18

Note

The section 56 change isn't in the PR itself yet, as I only figured out the proper timing for the note when gathering screenshots for this comment.

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 21, 2024

Thanks for the changelog!
However, your screenshots (and your note changes) were affected by FunkinCrew/Funkin#3580, which can be fixed by re-selecting Eggnog after switching to Nightmare difficulty.
Another tip is to select the notes you made changes to for easy readability!

@afreetoplaynoob
Copy link
Author

Yes, I noticed I forgot to highlight the notes after I posted my comment. Hopefully I've managed to still describe what and where the chart changes were supposed to be regardless.
And thank you for the switching difficulty tip! It was... inconvenient to playtest with this bug, to say the least; will try it later!

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 21, 2024

Yeah, the grid is super important!
I'll clarify that it looks like the new notes you've placed align in 32nds with the regular variation's 150bpm grid, affecting the accuracy of the placements. Please revise these placements on the correct grid and update me with the changes!

@afreetoplaynoob
Copy link
Author

Alright, here's the changes again, now in their proper positions and sections, with the changed notes highlighted and already manually edited into the original file:

Section 10 - Parents

Screenshot 2024-11-21 20 26 23

Section 14 - Parents

Screenshot 2024-11-21 20 26 40

Section 44 - Parents

Screenshot 2024-11-21 20 26 57

Section 52 - Boyfriend

Screenshot 2024-11-21 20 27 06

Section 71 - Both

Here's how it looks like in 70 BPM:
Screenshot 2024-11-21 20 27 33
And how it looks like in 140 BPM, for easier viewing:
Screenshot 2024-11-21 20 29 12

Section 28 - Boyfriend [ERECT Only]

Screenshot 2024-11-21 20 27 54

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 22, 2024

Thanks so much! I'll look into these changes when I have the time and create a branch in my fork that manually edits the json files without re-exporting the chart.

That way we can use the GitHub diffs for comparison and save some disk space for the game!

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 22, 2024

Oh whoops, didn't notice you had noted that you already made those manual edits.
Can you commit those changes to this PR so I can examine them?

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 22, 2024

Also just noticed that you already made a commit for those manual edits.
I wonder why GitHub is displaying the diffs as if they're replacing entire chunks instead of just the parts you changed...

@afreetoplaynoob
Copy link
Author

Pretty sure it's from all the changes and addendums we've made to the file; this PR started with an exported chart file and then went back to the original file format, so it's seeing it as the diffs to the exported format.

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 22, 2024

Got it, then I will manually copy over your files to my fork and see if the diffs are small there

@Hundrec
Copy link
Collaborator

Hundrec commented Nov 22, 2024

Just checked out your changes and they're great!
If you can squash this PR into one commit, it should look like this branch and be easily comparable.

One minor detail is that I would keep the "generatedBy" in both files 0.3.2, rather than 0.5.3 since this is now a manual edit.

@afreetoplaynoob
Copy link
Author

afreetoplaynoob commented Nov 22, 2024

Looks like I miscalculated there, the diffs are still messed up... I either squashed wrong or that's just how it was going to end up anyway, seeing as it was still 8 commits that probably collectively messed up the format/diffs.
At this point we're better off if l delete the repo and close this PR so I can make a new one with good diffs.

@AbnormalPoof AbnormalPoof added the status: rejected PR did not pass review or cannot be approved. label Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: rejected PR did not pass review or cannot be approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants