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.

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

Migrate to bevy 0.11 #65

Merged
merged 35 commits into from
Sep 20, 2023
Merged

Migrate to bevy 0.11 #65

merged 35 commits into from
Sep 20, 2023

Conversation

jkb0o
Copy link
Owner

@jkb0o jkb0o commented Aug 4, 2023

About

This is migration to bevy 0.11 PR.

A lot of work was done by @PhaestusFox in #63, and this PR is logical continuation of his work. If you want to contribute to bevy-011 version, please make pr to this branch.

You can test this by adding belly to deps:

# bash
cargo add --git https://github.com/jkb0o/belly.git --branch bevy-0.11 belly
# Cargo.toml
[dependencies]
belly = { git = "https://github.com/jkb0o/belly.git", branch = "bevy-0.11", version = "0.2.0" }

Tracker

  • Migrate most of the code (update to 0.11 #63)
  • Fix runtime crashes (11858da & 1c53d16)
  • Proper organise systems ordering according to new bevy schedules (ed69540)
  • Fix text input selection flickering (2029df8)
  • Fix relations systems CPU leaking (37d0adb)
  • Fix stylebox rendering (7e17ac4)
  • Adjust examples to the new version (7c258b2 & 59321a4)
  • Get rid or deprecate undefined style properties in favour of removed Val::Undefined
  • Implement new column & row gap style properties (91e1d58)
  • Remove line breaks (337a535)
  • Implement grid properties (2e213e8 & 87c6541)
  • Add CHANGELOG.md
  • Update README.md
  • Update props & widgets docs
  • Bump version to 0.3

Known bugs:

this is the best I can do to bring this to bevy 0.11, I hade to cut overflow since it is a struct now and I don't know how to convert. also the mouse position is inverted the top is the bottom and vice virce, don't know where to look to fix that one
not quite sure why vscode is not showing some errors
I still have questions about some of the other comments, like removing undefined outright, I like that ess is close to css and should keep it unless it is impossible to represent with Bevy, and if it is removed should also only be depreciated otherwise the parser fails and brakes shit in a unnecessary way, just emit a warning till bevy 0.12?
it does not need to be mut
@jkb0o jkb0o mentioned this pull request Aug 4, 2023
Copy link
Collaborator

@baitcode baitcode left a comment

Choose a reason for hiding this comment

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

Disregard my comments on Undefined. I've read PR description. And it's cool.

this is actually 4 PR merged in one

  1. update to bevy 0.11
  2. step 1 in removal of undefined var
  3. style refactoring (min_width and width props moved e.t.c.)
  4. update examples
    would be easier to maintain and review.

Besides that, great job. Hopefully I'll be able to use this in my game after merge.

@@ -412,6 +412,7 @@ pub trait Widget {
ctx.insert(policy);
ctx.insert(Interaction::default());
}
ctx.insert(Name::new(self.name().as_str()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if Name is already there?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth adding some prefix so it would be obvious where the name comes from. like "belly_" maybe?

@@ -35,7 +35,7 @@ style_property! {
}

compound_style_property! {
#[doc = " Specify element position by providing values to `Style.position`:"]
#[doc = " Specify element position by providing values to `Style.top` `Style.left` `Style.bottom` `Style.right`:"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -55,6 +55,35 @@ impl PropertyParser<Val> for ValParser {
}
}

pub fn overflow(prop: &StyleProperty) -> Result<Overflow, ElementsError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

🥇

@@ -90,7 +90,8 @@ impl StylePropertyToken {
StylePropertyToken::Percentage(p) => Ok(Val::Percent(p.to_float())),
StylePropertyToken::Dimension(d, u) if u == "px" => Ok(Val::Px(d.to_float())),
StylePropertyToken::Identifier(i) if i == "auto" => Ok(Val::Auto),
StylePropertyToken::Identifier(i) if i == "undefined" => Ok(Val::Undefined),
StylePropertyToken::Identifier(i) if i == "undefined" => Ok(Val::Px(0.)),
StylePropertyToken::Identifier(i) if i == "undefined" => Ok(Val::Px(0.)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually this seems to be like an implicit rule. Maybe use 0 explicitly and deprecate undefined value.

let watchers = self.watchers.write().unwrap();
if watchers.contains(&watcher) {
return;
if !self.watchers.read().unwrap().contains(&watcher) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If two watches would contain two watchers, which watch would watch which watcher

Copy link
Collaborator

Choose a reason for hiding this comment

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

previous version with early returns was nicer. but yeah, does not work the same.

@@ -4,8 +4,9 @@ use belly_macro::*;
use bevy::{
asset::Asset,
prelude::*,
utils::{HashMap, HashSet},
// utils::{HashMap, HashSet},
Copy link
Collaborator

Choose a reason for hiding this comment

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

yack

@@ -99,7 +99,7 @@ stylebox-source: "images/stylebox.png"
<!-- @property-type=$val -->
Size type representing `bevy::prelude::Val` type. Possible values:
- `auto` for `Val::Auto`
- `undefined` for `Val::Undefined`
- `undefined` for `Val::Px(0.)`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is kinda weird, right?

Choose a reason for hiding this comment

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

Yeah, if you need an undefined defined value, then it should probably map to Val::Auto.

.in_base_set(CoreSet::PreUpdate)
.in_set(Label::Signals)
.after(InputSystem),
.add_systems(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add comments on what is happening here? This is the most complex place in whole PR and seems like it's a very important, it defines input processing pipeline, I can't comprehend.

@jkb0o jkb0o merged commit ddd29fc into main Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants