[Merged by Bors] - Added TryFrom for VertexAttributeValues #1963

simensgreen · 2021-04-19T17:09:57Z

This implementations allows you
convert std::vec::Vec to VertexAttributeValues::T and back.

Examples

use std::convert::TryInto;
use bevy_render::mesh::VertexAttributeValues;

// creating vector of values
let before = vec![[0_u32; 4]; 10];
let values = VertexAttributeValues::from(before.clone());
let after: Vec<[u32; 4]> = values.try_into().unwrap();

assert_eq!(before, after);

alice-i-cecile

I like splitting this out into a conversions.rs file! And seems like helpful functionality with great tests.

simensgreen · 2021-04-19T17:21:46Z

I like splitting this out into a conversions.rs file! And seems like helpful functionality with great tests.

Thanks

crates/bevy_render/src/mesh/mesh/conversions.rs

inodentry · 2021-04-19T20:04:43Z

You should impl TryFrom instead of TryInto. The Rust std library provides TryInto impls for anything that impls TryFrom, but not the other way around.

simensgreen · 2021-04-20T09:06:39Z

You should impl TryFrom instead of TryInto. The Rust std library provides TryInto impls for anything that impls TryFrom, but not the other way around.

Yes, I completely agree, I will do it now, thank you

- `Short2` - `Short2Norm` - `Ushort2` - `Ushort2Norm` - `Short4` - `Short4Norm` - `Ushort4` - `Ushort4Norm` - `Char2` - `Char2Norm` - `Uchar2` - `Uchar2Norm` - `Char4` - `Char4Norm` - `Uchar4`

alice-i-cecile · 2021-04-20T13:08:51Z

Can you change the PR name? It'll get squashed down into a single commit with that name when we merge and it's now backwards :)

simensgreen · 2021-04-20T13:35:05Z

crates/bevy_render/src/mesh/mesh/conversions.rs

+            VertexAttributeValues::Uchar2Norm(..) => "VertexAttributeValues::Uchar2Norm",
+            VertexAttributeValues::Char4(..) => "VertexAttributeValues::Char4",
+            VertexAttributeValues::Char4Norm(..) => "VertexAttributeValues::Char4Norm",
+            VertexAttributeValues::Uchar4(..) => "VertexAttributeValues::Uchar4",
            VertexAttributeValues::Uchar4Norm(..) => "VertexAttributeValues::Uchar4Norm",


I have not added a _ placeholder so that whoever adds new formats will take care of conversions

crates/bevy_render/src/mesh/mesh/conversions.rs

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

crates/bevy_render/src/mesh/mesh/conversions.rs

Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>

cart · 2021-04-20T20:33:50Z

Looks good to me! I moved the tests into their own submodule to ensure imports aren't unused when building without tests.

I'd also like to remove the manual "type name strings" everywhere. But I understand that rust doesn't give us the tools we need to get enum variant names.

We could use a crate like strum to do this, but thats a bit too big for such a small need. I think I'll just whip up a quick derive macro (which we can then use and/or copy for bevy_reflect enum support).

For now, lets just merge this!

cart · 2021-04-20T20:33:55Z

bors r+

This implementations allows you convert std::vec::Vec<T> to VertexAttributeValues::T and back. # Examples ```rust use std::convert::TryInto; use bevy_render::mesh::VertexAttributeValues; // creating vector of values let before = vec![[0_u32; 4]; 10]; let values = VertexAttributeValues::from(before.clone()); let after: Vec<[u32; 4]> = values.try_into().unwrap(); assert_eq!(before, after); ``` Co-authored-by: aloucks <aloucks@cofront.net> Co-authored-by: simens_green <34134129+simensgreen@users.noreply.github.com> Co-authored-by: Carter Anderson <mcanders1@gmail.com>

bors · 2021-04-20T20:39:44Z

Build failed:

build (stable, ubuntu-latest)

cart · 2021-04-20T20:41:17Z

bors r+

This implementations allows you convert std::vec::Vec<T> to VertexAttributeValues::T and back. # Examples ```rust use std::convert::TryInto; use bevy_render::mesh::VertexAttributeValues; // creating vector of values let before = vec![[0_u32; 4]; 10]; let values = VertexAttributeValues::from(before.clone()); let after: Vec<[u32; 4]> = values.try_into().unwrap(); assert_eq!(before, after); ``` Co-authored-by: aloucks <aloucks@cofront.net> Co-authored-by: simens_green <34134129+simensgreen@users.noreply.github.com> Co-authored-by: Carter Anderson <mcanders1@gmail.com>

bors · 2021-04-20T21:04:00Z

Pull request successfully merged into main.

Build succeeded:

There are cases where we want an enum variant name. Right now the only way to do that with rust's std is to derive Debug, but this will also print out the variant's fields. This creates the unfortunate situation where we need to manually write out each variant's string name (ex: in #1963), which is both boilerplate-ey and error-prone. Crates such as `strum` exist for this reason, but it includes a lot of code and complexity that we don't need. This adds a dead-simple `EnumVariantMeta` derive that exposes `enum_variant_index` and `enum_variant_name` functions. This allows us to make cases like #1963 much cleaner (see the second commit). We might also be able to reuse this logic for `bevy_reflect` enum derives.

This implementations allows you convert std::vec::Vec<T> to VertexAttributeValues::T and back. # Examples ```rust use std::convert::TryInto; use bevy_render::mesh::VertexAttributeValues; // creating vector of values let before = vec![[0_u32; 4]; 10]; let values = VertexAttributeValues::from(before.clone()); let after: Vec<[u32; 4]> = values.try_into().unwrap(); assert_eq!(before, after); ``` Co-authored-by: aloucks <aloucks@cofront.net> Co-authored-by: simens_green <34134129+simensgreen@users.noreply.github.com> Co-authored-by: Carter Anderson <mcanders1@gmail.com>

There are cases where we want an enum variant name. Right now the only way to do that with rust's std is to derive Debug, but this will also print out the variant's fields. This creates the unfortunate situation where we need to manually write out each variant's string name (ex: in bevyengine#1963), which is both boilerplate-ey and error-prone. Crates such as `strum` exist for this reason, but it includes a lot of code and complexity that we don't need. This adds a dead-simple `EnumVariantMeta` derive that exposes `enum_variant_index` and `enum_variant_name` functions. This allows us to make cases like bevyengine#1963 much cleaner (see the second commit). We might also be able to reuse this logic for `bevy_reflect` enum derives.

Added TryInto for VertexAttributeValues

85f557e

alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-Rendering Drawing game state to the screen labels Apr 19, 2021

alice-i-cecile approved these changes Apr 19, 2021

View reviewed changes

jakobhellermann reviewed Apr 19, 2021

View reviewed changes

crates/bevy_render/src/mesh/mesh/conversions.rs Outdated Show resolved Hide resolved

simensgreen added 3 commits April 20, 2021 12:23

TryInto converted to TryFrom

ab1e5f7

Improve tests and docs

444495b

Informative error messages

e7df24f

simensgreen requested review from alice-i-cecile and jakobhellermann April 20, 2021 10:47

Add additional vertex formats (bevyengine#1878)

49438a6

- `Short2` - `Short2Norm` - `Ushort2` - `Ushort2Norm` - `Short4` - `Short4Norm` - `Ushort4` - `Ushort4Norm` - `Char2` - `Char2Norm` - `Uchar2` - `Uchar2Norm` - `Char4` - `Char4Norm` - `Uchar4`

New implements after adding new vertex formats

aafead9

simensgreen changed the title ~~Added TryInto for VertexAttributeValues~~ Added TryFrom for VertexAttributeValues Apr 20, 2021

simensgreen commented Apr 20, 2021

View reviewed changes

alice-i-cecile approved these changes Apr 20, 2021

View reviewed changes

crates/bevy_render/src/mesh/mesh/conversions.rs Outdated Show resolved Hide resolved

Сorrection of a mistake in a word

7fac3fe

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

jakobhellermann approved these changes Apr 20, 2021

View reviewed changes

MinerSebas approved these changes Apr 20, 2021

View reviewed changes

crates/bevy_render/src/mesh/mesh/conversions.rs Outdated Show resolved Hide resolved

cart and others added 2 commits April 20, 2021 13:29

fmt

bd993f8

Update crates/bevy_render/src/mesh/mesh/conversions.rs

a32ad7b

Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>

clippy

1074202

bors bot changed the title ~~Added TryFrom for VertexAttributeValues~~ [Merged by Bors] - Added TryFrom for VertexAttributeValues Apr 20, 2021

bors bot closed this Apr 20, 2021

cart mentioned this pull request Apr 20, 2021

[Merged by Bors] - EnumVariantMeta derive #1972

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Added TryFrom for VertexAttributeValues #1963

[Merged by Bors] - Added TryFrom for VertexAttributeValues #1963

simensgreen commented Apr 19, 2021

alice-i-cecile left a comment

simensgreen commented Apr 19, 2021

inodentry commented Apr 19, 2021

simensgreen commented Apr 20, 2021

alice-i-cecile commented Apr 20, 2021

simensgreen Apr 20, 2021

cart commented Apr 20, 2021

cart commented Apr 20, 2021

bors bot commented Apr 20, 2021

cart commented Apr 20, 2021

bors bot commented Apr 20, 2021

[Merged by Bors] - Added TryFrom for VertexAttributeValues #1963

[Merged by Bors] - Added TryFrom for VertexAttributeValues #1963

Conversation

simensgreen commented Apr 19, 2021

Examples

alice-i-cecile left a comment

Choose a reason for hiding this comment

simensgreen commented Apr 19, 2021

inodentry commented Apr 19, 2021

simensgreen commented Apr 20, 2021

alice-i-cecile commented Apr 20, 2021

simensgreen Apr 20, 2021

Choose a reason for hiding this comment

cart commented Apr 20, 2021

cart commented Apr 20, 2021

bors bot commented Apr 20, 2021

cart commented Apr 20, 2021

bors bot commented Apr 20, 2021