Closes https://github.com/ppy/osu/issues/32908.
Have you ever been in a situation wherein you find out you fixed a bug
that you didn't know existed, but that makes *another* bug appear
because it was relying on the other bug? This is where I'm at right now.
But, to start from the top.
`TextFlowContainer.Text` (the setter) is a convenience property that you
use to set the text in one go. Internally it uses `AddText()`:
https://github.com/ppy/osu-framework/blob/681900ffb70adfeede4e3fa32a69da66252691ee/osu.Framework/Graphics/Containers/TextFlowContainer.cs#L81-L94
`AddText()`'s xmldoc says:
The \n character will create a new paragraph, not just a line break.
If you need \n to be a line break, use <see cref="AddParagraph{TSpriteText}(LocalisableString, Action{TSpriteText})"/> instead.
https://github.com/ppy/osu-framework/blob/681900ffb70adfeede4e3fa32a69da66252691ee/osu.Framework/Graphics/Containers/TextFlowContainer.cs#L226-L239
That's right. This portion of xmldoc was *straight up false* and
*silently broken* before https://github.com/ppy/osu-framework/pull/6556.
If you want to check that out yourself, apply the following patch to
framework:
diff --git a/osu.Framework.Tests/Visual/Containers/TestSceneTextFlowContainer.cs b/osu.Framework.Tests/Visual/Containers/TestSceneTextFlowContainer.cs
index 464f47c2c..e1ad521a7 100644
--- a/osu.Framework.Tests/Visual/Containers/TestSceneTextFlowContainer.cs
+++ b/osu.Framework.Tests/Visual/Containers/TestSceneTextFlowContainer.cs
@@ -180,6 +180,22 @@ public void TestAlignmentIsCorrectWhenLineBreaksAtLastWordOfParagraph(Anchor tex
});
}
+ [Test]
+ public void TestSetTextWithNewLine()
+ {
+ AddStep("set text", () => textContainer.Text = "this text\nhas a newline");
+ AddStep("clear and add text", () =>
+ {
+ textContainer.Clear();
+ textContainer.AddText("this text\nhas a newline");
+ });
+ AddStep("clear and add paragraph", () =>
+ {
+ textContainer.Clear();
+ textContainer.AddParagraph("this text\nhas a newline");
+ });
+ }
+
private void assertSpriteTextCount(int count)
=> AddAssert($"text flow has {count} sprite texts", () => textContainer.ChildrenOfType<SpriteText>().Count() == count);
On `master`, there will be a difference between the first two steps, and
the third. On 2025.321.0, *there will be none*.
My working theory as to why this was always busted is that the
corresponding code that was there before in
https://github.com/bdach/osu-framework/blob/c31a48178889ca2f9b4d257d2d64915eee90338a/osu.Framework/Graphics/Containers/TextFlowContainer.cs#L454-L458
just straight up ran too late. *The height of the container is being
changed after the flow has laid itself out, without adjusting subsequent
children in any way.*
There is potentially a discussion to be had as to whether the emergent
behaviour of `TextFlowContainer.Text` with respect to `\n` character is
correct, but I'm just going to start with this diff and see what the
reaction is.
Kind of a big oversight this. In wanting to get the leaderboard
refactors to move forward I sort of didn't realise the fact that all of
the error handling related to online status and such in
`BeatmapLeaderboard` kind of... can't stay there...
It's also an all-or-nothing business too - moving this stuff can't
really be done only in part.
Not sure whether tests are warranted if it's more or less moving logic
across?
Regressed with 102085668f because the
stupid magic alpha transform addition was also implicitly changing the
value of `IsDrawable` from false to true because that property checks
for presence of any commands.
Apparently past me, in his infinite wisdom, did not decide it pertinent
to test that change against, you know, *a beatmap with a storyboard*.
Great job, past me, good show all around.
Supersedes https://github.com/ppy/osu/pull/32817.
The messaging of the failure to the user is maybe not the cleanest, but
I'm not sure it's worth putting time in to improve it?
Closes https://github.com/ppy/osu/issues/32825.
Tested manually to fix the issue. Setting up test coverage for this is
going to likely take over an hour compared to the 30 second fix, so
please advise if required. I couldn't find any existing tests which
perform this flow.
Audio offset is integer based in configuration, so let's make sure not
to show that there's an applicable offset when the value difference is
too low.
I've also fixed rounding to match expectations (`AudioOffset` is
precision limited to integer), and handled the case where a user adjusts
the slider but also has a suggested offset – previously it would not
enable the button after slider adjustments but now it will work as
expected.