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

Clean up format of Skinning/Sounds and add Nightcore sample info #10863

Merged
merged 15 commits into from
Aug 16, 2024

Conversation

CloneWith
Copy link
Contributor

This might be part of the solution of #2589

Self-check

Copy link
Member

@Walavouchey Walavouchey left a comment

Choose a reason for hiding this comment

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

  • why are only some of these in tables? the ones which have additional information could have a "Notes" column for instance
  • expand contractions ("aren't" --> "are not", etc.)
  • "osu! client", "osu! cookie" instead of "osu!client" and "osu!cookie"
  • the article should probably be renamed to something more grammatically correct than "Sounds skinning"

will check in more detail later

@CloneWith
Copy link
Contributor Author

CloneWith commented Feb 1, 2024

why are only some of these in tables?

Some of sounds are similar in filename and usage, then I tried to merge them into one line, for example the hitsound sets and the key-press-* set.

"osu! client", "osu! cookie" instead of "osu!client" and "osu!cookie"
the article should probably be renamed to something more grammatically correct than "Sounds skinning"

Just kept them without changes when rewriting, will change later.

the article should probably be renamed to something more grammatically correct than "Sounds skinning"

Skinnable sounds maybe I think?

@cl8n
Copy link
Member

cl8n commented Feb 7, 2024

I have very far down a todo list that I wanted to write about these files:

nightcore-kick.wav
nightcore-hat.wav
nightcore-clap.wav
nightcore-finish.wav

idk if I'll ever get to it but maybe it's relevant to do in this PR.

@CloneWith
Copy link
Contributor Author

CloneWith commented Feb 8, 2024

Have read osu! lazer source code and tried on stable by myself. It's really odd that I didn't find nightcore-hat.wav being used in stable (not sure) but did in lazer, and the corresponding wiki page) seems unclear about this, not even mentioning it.

And found the sound playing mechanic of */3 or */6 times, can add it to this article (or Game modifier/Target Practice?)

@TicClick TicClick self-requested a review February 28, 2024 00:02
@osu-wiki-observatory osu-wiki-observatory bot mentioned this pull request May 2, 2024
2 tasks
@cl8n
Copy link
Member

cl8n commented Aug 14, 2024

in stable

  • the files are sourced from your skin, but not beatmap skin
  • nightcore-hat plays only when slidertickrate is a multiple of 2
  • nightcore-finish doesn't play if it's the first beat of a timing point that is omitting first barline
  • non-4/4 works differently, to be hoenst it's so confusing and these skin elements are so niche that I wouldn't even bother documenting non-4/4

otherwise it works same as lazer

Copy link
Member

@cl8n cl8n left a comment

Choose a reason for hiding this comment

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

rest of the changes seem fine, there's still a lot to improve about this article but the new layout here is certainly better

just fix NC and that's good for this PR

@cl8n cl8n removed the request for review from TicClick August 16, 2024 02:43
@cl8n cl8n enabled auto-merge August 16, 2024 03:12
@cl8n cl8n disabled auto-merge August 16, 2024 03:12
@cl8n cl8n changed the title Rewrite Skinning/Sounds Clean up format of Skinning/Sounds and add Nightcore sample info Aug 16, 2024
@cl8n cl8n enabled auto-merge August 16, 2024 03:13
@cl8n cl8n merged commit 9626b5c into ppy:master Aug 16, 2024
2 checks passed
@CloneWith CloneWith deleted the Patch-Skin+ branch August 16, 2024 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants