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

Inconsistent resolution/segments naming #13412

Closed
lynn-lumen opened this issue May 17, 2024 · 1 comment · Fixed by #13438
Closed

Inconsistent resolution/segments naming #13412

lynn-lumen opened this issue May 17, 2024 · 1 comment · Fixed by #13438
Labels
A-Gizmos Visual editor and debug gizmos A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation

Comments

@lynn-lumen
Copy link
Contributor

lynn-lumen commented May 17, 2024

bevy_gizmos uses the term segments to denote the amount of vertices/lines to draw for curved shapes whereas
bevy_render uses the term resolution for this purpose and denotes the amount of "cuts" along the height of shapes like Cylinders with segments.

This is kind of confusing when working with both gizmos and meshes.

let cylinder = Cylinder::default();
let mesh = cylinder.mesh().segments(5).build(); // This is a cylinder with 32 vertices per top/bottom cap
gizmos.primitive_3d(cylinder, ...).segments(5); // This is a cylinder with 5 vertices per top/bottom cap

I would prefer keeping the naming conventions of bevy_render since this would just introduce a breaking change (replacing all segments methods on gizmos with resolution) whereas keeping the naming convention of bevy_gizmos would effectively flip the meaning of segments and resolution in bevy_render and introduce a lot of silent bugs for users.

@lynn-lumen lynn-lumen added C-Docs An addition or correction to our documentation S-Needs-Triage This issue needs to be labelled labels May 17, 2024
@pablo-lua pablo-lua added A-Rendering Drawing game state to the screen A-Gizmos Visual editor and debug gizmos and removed S-Needs-Triage This issue needs to be labelled labels May 18, 2024
@mweatherley
Copy link
Contributor

I agree that this is confusing; I want to also mention that in changing this, we should keep in mind that the mesh builders are also likely to have another parameter for subdivisions in the future (e.g. making a rectangle out of many triangles instead of just two).

github-merge-queue bot pushed a commit that referenced this issue May 21, 2024
# Objective

- Fixes #13412

## Solution

- Renamed `segments` in `bevy_gizmos` to `resolution` and adjusted
examples

## Migration Guide

- When working with gizmos, replace all calls to `.segments(...)` with
`.resolution(...)`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Gizmos Visual editor and debug gizmos A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants