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

Update example in Readme #336

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Update example in Readme #336

wants to merge 3 commits into from

Conversation

RCoder01
Copy link

Objective

The link in the current readme is broken and references an older version of the bevy 3d/3d_scene example.

Solution

This PR updates the readme example to be based on the new bevy example and fixes the link.

There may be a better way to rotate the cylinder collider than using Collider::compound that I'm not aware of, feel free to update this PR.

fix dead link and update to current 3d_scene example
@Jondolf Jondolf added the documentation Improvements or additions to documentation label Feb 23, 2024
Comment on lines +107 to +111
Collider::compound(vec![(
Position(Vec3::ZERO),
Rotation(Quat::from_rotation_x(-std::f32::consts::FRAC_PI_2)),
Collider::cylinder(0.002, 4.0),
)]),
Copy link
Owner

@Jondolf Jondolf Feb 23, 2024

Choose a reason for hiding this comment

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

This is meant to be an extremely basic example that demonstrates the basic API, so this shouldn't be using a compound collider and Position and Rotation here. As you mentioned, it's needed since you rotate the circle mesh, which also rotates this cylinder, but it makes the example too confusing for an introductory example.

I'd just use the old cuboid/plane here, for both the mesh and collider, since it's a lot simpler. As mentioned above this example, this is a modified version of Bevy's example, so we can change some things. We could also just remove the link to the example completely and have this be a standalone thing

commands.spawn((
RigidBody::Dynamic,
AngularVelocity(Vec3::new(2.5, 3.4, 1.6)),
Collider::cuboid(1.0, 1.0, 1.0),
PbrBundle {
mesh: meshes.add(Cuboid::default()),
material: materials.add(Color::rgb(0.8, 0.7, 0.6)),
mesh: meshes.add(Cuboid::new(1.0, 1.0, 1.0)),
Copy link
Owner

Choose a reason for hiding this comment

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

There's no need to change this imo

Suggested change
mesh: meshes.add(Cuboid::new(1.0, 1.0, 1.0)),
mesh: meshes.add(Cuboid::default()),

transform: Transform::from_xyz(0.0, 4.0, 0.0),
..default()
},
));

// Light
// light
Copy link
Owner

Choose a reason for hiding this comment

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

Nit, but I prefer to start comments with a capital letter, and have the extra newline between spawns, like what it was before. Bevy's own style guide says that comments should be capitalized, but it's contradicting itself in this example

Suggested change
// light
// Light

(this would of course apply to all other comments here too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants