Skip to content

Expand the chart editor's user guide - #146

Open
MAZ12211 wants to merge 2 commits into
FunkinCrew:developfrom
MAZ12211:main
Open

Expand the chart editor's user guide#146
MAZ12211 wants to merge 2 commits into
FunkinCrew:developfrom
MAZ12211:main

Conversation

@MAZ12211

@MAZ12211 MAZ12211 commented Mar 21, 2025

Copy link
Copy Markdown
  • Bumps up the dialogue size of the user guide
  • Adds a Patterns page which contains a way to describe patterns in text-form and lists all common patterns from most to least common in the FNF space.
  • Adds a Terms page which contains must-know terms (note, chord, BPM, offset) and general terms (Pitch relevance, Layering, Sightread).
  • Adds a Tips page which contains advice on how to make a good chart

All references are listed in the file
Thanks to Friday Night Moddin's discord server (#charting-problems) and Etterna's discord server for feedback

Funkin_C3iFCEoywr.mp4

This is my first time using HaxeUI, so expect wacky XML code
It may have some stuff missing like how to get feedback or what FNFC means or a UI rundown of the editor, but I didn't have enough time to include them as I don't know a whole lot about them and didn't want to invest a lot of time on it.

@Hundrec Hundrec added size: large A large pull request with more than 100 changes. type: enhancement Provides an enhancement or new feature. topic: chart editor Related to the operation of the Chart Editor. status: pending triage Awaiting review. labels Mar 21, 2025

@EliteMasterEric EliteMasterEric left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have a few changes, and our other reviewers might have some too, but overall this is excellent! I love the in-line images.

It may have some stuff missing like what FNFC means

.fnfc is short for Friday Night Funkin' Chart. We might have other extensions in the future like .fnfmod or something

or a UI rundown of the editor

This was actually the main intent for the Help menu, but this is nice to have as well.

Comment thread preload/data/ui/chart-editor/dialogs/user-guide.xml Outdated
</vbox>

<vbox width="100%">
<label width="100%" text="Treat these as tips I recommend to follow and not rules you should follow as charters have different perspectives on what is considered a bad chart and a good chart.\n\n\n" style="font-weight: bold" />

@EliteMasterEric EliteMasterEric Mar 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We'll probably have our charting team proofread and tweak these tips to help match their style of charting (but that'll be after merge).

@Lasercar

Lasercar commented Mar 21, 2025

Copy link
Copy Markdown
Contributor

You might want to use this chance to update the hotkeys too with the ones that are currently missing (/ will be added by PRs).

Also, I wouldn't have made these changes on the main branch, as now you'll be waiting for this to be merged before you can fix/change anything else in the assets.

@MAZ12211

Copy link
Copy Markdown
Author

Also, I wouldn't have made these changes on the main branch, as now you'll be waiting for this to be merged before you can fix/change anything else in the assets.

Oh yeah I forgot doing that in my fork, don't know what to do at this point lol

You might want to use this chance to update the hotkeys too with the ones that are currently missing (/ will be added by PRs).

What missing hotkeys?

@Lasercar

Lasercar commented Mar 21, 2025

Copy link
Copy Markdown
Contributor

Also, I wouldn't have made these changes on the main branch, as now you'll be waiting for this to be merged before you can fix/change anything else in the assets.

Oh yeah I forgot doing that in my fork, don't know what to do at this point lol

You can create a branch based off your main (basically copying/saving the changes), and then reset it (delete it, and then create it again and base it off the upstream main branch, make sure the other new branch has the changes first!), but not yet - doing that now will close this issue and whatnot, so you'll still have to wait before you can do that, unless eric would be ok with this being closed/redone because it's on the main branch?

You might want to use this chance to update the hotkeys too with the ones that are currently missing (/ will be added by PRs).

What missing hotkeys?

Hmm, I though the middle mouse click one wasn't in there, but it is, so it's only just the PR ones that are missing, so don't worry about that.

Wait, I think the shift + right click on a note isn't mentioned in it?

That, and the middle mouse click one is wrong, is should be middle click + mouse move OR arrows/mouse wheel.

And, there's a few edit ones that aren't in it too:

Paste unsnapped, flip notes, select all events (select all is actually select all notes)

And also, right clicking on a hold of a hold note removes it.

@EliteMasterEric

Copy link
Copy Markdown
Member

I went to review this but all the images from the video are missing! They're not part of the PR either.

@EliteMasterEric EliteMasterEric added status: needs revision Cannot be approved because it is awaiting some work by the contributor. and removed status: pending triage Awaiting review. labels Apr 3, 2025
@MAZ12211

MAZ12211 commented Apr 4, 2025

Copy link
Copy Markdown
Author

OH Right! I forgot!

@MAZ12211

MAZ12211 commented Apr 4, 2025

Copy link
Copy Markdown
Author

@EliteMasterEric Should work now 👍

@MAZ12211
MAZ12211 requested a review from EliteMasterEric April 4, 2025 11:35
@Lasercar

Lasercar commented Apr 4, 2025

Copy link
Copy Markdown
Contributor

Pro tip, use github desktop so you can do all that with one commit/change, rather than 9.

@MAZ12211

MAZ12211 commented Apr 4, 2025

Copy link
Copy Markdown
Author

Pro tip, use github desktop so you can do all that with one commit/change, rather than 9.

I did it in the browser and vscode in the browser because I don't actually have the fork stored in my hard drive (slow WIFI with cloning large repos)

@Lasercar

Lasercar commented Apr 5, 2025

Copy link
Copy Markdown
Contributor

Pro tip, use github desktop so you can do all that with one commit/change, rather than 9.

I did it in the browser and vscode in the browser because I don't actually have the fork stored in my hard drive (slow WIFI with cloning large repos)

OH, I see. Carry on then.

@Lasercar

Copy link
Copy Markdown
Contributor

This PR needs a rebase. Click the dropdown you see next to the conflicts bit below, swap it to rebase and then click the button.

@MAZ12211

MAZ12211 commented Apr 30, 2025

Copy link
Copy Markdown
Author

There's no dropdown on my end, but I synced my fork just now

Screenshot_2025-04-30-15-37-55-26_3aea4af51f236e4932235fdada7d1643

@AbnormalPoof

Copy link
Copy Markdown
Member

There's 2 leftover commits from the old main branch

@MAZ12211

Copy link
Copy Markdown
Author

What do I do about that?
Genuinely asking

@MAZ12211

MAZ12211 commented May 2, 2025

Copy link
Copy Markdown
Author

Fork got synced

@Lasercar

Lasercar commented May 3, 2025

Copy link
Copy Markdown
Contributor

Fork got synced

Well yes, but actually no. It still has those two unnecessary commits. You need to rebase merge it still.

Next to the button for the update branch, you'll see a dropdown - swap it to rebase and click it.

@MAZ12211

MAZ12211 commented May 3, 2025

Copy link
Copy Markdown
Author

@Lasercar
I genuinely can't find it on my end

Screenshot_2025-05-03-18-49-09-32_3aea4af51f236e4932235fdada7d1643.jpg

@Hundrec

Hundrec commented May 3, 2025

Copy link
Copy Markdown
Member

Lasercar was referring to a button on the bottom of this PR page, but that option might not show up here either.
So you see anything on this page?

@MAZ12211

MAZ12211 commented May 3, 2025

Copy link
Copy Markdown
Author

@Hundrec Nope

image

@Hundrec

Hundrec commented May 3, 2025

Copy link
Copy Markdown
Member

Cool, then you can leave this PR like this for now. Thanks for writing this up!

@MAZ12211 MAZ12211 changed the title Expanded the chart editor's user guide Expand the chart editor's user guide May 20, 2025
@github-actions github-actions Bot added the pr: art PR modifies art or animations. label Jun 2, 2025
@AbnormalPoof AbnormalPoof added the status: reviewing internally Under consideration and testing. label Aug 4, 2025
@AbnormalPoof

Copy link
Copy Markdown
Member

Cleaned up the history on this one

@AbnormalPoof AbnormalPoof left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

THERE'S NO IMAGES AAAAAAAAHHHHHHHHHHHHHHHHHHHHHHHHHHH!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! THE WORLD IS ENDING

@AbnormalPoof AbnormalPoof added status: needs revision Cannot be approved because it is awaiting some work by the contributor. and removed status: reviewing internally Under consideration and testing. labels Aug 4, 2025
@MAZ12211

MAZ12211 commented Aug 4, 2025

Copy link
Copy Markdown
Author

Wtf
Hold on

@AbnormalPoof
AbnormalPoof force-pushed the develop branch 3 times, most recently from e0ba6ab to bcd30e1 Compare August 4, 2025 17:51
@github-actions github-actions Bot added size: huge A huge pull request with more than 500 changes. size: large A large pull request with more than 100 changes. pr: github PR modifies GitHub metadata files. pr: documentation PR modifies documentation or README files. and removed size: large A large pull request with more than 100 changes. labels Aug 4, 2025
@MAZ12211
MAZ12211 requested a review from AbnormalPoof August 4, 2025 18:16
@github-actions github-actions Bot removed pr: github PR modifies GitHub metadata files. pr: documentation PR modifies documentation or README files. size: huge A huge pull request with more than 500 changes. labels Aug 4, 2025
@AbnormalPoof AbnormalPoof added status: reviewing internally Under consideration and testing. and removed status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Aug 5, 2025
@MAZ12211

MAZ12211 commented Aug 5, 2025

Copy link
Copy Markdown
Author

If this gets in V0.7.4, then I should mention that I just did another read through for the user guide today, and it needs proofreading at some point after merge (or now after review if you want)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: art PR modifies art or animations. size: large A large pull request with more than 100 changes. status: reviewing internally Under consideration and testing. topic: chart editor Related to the operation of the Chart Editor. type: enhancement Provides an enhancement or new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants