UmbraLuminosa / sickle_ui

A widget library built on top of bevy_ui.
Apache License 2.0
205 stars 24 forks source link

SetLabelTextExt is not good #25

Open ethereumdegen opened 1 month ago

ethereumdegen commented 1 month ago

SetLabelTextExt is not good !!!

it messes with the TextStyle of the label being edited which breaks stuff in my app !!

I do not like how it takes the TextStyle from the config and just applies it when ALL i want to do is change the text label. I am having to hack this up and make custom ext because of how annoying this is.



struct UpdateLabelText {
    text: String,
}

impl EntityCommand for UpdateLabelText {
    fn apply(self, entity: Entity, world: &mut World) {
        let Some(config) = world.get::<LabelConfig>(entity) else {
            warn!(
                "Failed to set label text on entity {:?}: No LabelConfig component found!",
                entity
            );

            return;
        };
        let style = config.text_style();
        let Some(mut text) = world.get_mut::<Text>(entity) else {
            warn!(
                "Failed to set label text on entity {:?}: No Text component found!",
                entity
            );

            return;
        };

        text.sections = vec![TextSection::new(self.text, style)];
    }
}

pub trait SetLabelTextExt {
    fn set_label_text(&mut self, text: impl Into<String>) -> &mut Self;
}

impl SetLabelTextExt for EntityCommands<'_> {
    fn set_label_text(&mut self, text: impl Into<String>) -> &mut Self {
        self.add(UpdateLabelText { text: text.into() });

        self
    }
}
ethereumdegen commented 1 month ago

Here is my updated version of this extension that works much much better for me personally


struct UpdateLabelText {
    label: String,
}

impl EntityCommand for UpdateLabelText {
    fn apply(self, entity: Entity, world: &mut World) {

        let Some(mut text) = world.get_mut::<Text>(entity) else {
            warn!(
                "Failed to set label text on entity {:?}: No Text component found!",
                entity
            );

            return;
        };

            text.sections[0].value = self.label.clone() ; 

    }
}

pub trait SetLabelTextCustomExt {
    fn set_label_text_custom(&mut self, text: impl Into<String>) -> &mut Self;
}

impl SetLabelTextCustomExt for EntityCommands<'_> {
    fn set_label_text_custom(&mut self, label: impl Into<String>) -> &mut Self {
        self.add(UpdateLabelText { label: label.into() });

        self
    }
}

I think if you want to have a fn that sets text style, make it a separate EXT and DEFINITELY do not assume i want to overwrite the style with whatever is in the config

eidloi commented 1 month ago

I feel you mate, there are two other extensions on the entity commands to replace the text (with a style provided) and one to replace with sections provided. You can find these under the ui_commands.rs.

Just yesterday I considered adding / replacing extensions that do not touch the style as it is also an issue for the theming. However the issue is that currently bevy ties the style to a section of the text which has it's ups and downs. Normally, I could just use the first section and replace the text there (labels don't create extra sections), but this would be a happy hack.

So (again, yesterday) I opted to not touch these for the moment as I intend to create my own text rendering up next.

Also, creating your own extension is perfectly fine and I think it is the correct way of developing these kinds of interfaces when working with bevy. Once I have a good solution for you I'll post it in this issue.

UkoeHB commented 1 month ago

Also, creating your own extension is perfectly fine and I think it is the correct way of developing these kinds of interfaces when working with bevy.

I wrote several extensions in bevy_cobweb_ui, it is definitely a big part of using sickle.

ethereumdegen commented 1 month ago

hmm... see like this is the messy way im making a label now

  let mut container_builder = commands .ui_builder(container_node);

    let mut current_region_label = 
     container_builder.label(LabelConfig {
                label: "[current_region]".to_string(), 
                ..default()
            } )  ;

    current_region_label.insert( DynamicTextDataSource::new ( Some( Box::new(  PlayerCurrentMapRegionName   ))  ) ) ;

     current_region_label.style()
      .padding(UiRect::new(Val::Px(8.0), Val::Px(8.0), Val::Px(16.0), Val::Px(16.0))  );

     current_region_label.entity_commands().ui_font(  
                    region_label_font,
                    32.0,
                    Color::rgb(0.271, 0.378, 0.453),
      ) ;

Im having to set the font in a custom EXT (.ui_font) and i cant chain anything together now because i cant simply extend style() at all so i cant chain to it. and i cant add any custom functions to style() either.

ethereumdegen commented 1 month ago

Maybe if i could turn style() chain back into a ui_builder to keep chaining that would be nice? or if i could somehow create custom functions for style() that would be great too

ethereumdegen commented 1 month ago

But since i can only create custom functions that operate on a EntityCommands its pushing me down this path...

eidloi commented 1 month ago

I am not sure what prevents you from adding an extension to UiStyle? These are designed to be extended so you should be able to. I am doing these a lot on the theming branch, for instance:

pub trait SetImageExt<'a> {
    fn image(&'a mut self, source: ImageSource) -> &mut UiStyle<'a>;
}

impl<'a> SetImageExt<'a> for UiStyle<'a> {
    fn image(&'a mut self, source: ImageSource) -> &mut UiStyle<'a> {
        self.commands.add(SetImage {
            source,
            check_lock: true,
        });
        self
    }
}

And then SetImage is a struct with the two fields and an impl EntityCommand for SetImage

ethereumdegen commented 1 month ago

huh well i tried this

pub trait SetUiFontExt<'a> {
    fn ui_font(    &'a mut self,
        font: impl Into<String>,
        size: f32,
        color: Color ) -> &mut UiStyle<'a>;
}

impl<'a> SetUiFontExt<'a> for UiStyle<'a> {
    fn ui_font(    &'a mut self,
        font: impl Into<String>,
        size: f32,
        color: Color, ) -> &mut UiStyle<'a> {

        self.commands.add( SetFont(font.into(), size, color)  );
        self
    }
}

and it says the field commands is private

eidloi commented 1 month ago

aaaaaaaarrrggghh damn, it is on main. I need a couple of days to wrap up the theming enough, but this will be public with the next update if that works for you?

ethereumdegen commented 1 month ago

that works for me