osu icon indicating copy to clipboard operation
osu copied to clipboard

Always allow a map's user-tags to be read

Open NotStirred opened this issue 6 months ago • 5 comments

Currently when viewing a score either in the results screen or on the leaderboard you have to have at least C on the map to view its tags.

This PR allows a user to view any score and see its tags, though they may only modify it if they meet the existing requirements.

User meets requirements to modify (behaviour is unchanged):

Screencast_20250607_224045.webm

User doesn't meet the requirements:

Screencast_20250607_223845.webm

I didn't want to make other changes, though given that both the tags and message are now viewable at once it probably makes sense to left-align the text. Also some UX, the buttons could probably convey that you aren't allowed to edit them (flashing red instead of white, etc.).

If this PR is wanted I'm more than happy to make those changes before merge.

NotStirred avatar Jun 07 '25 22:06 NotStirred

Currently UserTagControl has no tests that verify the behaviour, given that I'm changing the behaviour should I add tests?

NotStirred avatar Jun 07 '25 22:06 NotStirred

Currently UserTagControl has no tests that verify the behaviour, given that I'm changing the behaviour should I add tests?

Test coverage is in TestSceneStatisticsPanel. All that it's sufficient to do is

diff --git a/osu.Game.Tests/Visual/Ranking/TestSceneStatisticsPanel.cs b/osu.Game.Tests/Visual/Ranking/TestSceneStatisticsPanel.cs
index f92dc0313e..b682ec7265 100644
--- a/osu.Game.Tests/Visual/Ranking/TestSceneStatisticsPanel.cs
+++ b/osu.Game.Tests/Visual/Ranking/TestSceneStatisticsPanel.cs
@@ -256,6 +256,7 @@ public void TestTaggingWhenRankTooLow()
             var score = TestResources.CreateTestScoreInfo();
             score.Rank = ScoreRank.D;
 
+            setUpTaggingRequests(() => score.BeatmapInfo);
             AddStep("load panel", () =>
             {
                 Child = new PopoverContainer
@@ -278,6 +279,7 @@ public void TestTaggingConvert()
             var score = TestResources.CreateTestScoreInfo();
             score.Ruleset = new ManiaRuleset().RulesetInfo;
 
+            setUpTaggingRequests(() => score.BeatmapInfo);
             AddStep("load panel", () =>
             {
                 Child = new PopoverContainer



I'm not sure about this in general. One thing is that the tags are displayed in song select v2 so I'm unsure whether this helps much anything at all.

Secondly, even though that the "read-only" mode seems to be correctly preventing tagging, the UX does not reflect the fact that tagging is prohibited at all:

https://github.com/user-attachments/assets/1539d9ce-6e92-473d-8f51-f7f5b21c669a

i.e. there is still visual feedback and no dimming of the control or anything.

@peppy before this goes any further, my question would be - do we want this even? I'm not convinced that we do.

bdach avatar Jun 09 '25 07:06 bdach

I don't have any strong feelings against the change. I also don't mind the tags highlighting/animating on hover, personally.

peppy avatar Jun 09 '25 07:06 peppy

I am mostly unfamiliar with more modern C#, I hope I have interpreted "init property" correctly

Additionally, perhaps UserModifiable is a better name for the field than Writable, do let me know.

NotStirred avatar Jun 11 '25 03:06 NotStirred

@peppy be aware that author here PR'd off their master if you're going to try pushing changes

bdach avatar Jun 11 '25 08:06 bdach

oops, wrong button here (fixed issue via https://github.com/peppy/osu/commit/584362efc3b25828663bdf894fb54f053d98bcc8).

peppy avatar Jul 29 '25 07:07 peppy