Skip to content

Conversation

@metalflow
Copy link

This is a pretty gross initial edit I dashed out in about an hour. I am not proud of the list of If statements in update_orbit_transform, but its what made sense and seemed easiest to debug when I was writing. The list of If statements could probably be replaced with a match statement that returns the appropriate boolean. Also, it might be nicer to just have the update return the new_transform and then do these comparisons around lib.rs line 825 or so. I wasn't sure how I would go about testing these edits, so they are completely untested at this time.

…bit_transform could probably be better replaced witha match statement.
Copy link
Owner

@Plonq Plonq left a comment

Choose a reason for hiding this comment

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

Thanks for PR. Initial review, haven't tested it yet.

Comment on lines +288 to +299
/// Adding maximum x bound for the camera transform. Default value of None has no bound.
pub camera_transform_x_max: Option<f32>,
/// Adding minimum x bound for the camera transform. Default value of None has no bound.
pub camera_transform_x_min: Option<f32>,
/// Adding maximum y bound for the camera transform. Default value of None has no bound.
pub camera_transform_y_max: Option<f32>,
/// Adding minimum y bound for the camera transform. Default value of None has no bound.
pub camera_transform_y_min: Option<f32>,
/// Adding maximum z bound for the camera transform. Default value of None has no bound.
pub camera_transform_z_max: Option<f32>,
/// Adding minimum z bound for the camera transform. Default value of None has no bound.
pub camera_transform_z_min: Option<f32>,
Copy link
Owner

Choose a reason for hiding this comment

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

Is there any reason not to use a single Cuboid? You could potentially use closest_point() to do the constraining.

new_transform.rotation *= yaw_rot * pitch_rot;
new_transform.translation += focus + new_transform.rotation * Vec3::new(0.0, 0.0, radius);
if (x_max.is_some()) && (new_transform.translation.x > x_max.unwrap()) {
return false;
Copy link
Owner

Choose a reason for hiding this comment

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

Returning bool seems unnecessary. Just clamp/constrain it - if the values end up being the same as before, it doesn't matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants