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

Reverse Camera2D zoom #55731

Closed
wants to merge 1 commit into from
Closed

Reverse Camera2D zoom #55731

wants to merge 1 commit into from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Dec 8, 2021

Bugsquad edit: Implements and closes godotengine/godot-proposals#3888

This PR changes how zoom property of Camera2D works, so e.g. "zoom = 2" is now "zoom-in 2x", which is much more intuitive (previously it would zoom out).

Alternatively we could rename the property to view_rect_scale or similar, because that's what it actually does right now.

@KoBeWi KoBeWi added this to the 4.0 milestone Dec 8, 2021
@KoBeWi KoBeWi requested a review from a team as a code owner December 8, 2021 14:52
@raulsntos
Copy link
Member

The documentation should be updated too.

<member name="zoom" type="Vector2" setter="set_zoom" getter="get_zoom" default="Vector2(1, 1)">
The camera's zoom relative to the viewport. Values larger than [code]Vector2(1, 1)[/code] zoom out and smaller values zoom in. For an example, use [code]Vector2(0.5, 0.5)[/code] for a 2× zoom-in, and [code]Vector2(4, 4)[/code] for a 4× zoom-out.
</member>

@KoBeWi KoBeWi requested a review from a team as a code owner December 8, 2021 16:09
Point2 old_smoothed_camera_pos = smoothed_camera_pos;
_update_scroll();
smoothed_camera_pos = old_smoothed_camera_pos;
};

Vector2 Camera2D::get_zoom() const {
return zoom;
return Vector2(1, 1) / zoom;
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if zoom == Vector2(0, 0) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can't, unless you somehow set it outside setter.

Point2 old_smoothed_camera_pos = smoothed_camera_pos;
_update_scroll();
smoothed_camera_pos = old_smoothed_camera_pos;
};

Vector2 Camera2D::get_zoom() const {
return zoom;
return Vector2(1, 1) / zoom;
Copy link
Contributor

@madmiraal madmiraal Jan 29, 2022

Choose a reason for hiding this comment

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

Won't this cause a rounding error?
Vector2(1, 1) / (Vector2(1, 1) / zoom) does not always equal to zoom.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. You PR is probably better then.

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 13, 2022

Closing in favor of #57392

@KoBeWi KoBeWi closed this Mar 13, 2022
@KoBeWi KoBeWi deleted the mooz branch March 14, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invert Camera2D zoom values to make it intuitive
5 participants