Skip to content

UI Slider Widget#6236

Closed
Pietrek14 wants to merge 142 commits intobevyengine:mainfrom
Pietrek14:slider
Closed

UI Slider Widget#6236
Pietrek14 wants to merge 142 commits intobevyengine:mainfrom
Pietrek14:slider

Conversation

@Pietrek14
Copy link
Contributor

Objective

Added a premade UI slider widget. Fixes #6196.

Solution

  • Added a RelativeCursorPosition component that updates alongside Interaction, which stores the cursor position relative to the node.
  • Added a Slider component that stores values specific to sliders
  • Added a SliderBundle and a SliderHandleBundle
  • Added update_slider_value and update_slider_handle systems
  • Made a slider example in the ui directory

Changelog

  • There is an easy way to create UI sliders now
  • If applicable, organize changes under "Added", "Changed", or "Fixed" sub-headings
  • Added:
    • RelativeCursorPosition component
    • Slider component
    • SliderBundle
    • SliderHandleBundle
    • update_slider_value and update_slider_handle systems
    • slider example

@mockersf
Copy link
Member

mockersf commented Oct 11, 2022

how the example looks like:

slider.mp4

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets labels Oct 11, 2022
@alice-i-cecile alice-i-cecile requested a review from Weibye October 11, 2022 12:47
Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

  1. I really like how simple this is! I think this is a solid step in the right direction and that we can make a lot more widgets in this manner.
  2. The PR needs doc-comments on all public structs and methods.
  3. If we can find a better approach to the slider-handle than adjusting the margin I would be very happy! :D

ShinySapphic and others added 25 commits December 17, 2022 22:04
…ine#6159)

# Objective

- Addresses bevyengine#6146 by allowing manual `Time` updating

## Solution
- Create `TimeUpdateStrategy` config resource
- Allow users to specify a manual `Instant/Duration` or leave as default (automatic)
- Get resource in `bevy_time::time_system`and update time with desired value
---

## Changelog

- Add `TimeUpdateStrategy` resource
- Update `bevy_time::time_system` to use optional manual values

Co-authored-by: BuildTools <unconfigured@null.spigotmc.org>
Co-authored-by: Lucidus115 <92978847+Lucidus115@users.noreply.github.com>
…6189)

I found myself doing
```rust
let child = commands.spawn(..).id();
commands.entity(parent).add_child(child);
```
When that could just be
```rust
commands.spawn(..).set_parent(parent);
```

Adding `set_parent` was trivial as it's just an `AddChild` command. Most of the changes are for `remove_parent`.
Also updated some outdated docs.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
# Objective

Fixes bevyengine#6339.

## Solution

This PR adds a new type, `GamepadInfo`, which holds metadata associated with a particular `Gamepad`. The `Gamepads` resource now holds a `HashMap<Gamepad, GamepadInfo>`. The `GamepadInfo` is created when the gamepad backend (by default `bevy_gilrs`) emits a "gamepad connected" event.

The `gamepad_viewer` example has been updated to showcase the new functionality.

Before:

![bevy-gamepad-old](https://user-images.githubusercontent.com/86984145/197359427-2130a3c0-bd8a-4683-ae24-2a9eaa98b586.png)

After:

![bevy-gamepad-new](https://user-images.githubusercontent.com/86984145/197359429-f7963163-df26-4906-af7f-6186fe3bd338.png)


---

## Changelog

### Added

- Added `GamepadInfo`.
- Added `Gamepads::name()`, which returns the name of the specified gamepad if it exists.

### Changed

- `GamepadEventType::Connected` is now a tuple variant with a single field of type `GamepadInfo`.
- Since `GamepadInfo` is not `Copy`, `GamepadEventType` is no longer `Copy`. The same is true of `GamepadEvent` and `GamepadEventRaw`.

## Migration Guide

- Pattern matches on `GamepadEventType::Connected` will need to be updated, as the form of the variant has changed.
- Code that requires `GamepadEvent`, `GamepadEventRaw` or `GamepadEventType` to be `Copy` will need to be updated.
# Objective

- Clipping (visible in the UI example with text scrolling) is funky 
- Fixes bevyengine#6287 

## Solution

- Fix UV calculation:
  - correct order for values (issue introduced in bevyengine#6000)

  - add the `y` values instead of subtracting them now that vertical order is reversed
  - take scale factor into account (bug already present before reversing the order)
- While around clipping, I changed clip to only mutate when changed

No more funkiness! 😞 

<img width="696" alt="Screenshot 2022-10-23 at 22 44 18" src="https://user-images.githubusercontent.com/8672791/197417721-30ad4150-5264-427f-ac82-e5265c1fb3a9.png">
…ns during serialization (bevyengine#6288)

# Objective

When running the scene example, you might notice we end up printing out the following:
```ron
// ...
{
  "scene::ComponentB": (
    value: "hello",
    _time_since_startup: (
      secs: 0,
      nanos: 0,
    ),
  ),
},
// ...
```

We should not be printing out `_time_since_startup` as the field is marked with `#[reflect(skip_serializing)]`:

```rust
#[derive(Component, Reflect)]
#[reflect(Component)]
struct ComponentB {
  pub value: String,
  #[reflect(skip_serializing)]
  pub _time_since_startup: Duration,
}
```

This is because when we create the `DynamicScene`, we end up calling `Reflect::clone_value`:

https://github.com/bevyengine/bevy/blob/82126697ee4f635cf6b22e0b9f25e5aca95fda4a/crates/bevy_scene/src/dynamic_scene_builder.rs#L114-L114

This results in non-Value types being cloned into Dynamic types, which means the `TypeId` returned from `reflected_value.type_id()` is not the same as the original component's. 

And this meant we were not able to locate the correct `TypeRegistration`.

## Solution

Use `TypeInfo::type_id()` instead of calling `Any::type_id()` on the value directly.

---

## Changelog

* Fix a bug introduced in `0.9.0-dev` where scenes disregarded component's type registrations
# Objective

Fixes bevyengine#6279.

## Solution

Added documentation explaining the meanings and default values of `PerspectiveProjection`'s fields.


Co-authored-by: dataphract <86984145+dataphract@users.noreply.github.com>
Signed-off-by: Lena Milizé <me@lvmn.org>

# Objective

Fixes bevyengine#6277.

## Solution

Adds `# Panics` section to [`fn insert_non_send_resource`](http://dev-docs.bevyengine.org/bevy/ecs/world/struct.World.html#method.insert_non_send_resource) documentation, which explains that it panics when called from thread other than main thread.
…ent` (bevyengine#6329)

# Objective

See title

## Before / After

<img width="988" alt="Screen Shot 2022-10-21 at 10 51 12 AM" src="https://user-images.githubusercontent.com/200550/197258517-29fec3e0-e272-4ab1-9f4c-c646b04876f2.png">
<img width="988" alt="Screen Shot 2022-10-21 at 10 51 24 AM" src="https://user-images.githubusercontent.com/200550/197258519-7fbaf058-fc2c-469e-ae34-5531f02a632f.png">

## Open questions

~~The old docs previously linked to a winit but that was preventing transparency for working on Windows 11. The recent winit upgrade should have fixed this.~~

~~I'm unable to test on Windows 11 though, so someone should verify that we no longer need to call this out as being broken.~~

edit: Seems like we're good on Windows 11, thanks.
# Objective

- Being able to set the `CompositeAlphaMode`

## Solution

- Expose it on `WindowDescriptor`, in the same way as `PresentMode` is exposed
…e#6255)

# Objective

- Avoids creating a `SurfaceConfiguration` for every window in every frame for the `prepare_windows` system
- As such also avoid calling `get_supported_formats` for every window in every frame

## Solution

- Construct `SurfaceConfiguration` lazyly in `prepare_windows`

---

This also changes the error message for failed initial surface configuration from "Failed to acquire next swapchain texture" to "Error configuring surface".
# Objective

- Improve bevyengine#3953

## Solution

- The very specific circumstances under which the render world is reset meant that the flush_as_invalid function could be replaced with one that had a noop as its init method.
- This removes a double-writing issue leading to greatly increased performance.

Running the reproduction code in the linked issue, this change nearly doubles the framerate.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective

- Fixes bevyengine#6355 

## Solution

- Add the removed local bool from bevyengine#6159
# Objective

Improve ergonomics by passing on the `IntoIterator` impl of the underlying type to wrapper types.

## Solution

Implement `IntoIterator` for ECS wrapper types (Mut, Local, Res, etc.).

Co-authored-by: devil-ira <justthecooldude@gmail.com>
…te the existing animation if it's already playing (bevyengine#6350)

# Objective

- You usually want to say that a given animation *should* be playing, doing nothing if it's already playing.

## Solution

- Rename play to start and add new play method that won't overwrite the existing animation if it's already playing bevyengine#6350

---

## Changelog

### Changed

`AnimationPlayer::play` will now not restart the animation if it's already playing

### Added

An `AnimationPlayer ::start` method, which has the old behavior of `play`

## Migration guide

- If you were using `play` to restart an animation that was already playing, that functionality has been moved to `start`. Now, `play` won't have any effect if the requested animation is already playing.
# Objective

Scenes are currently represented as a list of entities. This is all we need currently, but we may want to add more data to this format in the future (metadata, asset lists, etc.). 

It would be nice to update the format in preparation of possible future changes. Doing so now (i.e., before 0.9) could mean reduced[^1] breakage for things added in 0.10.

[^1]: Obviously, adding features runs the risk of breaking things regardless. But if all features added are for whatever reason optional or well-contained, then users should at least have an easier time updating.

## Solution

Made the scene root a struct rather than a list.

```rust
(
  entities: [
    // Entity data here...
  ]
)
```

---

## Changelog

* The scene format now puts the entity list in a newly added `entities` field, rather than having it be the root object

## Migration Guide

The scene file format now uses a struct as the root object rather than a list of entities. The list of entities is now found in the `entities` field of this struct.

```rust
// OLD
[
  (
    entity: 0,
    components: [
      // Components...
    ]
  ),
]

// NEW
(
  entities: [
    (
      entity: 0,
      components: [
        // Components...
      ]
    ),
  ]
)
```


Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective

Fixes bevyengine#5884 bevyengine#2879
Alternative to bevyengine#2988 bevyengine#5885 bevyengine#2886

"Immutable" Plugin settings are currently represented as normal ECS resources, which are read as part of plugin init. This presents a number of problems:

1. If a user inserts the plugin settings resource after the plugin is initialized, it will be silently ignored (and use the defaults instead)
2. Users can modify the plugin settings resource after the plugin has been initialized. This creates a false sense of control over settings that can no longer be changed.

(1) and (2) are especially problematic and confusing for the `WindowDescriptor` resource, but this is a general problem.

## Solution

Immutable Plugin settings now live on each Plugin struct (ex: `WindowPlugin`). PluginGroups have been reworked to support overriding plugin values. This also removes the need for the `add_plugins_with` api, as the `add_plugins` api can use the builder pattern directly. Settings that can be used at runtime continue to be represented as ECS resources.

Plugins are now configured like this:

```rust
app.add_plugin(AssetPlugin {
  watch_for_changes: true,
  ..default()
})
```

PluginGroups are now configured like this:

```rust
app.add_plugins(DefaultPlugins
  .set(AssetPlugin {
    watch_for_changes: true,
    ..default()
  })
)
```

This is an alternative to bevyengine#2988, which is similar. But I personally prefer this solution for a couple of reasons:
* ~~bevyengine#2988 doesn't solve (1)~~ bevyengine#2988 does solve (1) and will panic in that case. I was wrong!
* This PR directly ties plugin settings to Plugin types in a 1:1 relationship, rather than a loose "setup resource" <-> plugin coupling (where the setup resource is consumed by the first plugin that uses it).
* I'm not a huge fan of overloading the ECS resource concept and implementation for something that has very different use cases and constraints.

## Changelog

- PluginGroups can now be configured directly using the builder pattern. Individual plugin values can be overridden by using `plugin_group.set(SomePlugin {})`, which enables overriding default plugin values.  
- `WindowDescriptor` plugin settings have been moved to `WindowPlugin` and `AssetServerSettings` have been moved to `AssetPlugin`
- `app.add_plugins_with` has been replaced by using `add_plugins` with the builder pattern.

## Migration Guide

The `WindowDescriptor` settings have been moved from a resource to `WindowPlugin::window`:

```rust
// Old (Bevy 0.8)
app
  .insert_resource(WindowDescriptor {
    width: 400.0,
    ..default()
  })
  .add_plugins(DefaultPlugins)

// New (Bevy 0.9)
app.add_plugins(DefaultPlugins.set(WindowPlugin {
  window: WindowDescriptor {
    width: 400.0,
    ..default()
  },
  ..default()
}))
```


The `AssetServerSettings` resource has been removed in favor of direct `AssetPlugin` configuration:

```rust
// Old (Bevy 0.8)
app
  .insert_resource(AssetServerSettings {
    watch_for_changes: true,
    ..default()
  })
  .add_plugins(DefaultPlugins)

// New (Bevy 0.9)
app.add_plugins(DefaultPlugins.set(AssetPlugin {
  watch_for_changes: true,
  ..default()
}))
```

`add_plugins_with` has been replaced by `add_plugins` in combination with the builder pattern:

```rust
// Old (Bevy 0.8)
app.add_plugins_with(DefaultPlugins, |group| group.disable::<AssetPlugin>());

// New (Bevy 0.9)
app.add_plugins(DefaultPlugins.build().disable::<AssetPlugin>());
```
# Objective

- update deny config

## Solution

- update nix duplicate version to ignore
- update security advisories
# Objective

- `QueryCombinationIter` can have sizes greater than `usize::MAX`.
- Fixes bevyengine#5846 

## Solution

- Only the implementation of `ExactSizeIterator` has been removed. Instead of using `query_combination.len()`, you can use `query_combination.size_hint().0` to get the same value as before.

---

## Migration Guide

- Switch to using other methods of getting the length.
…ne#6260)

# Objective

I was trying to implement a collision system for my game, and believed that the iter_combinations method might be what I need. But I couldn't find a simple explanation of what a combination was in Bevy and thought it could use some more explanation. 

## Solution

I added some description to the documentation that can hopefully further elaborate on what a combination is. 

I also changed up the docs for the method because a combination is a different thing than a permutation but the Bevy docs seemed to use them interchangeably.
@Pietrek14
Copy link
Contributor Author

I tried fixing the merge conflicts, but I think I just broke it even more. Closing in favor of #7116 .

@Pietrek14 Pietrek14 closed this Jan 7, 2023
Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

ouch. Just now realized I reviewed the wrong PR. Oh well, just look at the comments and see if they apply on the new one.

Comment on lines +114 to +117
.add_system_to_stage(
CoreStage::Update,
widget::update_slider_value.label(widget::UpdateSliderValue),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the update_slider_value added to both update and post update? Is that necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I have no idea why I did that.

Comment on lines +127 to +130
.add_system_to_stage(
CoreStage::PostUpdate,
widget::update_slider_handle.after(widget::UpdateSliderValue),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

update_slider_handle is added twice to the PostUpdate stage, which I'm pretty sure is incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto


/// A UI node that is a slider
#[derive(Bundle, Clone, Debug, Default)]
pub struct SliderBundle {
Copy link
Contributor

Choose a reason for hiding this comment

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

Both these bundles and their related system should belong to a bevy_widget module and not bevy_ui, see the approach of #6517

Deserialize,
)]
#[reflect(Component, Serialize, Deserialize, PartialEq)]
pub struct RelativeCursorPosition(Vec2);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the value of this component when the cursor is not inside the bounds of the belonging node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just goes beyond the 0..1 range. I added a comment to clarify that.

min: f32,
max: f32,
step: f32,
value: f32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove min and max and instead keep value in the range 0.0..=1.0? That reduces the API surface and I think also allows us to get rid of the step field?

This is my plan for #6517 as #6620 was deemed not desired.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed min and max, but I don't see how that makes the step redundant?


let slider_width = slider_node.size().x - slider_handle_node.size().x;

slider_handle_style.position.left = Val::Px(
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Using position instead of margin here is the correct approach. 👍
  2. I assume only works when this node's position type is set to PositionType::Relative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried changing it to absolute in the example and it seems to be working correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A slider UI widget