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] Freeplay Icon Metadata #3042

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

Conversation

anysad
Copy link

@anysad anysad commented Jul 18, 2024

Briefly describe the issue(s) fixed.

  • This PR aims to fix freeplay pixel icons being hardcoded, and give modders the opportunity to have the name of the icon whatever they want
  • This PR also fixes some offsets for the characters in the freeplay menu.

Include any relevant screenshots or videos.

  • The freeplay icon metadata takes 4 params: name, scale, flipX and offsets
    image

@anysad anysad changed the base branch from main to develop July 18, 2024 21:12
@charlesisfeline
Copy link

we already have #2465 but that one has conflicts so eh whatever-

@anysad
Copy link
Author

anysad commented Jul 19, 2024

we already have #2465 but that one has conflicts so eh whatever-

Ah, forgot it existed. Will leave this open for Eric to decide which one to use. Also, what conflicts does the PR you mentioned have?

@AbnormalPoof
Copy link

AbnormalPoof commented Jul 19, 2024

#2465 Hasn't been updated to 0.4.x's codebase, which causes it to have merge conflicts... I think?

@MadBear422
Copy link
Contributor

MadBear422 commented Jul 21, 2024

#2465 Hasn't been updated to 0.4.x's codebase, which causes it to have merge conflicts... I think?

The code itself functions fine with the most recent version of the game. The only conflict that it has pertains to the assets repo where I modified some json files to account for the corrected icon offsets. I just need to update that change to be up to date with the newer version of the game

@anysad
Copy link
Author

anysad commented Jul 21, 2024

#2465 Hasn't been updated to 0.4.x's codebase, which causes it to have merge conflicts... I think?

The code itself functions fine with the most recent version of the game. The only conflict that it has pertains to the assets repo where I modified some json files to account for the corrected icon offsets. I just need to update that change to be up to date with the newer version of the game

I guess I should close this PR then?

@MadBear422
Copy link
Contributor

#2465 Hasn't been updated to 0.4.x's codebase, which causes it to have merge conflicts... I think?

The code itself functions fine with the most recent version of the game. The only conflict that it has pertains to the assets repo where I modified some json files to account for the corrected icon offsets. I just need to update that change to be up to date with the newer version of the game

I guess I should close this PR then?

I resolved the merge conflicts with my PR. I'd suggest that you leave this one here for Eric to determine whichever one is more suited for the game

@anysad
Copy link
Author

anysad commented Jul 22, 2024

#2465 Hasn't been updated to 0.4.x's codebase, which causes it to have merge conflicts... I think?

The code itself functions fine with the most recent version of the game. The only conflict that it has pertains to the assets repo where I modified some json files to account for the corrected icon offsets. I just need to update that change to be up to date with the newer version of the game

I guess I should close this PR then?

I resolved the merge conflicts with my PR. I'd suggest that you leave this one here for Eric to determine whichever one is more suited for the game

Alright, will leave this open for now 🙂

@EliteMasterEric EliteMasterEric added status: pending triage The bug or PR has not been reviewed yet. type: enhancement Provides an enhancement or new feature. mods Issue is related to the use of mods. large A large pull request with more than 100 changes status: reviewing internally This PR is under internal review and quality assurance testing and removed status: pending triage The bug or PR has not been reviewed yet. labels Jul 23, 2024
@EliteMasterEric
Copy link
Member

Both this and #2465 add data to the opponent character to determine which icon to use, but I was thinking about it and it might make more sense to have the icon be defined by the song metadata?

This would give more flexibility to choose which icon you want for the song in exchange for making it so that you can't define an icon for all of a character's songs at once. Thoughts?

@AbnormalPoof
Copy link

Wouldn't that mean that you'd have to define the pixel icon for each song in the metadata, even if multiple songs feature the same opponent? There's also the Character Dropdown in the chart editor to worry about.

@anysad
Copy link
Author

anysad commented Jul 28, 2024

I mean as times move on, there might be added a feature that multiple characters sing (different to week5 parents, as just two or more characters, instead of one having -alt), so it might make sense that people want to personally customise for each song?

@anysad
Copy link
Author

anysad commented Jul 28, 2024

Besides that, we can just make it use song.characters.opponent if no asset is customly set. (As it is doing right now)

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 mods Issue is related to the use of mods. status: reviewing internally This PR is under internal review and quality assurance testing type: enhancement Provides an enhancement or new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants