-
-
Notifications
You must be signed in to change notification settings - Fork 21k
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
Improve the default size of 3D shapes and meshes (Box, Capsule, and Cylinder) #56365
Conversation
0b6cb3b
to
3044169
Compare
I agree with the new Box defaults, but I don't really like the cylinder/capsule ones. I would rather have a 0.5 default radius and a 1.0 default height. So they follow the same rationale of "a scale of 7 means it's 7 meters wide" that is mentioned in the proposal. In any case, there's never going to be a perfect default value, but if we are going to change any defaults, it should be done before alpha. |
3044169
to
ae6fa9d
Compare
ae6fa9d
to
1c0a5cc
Compare
After discussion in a PR meeting we suggesting using:
|
@@ -239,7 +239,7 @@ class CSGBox3D : public CSGPrimitive3D { | |||
virtual CSGBrush *_build_brush() override; | |||
|
|||
Ref<Material> material; | |||
Vector3 size = Vector3(2, 2, 2); | |||
Vector3 size = Vector3(1, 1, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as it follows the style guide,
Vector3 size = Vector3(1, 1, 1); | |
Vector3 size(1, 1, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually a style that we don't really use in the codebase so far, so it's better not to start using it as it adds inconsistency in the codebase.
1c0a5cc
to
8cfd264
Compare
This PR has been updated so that the capsule and cylinder defaults are now 0.5 radius and 2.0 height. While I still think 0.25 radius and 1.75 height is better, 0.5 radius and 2.0 height is still very good and it has consensus. |
Thanks! |
It seems SphereMesh is now twice as large as BoxMesh by default. Should we make spheres smaller to match BoxMesh again? The same applies to CSGSphere3D. |
@Calinou I'm not sure. On one hand, that makes sense. On the other hand, it's simpler to have the sphere be a radius of EDIT: Also, a radius of 0.5 would be more consistent with the ends of the new capsule size. So perhaps it's better. |
Implements and closes godotengine/godot-proposals#3732
The specific values are up for discussion in the above proposal, but I think what's currently in this PR is quite good.
I don't really have an opinion on the CSGTorus3D changes, they were copied from https://github.com/godotengine/godot/pull/49729/files#diff-a81f2e3a53dfacfdb0037de9538db28d5646ae0c3a11a52eca1caa5d665dfd1cR1676-R1678
Breaks compat, but we have already broken compatibility with these shapes for 4.0 so changing this is not a big deal.