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

Providing a better example for Mesh-building in docs. #8885

Merged
merged 7 commits into from
Jun 21, 2023
Merged

Providing a better example for Mesh-building in docs. #8885

merged 7 commits into from
Jun 21, 2023

Conversation

Adamkob12
Copy link
Contributor

Objective

  • Providing a "noob-friendly" example since not many people are proficient in 3D modeling / rendering concepts.

Solution

  • Adding more information to the example, with an explanation.

_Thanks to Nocta on discord for helping out when I didn't understand the subject well._

@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Rendering Drawing game state to the screen labels Jun 18, 2023
@Selene-Amanita
Copy link
Member

Selene-Amanita commented Jun 19, 2023

Code to test your mesh (because it's very easy to mess up vertex order, UVs, ... the best way to be sure is to test):

use bevy::{
    prelude::*, render::{mesh::Indices, render_resource::PrimitiveTopology},
};

fn main () {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup)
        .run();
}

fn setup(
    mut commands: Commands,
    assets: ResMut<AssetServer>,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
) {
    commands.spawn(Camera3dBundle {
        transform: Transform::from_xyz(0., 0., 5.).looking_at(Vec3::ZERO, Vec3::Y),
        ..default()
    });

    let mesh = meshes.add(create_triangle());
    let texture_image = assets.load("icon.png");
    let material = materials.add(StandardMaterial { 
        base_color_texture: Some(texture_image),
        ..default()
    });
    commands.spawn(PbrBundle {
        mesh,
        material,
        ..default()
    });

    commands.spawn(PointLightBundle {
        point_light: PointLight {
            color: Color::RED,
            intensity: 9000.0,
            ..default()
        },
        transform: Transform::from_xyz(0., 0., 5.).looking_at(Vec3::ZERO, Vec3::Y),
        ..default()
    });
}

// Copy your create_triangle function here

@Selene-Amanita Selene-Amanita self-requested a review June 19, 2023 22:57
@Adamkob12
Copy link
Contributor Author

made some improvements and tested the example. I am worried I made it too long, feel free to trim it down as you wish

@Adamkob12
Copy link
Contributor Author

Screen Shot 2023-06-20 at 4 04 22
(as intended)

@Selene-Amanita
Copy link
Member

made some improvements and tested the example. I am worried I made it too long, feel free to trim it down as you wish

I find it much better! I was thinking of stretching the sprite but that works too! Good job!

Put other reviews for potential improvements, but apart from the uppercase characters the rest looks good to me and is better than the current doc anyway.

I still think a visual and annotated image of what the mesh looks like would be helpful, but again not sure where to put the image (I think it is possible to add an image in rustdoc).

You can mark conversations where you fixed what was pointed out as "resolved".

@alice-i-cecile
Copy link
Member

@adamkobzan can you change the PR title to something more descriptive? These end up being used as commit messages in the final view (and for assembling patch notes), so it helps us out :)

@Selene-Amanita
Copy link
Member

Apart from minor formatting fixes, it looks good to me 👍

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Once the formatting and PR title are cleaned up this LGTM and I'll merge :)

@Adamkob12 Adamkob12 changed the title Minor improvement to docs Providing a better example for Mesh-building in docs. Jun 20, 2023
@Adamkob12
Copy link
Contributor Author

Sorry, when I tested with cargo doc that didnt show up

@Selene-Amanita Selene-Amanita added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 20, 2023
clearer triangle definition

Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 21, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 21, 2023
Merged via the queue into bevyengine:main with commit 284b6df Jun 21, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jul 31, 2023
# Objective

This PR continues #8885

It aims to improve the `Mesh` documentation in the following ways:
- Put everything at the "top level" instead of the "impl".
- Explain better what is a Mesh, how it can be created, and that it can
be edited.
- Explain it can be used with a `Material`, and mention
`StandardMaterial`, `PbrBundle`, `ColorMaterial`, and
`ColorMesh2dBundle` since those cover most cases
- Mention the glTF/Bevy vocabulary discrepancy for "Mesh"
- Add an image for the example
- Various nitpicky modifications

## Note

- The image I added is 90.3ko which I think is small enough?
- Since rustdoc doesn't allow cross-reference not in dependencies of a
subcrate [yet](rust-lang/rust#74481), I have a
lot of backtick references that are not links :(
- Since rustdoc doesn't allow linking to code in the crate (?) I put
link to github directly.
- Since rustdoc doesn't allow embed images in doc
[yet](rust-lang/rust#32104), maybe
[soon](rust-lang/rfcs#3397), I had to put only a
link to the image. I don't think it's worth adding
[embed_doc_image](https://docs.rs/embed-doc-image/latest/embed_doc_image/)
as a dependency for this.
cart pushed a commit that referenced this pull request Aug 10, 2023
# Objective

This PR continues #8885

It aims to improve the `Mesh` documentation in the following ways:
- Put everything at the "top level" instead of the "impl".
- Explain better what is a Mesh, how it can be created, and that it can
be edited.
- Explain it can be used with a `Material`, and mention
`StandardMaterial`, `PbrBundle`, `ColorMaterial`, and
`ColorMesh2dBundle` since those cover most cases
- Mention the glTF/Bevy vocabulary discrepancy for "Mesh"
- Add an image for the example
- Various nitpicky modifications

## Note

- The image I added is 90.3ko which I think is small enough?
- Since rustdoc doesn't allow cross-reference not in dependencies of a
subcrate [yet](rust-lang/rust#74481), I have a
lot of backtick references that are not links :(
- Since rustdoc doesn't allow linking to code in the crate (?) I put
link to github directly.
- Since rustdoc doesn't allow embed images in doc
[yet](rust-lang/rust#32104), maybe
[soon](rust-lang/rfcs#3397), I had to put only a
link to the image. I don't think it's worth adding
[embed_doc_image](https://docs.rs/embed-doc-image/latest/embed_doc_image/)
as a dependency for this.
dekirisu pushed a commit to dekirisu/bevy_gltf_trait that referenced this pull request Jul 7, 2024
# Objective

This PR continues bevyengine/bevy#8885

It aims to improve the `Mesh` documentation in the following ways:
- Put everything at the "top level" instead of the "impl".
- Explain better what is a Mesh, how it can be created, and that it can
be edited.
- Explain it can be used with a `Material`, and mention
`StandardMaterial`, `PbrBundle`, `ColorMaterial`, and
`ColorMesh2dBundle` since those cover most cases
- Mention the glTF/Bevy vocabulary discrepancy for "Mesh"
- Add an image for the example
- Various nitpicky modifications

## Note

- The image I added is 90.3ko which I think is small enough?
- Since rustdoc doesn't allow cross-reference not in dependencies of a
subcrate [yet](rust-lang/rust#74481), I have a
lot of backtick references that are not links :(
- Since rustdoc doesn't allow linking to code in the crate (?) I put
link to github directly.
- Since rustdoc doesn't allow embed images in doc
[yet](rust-lang/rust#32104), maybe
[soon](rust-lang/rfcs#3397), I had to put only a
link to the image. I don't think it's worth adding
[embed_doc_image](https://docs.rs/embed-doc-image/latest/embed_doc_image/)
as a dependency for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants