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

Add ability to rename the skin #31220

Merged
merged 5 commits into from
Dec 24, 2024
Merged

Conversation

normalid-awa
Copy link
Contributor

@normalid-awa normalid-awa commented Dec 21, 2024

Added a button to the skin section of the setting, when clicked, a popover will pop in, containing a text box and button for inputting the new skin name.

osu.2024.12.21.-.19.57.06.01.mp4

@peppy
Copy link
Member

peppy commented Dec 21, 2024

This should be in the skin layout editor similar to changing beatmap metadata.

@normalid-awa
Copy link
Contributor Author

normalid-awa commented Dec 22, 2024

This should be in the skin layout editor similar to changing beatmap metadata.

image
like this?
if so I think it's too hidden as the editor is a layout editor, it shouldn't modify the metadata kinds of stuff.

@Artcens
Copy link

Artcens commented Dec 22, 2024

like this? if so I think it's too hidden as the editor is a layout editor, it shouldn't modify the metadata kinds of stuff.

I kind of agree with this, but placing it in settings also makes settings more cluttered, especially with taking a whole button for one simple feature.

Since you don't want to make it "hidden," but I also want settings much cleaner, my suggestion is to remake the layout of the skin section with something like this:

image

The image above is just an illustration, so take this as an idea.
might as well clean the rest of the settings.

@peppy
Copy link
Member

peppy commented Dec 22, 2024

i can live with the proposal as a stop gap until someone realises the skin editor UI/UX.

the code to perform the rename should be portable enough that it can be moved when we have the chance to do so

@bdach
Copy link
Collaborator

bdach commented Dec 23, 2024

What we need is an actual skin management view at this point. Both solutions proposed here feel bad imo.

@peppy
Copy link
Member

peppy commented Dec 23, 2024

What we need is an actual skin management view at this point. Both solutions proposed here feel bad imo.

Yes, the hope would be something like the mod presets panels or collection management overlay, where each skin is a panel or whatever and could be renamed locally.

But as a skin author it should be feasible to change metadata from the skin layout editor (I'm assuming you don't have an issue with that as long as it's done similarly to the beatmap editor).

Renaming skins is a very commonly requested feature so even if a button at settings feels "bad" i'd still be willing to merge it if it's isolated and works.

@bdach
Copy link
Collaborator

bdach commented Dec 23, 2024

In skin layout is fine I guess as a transitional thing at least but as far as jamming it in settings goes, I'd prefer effort to go into an actual management view rather than Even More Settings.

@peppy peppy self-requested a review December 24, 2024 03:56
@peppy
Copy link
Member

peppy commented Dec 24, 2024

I'm gonna make a call to get this in as a short-term QoL improvement with the assumption it's going to be rewritten in the future. It will benefit players immediately.

Removed localisable strings because it doesn't make sense to localise them for such a temporary usage.

osu! 2024-12-24 at 04 19 39

@peppy peppy merged commit 362311d into ppy:master Dec 24, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants