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

feat: make fitToBox() abort itself when it is given an empty box #143

Merged
merged 1 commit into from
Oct 10, 2020

Conversation

0b5vr
Copy link
Collaborator

@0b5vr 0b5vr commented Oct 9, 2020

It seems doing fitToBox against empty boxes completely messes its internal state up since v1.20.x.
First of all I don't think people wants to do fitToBox against empty boxes.

@0b5vr 0b5vr requested a review from yomotsu October 9, 2020 13:21
@0b5vr 0b5vr self-assigned this Oct 9, 2020
Comment on lines +979 to +980
console.warn( 'camera-controls: fitTo() cannot be used with an empty box. Aborting' );
return;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I chose warn + early abort. Does it look good for you?

@yomotsu
Copy link
Owner

yomotsu commented Oct 10, 2020

Thanks for your investigation.

First of all I don't think people want to do fitToBox against empty boxes.

That's a good point. Now that I think of it, it could happen with default box3.

Copy link
Owner

@yomotsu yomotsu left a comment

Choose a reason for hiding this comment

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

Approved

@yomotsu
Copy link
Owner

yomotsu commented Oct 10, 2020

merged! thanks.
I'll add this to 0.25.1 and publish this weekend.

@yomotsu yomotsu merged commit 7be151d into dev Oct 10, 2020
@0b5vr 0b5vr deleted the fittobox-abort-empty branch October 11, 2020 23:55
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