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

[ENHANCEMENT] Custom Popups and Countdowns #3020

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

anysad
Copy link

@anysad anysad commented Jul 13, 2024

Briefly describe the issue(s) fixed.

  • This PR aims to give players the ability to have custom popups and countdown graphics/sounds. Currently, some of the assets were moved into their own folders, so that the player has an easy way of navigating and finding them.
  • By using this new ability, modders FINALLY have the option to have their own assets for popups and countdowns. (see video below)
  • This PR fixes the countdown assets having random sizes, depending on their stage. (the scaling of normal assets were based on pixel countdown assets) (see video below)
  • This PR fixes the Song Metadata box in the chart editor, letting you finally choose notestyles easily. (see photo below) Moved to [BUGFIX] Fix Song Metadata Height funkin.assets#54

Include any relevant screenshots or videos.

  • twinsomnia notestyle (no assets) / twinsomnia-booger notestyle (with assets)
based.on.notestyle.mp4
  • Fixed countdown assets size:
fix.scaling.mp4

@Hundrec
Copy link
Contributor

Hundrec commented Jul 13, 2024

Cool stuff anysad!

@anysad
Copy link
Author

anysad commented Jul 13, 2024

Cool stuff anysad!

thank you, mister Hundrec 🫡

@anysad
Copy link
Author

anysad commented Jul 13, 2024

just saw that I accidentally made a copy of the hand_textbox texture, whoops. will fix it a little later

@EliteMasterEric EliteMasterEric added the status: pending triage The bug or PR has not been reviewed yet. label Jul 14, 2024
@EliteMasterEric
Copy link
Member

Might review this and rework it myself later, but the way I originally architected this system, the plan was that custom popups and countdowns would be defined by the current note style.

@EliteMasterEric EliteMasterEric added type: enhancement Provides an enhancement or new feature. medium A medium pull request with 100 or fewer changes labels Jul 14, 2024
@anysad
Copy link
Author

anysad commented Jul 14, 2024

the plan was that custom popups and countdowns would be defined by the current note style

Your wish is my command. Starting to work on it.

@anysad
Copy link
Author

anysad commented Jul 16, 2024

@EliteMasterEric I made it use the current note style, and if no assets were found for the specific note style, it reverts back to using the default assets. (funkin/pixel based if the note style is pixel or not)

Copy link

@JVNpixels JVNpixels left a comment

Choose a reason for hiding this comment

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

You could center the countdown to make it look cleaner, besides that this PR deserves to be merged!

@EliteMasterEric
Copy link
Member

@EliteMasterEric I made it use the current note style, and if no assets were found for the specific note style, it reverts back to using the default assets. (funkin/pixel based if the note style is pixel or not)

Each note style defines a fallback note style, all assets should check if the specific note style has an asset, then check if the fallback has it, then check if ITS fallback has it, continuing until you reach a note style with no fallback. The default funkin note style should have every asset defined.

@anysad
Copy link
Author

anysad commented Jul 16, 2024

yes eric

@anysad
Copy link
Author

anysad commented Jul 17, 2024

@EliteMasterEric Done.

  • Countdown (twinsomnia-booger/twinsomnia/funkin)
countdown.mp4
  • Popup (twinsomnia-booger/twinsomnia/funkin) (no exact order)
popup.mp4

@anysad
Copy link
Author

anysad commented Jul 17, 2024

  • Manipulating a little bit with assets (removed the 0, and it will use funkin since there is no fallback)
manipulating.with.assets.mp4
  • And you can even have different styles! (it was lagging a bit due to having a lot of trace() in the code)
pixel.AND.funkin.mp4

@anysad
Copy link
Author

anysad commented Jul 17, 2024

Also, will have to cleanup the code tomorrow a little to remove the unnecessary fetchNoteStyle() calls

@anysad
Copy link
Author

anysad commented Jul 18, 2024

You could center the countdown to make it look cleaner, besides that this PR deserves to be merged!

As you suggested, I centered it.

Copy link

@JVNpixels JVNpixels left a comment

Choose a reason for hiding this comment

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

PR 100% Merge deserved

Copy link

@Cartridge-Man Cartridge-Man left a comment

Choose a reason for hiding this comment

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

This is gonna be so helpful if this gets merged ngl

@EliteMasterEric EliteMasterEric self-assigned this Jul 28, 2024
@EliteMasterEric EliteMasterEric removed the status: pending triage The bug or PR has not been reviewed yet. label Jul 28, 2024
@EliteMasterEric EliteMasterEric added status: reviewing internally This PR is under internal review and quality assurance testing large A large pull request with more than 100 changes and removed medium A medium pull request with 100 or fewer changes labels Jul 28, 2024
@EliteMasterEric
Copy link
Member

This code looks great overall and seems to not break anything.

There are some tweaks I want to make (for example, there are a lot of merge conflicts with our internal branch) but I will make those changes myself.

@EliteMasterEric EliteMasterEric added status: accepted Approved for contribution. If it's not already merged, it may be merged on a private branch. and removed status: reviewing internally This PR is under internal review and quality assurance testing labels Jul 28, 2024
@EliteMasterEric EliteMasterEric added this to the 0.5.0 milestone Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
large A large pull request with more than 100 changes status: accepted Approved for contribution. If it's not already merged, it may be merged on a private branch. type: enhancement Provides an enhancement or new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants