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.

Sign up for GitHub

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

Jump to bottom

Text Rework cleanup #15887

Merged
merged 12 commits into from
Oct 15, 2024
Merged

Text Rework cleanup #15887

merged 12 commits into from
Oct 15, 2024

Conversation

MiniaczQ
Copy link
Contributor

@MiniaczQ MiniaczQ commented Oct 13, 2024 •
edited
Loading

Objective

Cleanup naming and docs, add missing migration guide after #15591

All text root nodes now use Text (UI) / Text2d.
All text readers/writers use Text<Type>Reader/Text<Type>Writer convention.

Migration Guide

Doubles as #15591 migration guide.

Text bundles (TextBundle and Text2dBundle) were removed in favor of Text and Text2d.
Shared configuration fields were replaced with TextLayout, TextFont and TextColor components.
Just TextBundle's additional field turned into TextNodeFlags component,
while Text2dBundle's additional fields turned into TextBounds and Anchor components.

Text sections were removed in favor of hierarchy-based approach.
For root text entities with Text or Text2d components, child entities with TextSpan will act as additional text sections.
To still access text spans by index, use the new TextUiReader, Text2dReader and TextUiWriter, Text2dWriter system parameters.

@MiniaczQ MiniaczQ added D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Text Rendering and layout for characters labels Oct 13, 2024
@MiniaczQ MiniaczQ removed the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Oct 13, 2024
@MiniaczQ MiniaczQ changed the title [Text Rework](#15591) cleanup Text Rework cleanup Oct 13, 2024
@MiniaczQ MiniaczQ marked this pull request as ready for review October 13, 2024 19:27
@MiniaczQ MiniaczQ added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Oct 13, 2024
@MiniaczQ
Copy link
Contributor Author

I'm also thinking whether this should reintroduce deprecated Text2dBundle and TextBundle so people get a deprecation message with migration instructions
@alice-i-cecile

@MiniaczQ MiniaczQ removed the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Oct 13, 2024
@alice-i-cecile
Copy link
Member

I really like most of these changes :) If we revert the TextUi change IMO it's uncontroversial.

@JMS55 JMS55 added this to the 0.15 milestone Oct 13, 2024
@alice-i-cecile
Copy link
Member

I'm also thinking whether this should reintroduce deprecated Text2dBundle and TextBundle so people get a deprecation message with migration instructions @alice-i-cecile

Yes please!

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 14, 2024
@MiniaczQ MiniaczQ added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 14, 2024
@MiniaczQ MiniaczQ added the X-Uncontroversial This work is generally agreed upon label Oct 14, 2024
ickshonpe
ickshonpe suggested changes Oct 14, 2024
View reviewed changes
crates/bevy_ui/src/lib.rs Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 14, 2024
ickshonpe
ickshonpe approved these changes Oct 14, 2024
View reviewed changes
Copy link
Contributor

@ickshonpe ickshonpe left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. I'd prefer if TextUiReader/Writer was just called TextReader/Writer but then we'd need to rename the underlying type. It's not worth fussing over for now.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 15, 2024
Merged via the queue into bevyengine:main with commit f602eda Oct 15, 2024
28 of 29 checks passed
@alice-i-cecile alice-i-cecile mentioned this pull request Oct 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@ickshonpe ickshonpe ickshonpe approved these changes

@UkoeHB UkoeHB UkoeHB approved these changes

@BenjaminBrienen BenjaminBrienen BenjaminBrienen approved these changes

Assignees
No one assigned
Labels
A-Text Rendering and layout for characters D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
0.15
Development

Successfully merging this pull request may close these issues.
6 participants
@MiniaczQ @alice-i-cecile @ickshonpe @UkoeHB @BenjaminBrienen @JMS55