add `FontRef` and `DefaultFont` by IceSentry · Pull Request #5549 · bevyengine/bevy

IceSentry · 2022-08-02T03:45:48Z

Objective

Creating a TextStyle requires an Handle which means you either need to pass the text_style around or pass an asset_server everywhere. This makes declaring reusable ui components harder than necessary.

It should be possible to not need to pass around anything and create it where we want it and let bevy take care of loading the font.

Solution

Introduce a new FontRef inspired by the ShaderRef from the new Material
- A default font that can be configured globally with the new DefaultFont resource
- A string path to a font can be automatically converted to a FontRef and the new bevy_text::load_font() system will take care of loading the font asset and converting the path to an Handle.
- Keep support for manually loading fonts and convert the Handle to a FontRef::Handle

Changelog

Added FontRef
Added DefaultFont
TextStyle now uses FontRef instead of Handle

Migration Guide

Instead of requiring an asset_server to load a font you can now use a string path to a font asset and bevy will take care of loading it properly. This makes declaring TextStyle components much easier. If you want to keep the asset_server you can easily convert a Handle to a FontRed using .into().

// 0.8
commands.spawn_bundle(TextBundle::from_section(
    "hello bevy!",
    TextStyle {
        font: asset_server.load("fonts/FiraSans-Bold.ttf"),
		..default()
    },
));

// 0.9
commands.spawn_bundle(TextBundle::from_section(
    "hello bevy!",
    TextStyle {
        font: "fonts/FiraSans-Bold.ttf".into(),
		..default()
    },
));

Notes

I updated all the examples using fonts, but I only used the new DefaultFont on the game_menu example.
If anyone is scared of the amount of files changed, the vast majority of the changes in most files are just converting existing asset_server.load() to the new FontRef.

Closes #5515

mockersf · 2022-08-02T07:58:33Z

If the DefaultFont resource has not been set, would it make sense to pick the first font available in Assets and only fail if there isn't one?

IceSentry · 2022-08-02T13:55:57Z

That could work, but this new api encourages people to just use "path/to/font.ttf".into() everywhere which would mean it's very possible a font won't be loaded by the time it reaches a FontRef::Default.

Even if it did, it would be a bit weird how order dependent it would become. Like you change the order of a ui widget and now suddenly all the text in your app looks different.

IceSentry · 2022-08-02T13:58:02Z

I don't mind doing it as a fallback, but personally I'd rather get an error if it's not set. Eventually, once we ship an editor we'll be able to ship a font and initialize the resource with a default value, but right now it feels less error prone to not do that. Eventually we could also load the system font, but that should be done in a separate PR.

IceSentry · 2022-08-06T03:38:05Z

Alright, I implemented the fallback to the first font on default, but as I expected if the fonts are added in the same system it doesn't work because the font isn't added to the assets store yet.

I also fixed a bug where text wouldn't load on startup in desktop_app mode.

Weibye

I really like this small but useful ux improvement, thanks!

examples/games/game_menu.rs

Weibye

Changes look really good but we should add a note in the changelog explaining the change and addition of the DefaultFont resource

IceSentry · 2022-08-29T02:20:12Z

I added a short description to the migration guide. I'm not sure what would be needed in the changelog section.

Weibye

This looks good to me now, for some reason I had the recollection that we usually add things to the CHANGELOG.md but it seems we only do that when we do releases. In that case what is currently in the PR description should be enough.

Weibye · 2022-08-29T16:06:49Z

crates/bevy_text/src/lib.rs

                update_text2d_layout.after(ModifiesWindows),
-            );
+            )
+            .add_system(load_font);


Are there any system-ordering issues we should be careful about here?

Not really, I guess it could make sense to run it in PostUpdate so it can run in the same frame as when the Text was added but it will work either way

cart · 2022-08-30T22:08:54Z

crates/bevy_text/src/text.rs

+                }
+                bevy_asset::LoadState::Failed => panic!("Failed to load font {:?}", path),
+            };
+            section.style.font = FontRef::Handle(handle);


Not a huge fan of this dataflow. A style type should be "declarative". The user sets a value and the underlying system makes it happen under the hood, without changing the users' declared style.

Setting style.font = FontRef::Default should hold that default indefinitely, not serve as a placeholder/queued action that gets overwritten in place. Same goes for FontRef::Path.

I understand why you're doing this (we need to hold a strong handle somewhere to keep the font alive). But this weird dataflow isn't worth it from my perspective.

I agree, I'm just honestly not sure how else I could do that. I'll try to think of something better.

cart · 2022-08-30T22:09:38Z

crates/bevy_text/src/text.rs

+/// If the font has already been loaded then it simply converts the [`FontRef::Path`] to a [`FontRef::Handle`]
+/// Otherwise it tries to load the font with the [`AssetServer`]
+pub fn load_font(
+    mut query: Query<&mut Text, Added<Text>>,


This will only run for added text. FontRef should would consistently, even if changed at runtime.

Ah, right, I need to handle situations where fonts are updated, I didn't think about that one.

IceSentry · 2023-04-20T17:19:14Z

I closed this PR because the solution in #8445 solves this in a much nicer way.

IceSentry added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Aug 2, 2022

IceSentry changed the title ~~add FontRef~~ add FontRef and DefaultFont Aug 2, 2022

IceSentry force-pushed the fontref branch from a8224e3 to 4e19210 Compare August 2, 2022 04:23

alice-i-cecile added the X-Needs-SME This type of work requires an SME to approve it. label Aug 2, 2022

IceSentry force-pushed the fontref branch from ff3a24b to 42d735d Compare August 6, 2022 01:53

Weibye reviewed Aug 6, 2022

View reviewed changes

examples/games/game_menu.rs Outdated Show resolved Hide resolved

IceSentry force-pushed the fontref branch from 9f40573 to 5646239 Compare August 11, 2022 22:42

IceSentry force-pushed the fontref branch from 4ee0c71 to 2e9726c Compare August 20, 2022 16:08

Weibye suggested changes Aug 27, 2022

View reviewed changes

Weibye added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Aug 27, 2022

Weibye approved these changes Aug 29, 2022

View reviewed changes

IceSentry added 10 commits August 30, 2022 17:07

add FontRef

bf8b2db

use DefaultFont in text example

d896922

clippy

479df35

doc check

8828453

fix reflect

0f5fea5

default fallback to first font

d00a600

fix loading in desktop_app mode

2d3331a

impl Resouce for default font

387ba67

export DefaultFont in prelude

bd1f1b1

only send a single request redraw

62c9d02

IceSentry force-pushed the fontref branch 2 times, most recently from 1f1c939 to 5e5efec Compare August 30, 2022 22:04

fix build

660f948

cart reviewed Aug 30, 2022

View reviewed changes

IceSentry force-pushed the fontref branch from 5e5efec to 660f948 Compare August 30, 2022 22:13

nicopap mentioned this pull request Apr 20, 2023

add a default font #8445

Merged

IceSentry closed this Apr 20, 2023

Uh oh!

Conversation

IceSentry commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Changelog

Migration Guide

Notes

Uh oh!

mockersf commented Aug 2, 2022

Uh oh!

IceSentry commented Aug 2, 2022

Uh oh!

IceSentry commented Aug 2, 2022

Uh oh!

IceSentry commented Aug 6, 2022

Uh oh!

Weibye left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Weibye left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IceSentry commented Aug 29, 2022

Uh oh!

Weibye left a comment

Choose a reason for hiding this comment

Uh oh!

Weibye Aug 29, 2022

Choose a reason for hiding this comment

Uh oh!

IceSentry Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

cart Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

IceSentry Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

cart Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

IceSentry Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

IceSentry commented Apr 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

IceSentry commented Aug 2, 2022 •

edited

Loading

Weibye left a comment •

edited

Loading