Make TextFlowContainer wrap on individual characters as a last resort - #6775
Open
srb2thepast wants to merge 2 commits into
Open
Make TextFlowContainer wrap on individual characters as a last resort#6775srb2thepast wants to merge 2 commits into
srb2thepast wants to merge 2 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| [Test] | ||
| public void TestOverflowCharacterSplitting() | ||
| { | ||
| string overflowText = "LoremipsumdolorsitametconsecteturadipiscingelitIntegermattiseuturpisvitaeposuereOrcivariusnatoquepenatibusetmagnisdisparturientmontesnasceturridiculusmusEtiammaurisnibhfaucibusmaximusornareeuultricesutipsumProinrhoncusnuncetfaucibuspretiumnislnuncdapibusmassaetscelerisquenibhligulaidodioPraesentdapibusexsednuncegestasinplaceratrisusmattisNullasedligulavelitVestibulumauctorportaerosetcondimentumEtiamlaoreetnuncneclaciniapulvinarMaurishendreritmiataliquetcondimentumexexcursusdolornonportaeraterosidjustoCrasmalesuadatinciduntnuncattinciduntrisuseleifendidMaecenashendreritvenenatismietlobortisEtiamsemtortorelementumegetlacusnonportatristiquequamMorbisedlaciniaodioPhasellusutpretiumnuncFuscevitaemollismagnavelscelerisquedui"; |
| first = false; | ||
| } | ||
|
|
||
| void addCharacters(string text) |
| { | ||
| if (AutoSizeAxes.HasFlagFast(Axes.X)) | ||
| return true; | ||
| if (Parent == null) |
peppy
reviewed
Jul 17, 2026
| /// <returns>Whether the text fits within the bounds of this TextFlowContainer.</returns> | ||
| /// <exception cref="InvalidOperationException">If this container's <see cref="RelativeSizeAxes"/> == <see cref="Axes.X"/> and/or it hasn't loaded (Parent == null)</exception> | ||
| /// <remarks>The <paramref name="spriteText"/> provided will be pre-loaded by being passed into <see cref="CompositeDrawable.LoadComponent{TLoadable}(TLoadable)"/> to get its width.</remarks> | ||
| public bool TextFitsInFlow(SpriteText spriteText) |
Member
There was a problem hiding this comment.
should definitely not be public unless there's a use case for it.
| if (AutoSizeAxes.HasFlagFast(Axes.X)) | ||
| return true; | ||
| if (Parent == null) | ||
| throw new InvalidOperationException($"Cannot invoke {nameof(TextFitsInFlow)} before this {nameof(TextFlowContainer)} has a parent. Consider calling after this container has loaded."); |
Member
There was a problem hiding this comment.
should definitely not throw because there is no way a user can fix this.
| if (Parent == null) | ||
| throw new InvalidOperationException($"Cannot invoke {nameof(TextFitsInFlow)} before this {nameof(TextFlowContainer)} has a parent. Consider calling after this container has loaded."); | ||
| if (Flow.LoadState < LoadState.Ready) | ||
| throw new InvalidOperationException($"Cannot invoke {nameof(TextFitsInFlow)} before this container's {nameof(InnerFlow)} is ready. Consider calling after this container has loaded."); |
|
|
||
| protected partial class InnerFlow : FillFlowContainer | ||
| { | ||
| protected internal void LoadSpriteTextComponent(SpriteText item) |
Member
There was a problem hiding this comment.
this just cannot exist. you haven't even explained what or why or a
Author
There was a problem hiding this comment.
This was included so the TextFitsInFlow method could know what a SpriteText's size would be in relation to the InnerFlow if it were added, without needing to actually add it as a child to the InnerFlow. (If a large SpriteText were unknowingly added as a child, it would be loaded only to be removed from InnerFlow soon after and have each of its characters re-created & added, leading to unwanted results like flashing.)
The tradeoffs of this may not be reasonably worth it though. I agree that its best for this not to be included.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There are a few aspects of this implementation that are worth a second opinion, like how part recreation must be done in
LoadComplete()rather thanLoadCompleteAsync()due to the container's size being unknown at that point. Open to heavy feedback on where/how to improve this and any other aspect of the PR.Note: the "
ChangeLocalisationAfterAsyncLoad" test case inTestSceneTextFlowContainerLocalisation.csfails due to attempting to acquire parts before the container has calledLoadComplete().Video preview:
2026-07-16.20-16-06.mp4
2026-07-16.20-24-12.mp4
No AI was used at any point for this contribution.