Improve usability of Camera2D

[Lazy-Rabbit-2001]

Implements and closes: godotengine/godot-proposals#11527

Changes:

* Now the default limit will not be +/-1000000, instead, the top-left will be (0, 0) while the bottom-right will become (viewport_w, viewport_h)

• Added limit_enabled (true by default), disabling which will allow the camera focus to move anywhere.
  * Added a button Snap Limit to Viewport, on which you click will snap the top-left limit to the global position of the camera and the bottom-right limit to the global position plus the size of the viewport. (Will be implemented in a continuation in the future)
• Added a editor_draw_limits_color, which allows you to change the color of the limit rect.
• Now you can drag the dragger to resize and offset the limit rectangle, and thus to change the limit_* variables.

Issues

• Currently, scaling the camera will also scale the rect, which is easy to be fixed theoretically but hard to fix technically (?)

[timoschwarzer]

While I like the idea of making editing the camera limits a more interactive experience, I very much dislike the change of the default limit. I didn't try it out, but wouldn't this effectively make a camera that gets added to a node non-functional (as in, it doesn't move in any direction) unless you enlarge the limit rectangle? This would be a bad user experience, and lead to lots of confusion.

Edit: This is also why the unit tests fail.

Instead, I propose a Boolean property on the camera to enable/disable the limits, and have it disabled by default.

[Lazy-Rabbit-2001]

Instead, I propose a Boolean property on the camera to enable/disable the limits, and have it disabled by default.

Good idea, but the reason why I changed the default is that users couldn't see and drag the rectangle as it was too large to find the dragger if we still use default 1000000 rectangle, unless they change the limit_* manually in the inspector, which is worse than making it visible in a reasonable size so that the editor viewport can contain the full rectangle of the camera limits.
Yep, it is necessary to add a property that controls if the camera movement is limited, as by default the oversized limits are still "limited", so it will be more flexible to make this have the ability to be unlimited.
As for the unit test failure, I'll fix that soon.

[kleonc]

• Now the default limit will not be +/-1000000, instead, the top-left will be (0, 0) while the bottom-right will become (viewport_w, viewport_h)

This is compatibility breaking change. Properties set to their default values are not being saved to the resource (scene) file, meaning if someone was using Camera2D with its default limits unchanged, then such limits are not saved within the scene file containing the Camera2D. After this change loading such scene would create the Camera2D with the new defaults, resulting in changed behavior (likely breaking user's project).

[Lazy-Rabbit-2001]

I reverted the change of default value. What takes the place of the previous change has been added in the top post.

[kleonc]

• Added limit_unlimited, which allows the camera focus to move anywhere. Set to true by default.

Oh, haven't thought about that new bool suggestion previously, but limit_unlimited = true by default is compat breaking.

• For the case when the user uses default limits it indeed shouldn't be problematic because the current limit defaults (+/- 10_000_000) are practically speaking "infinite", meaning the behavior should be almost for sure unaffected.
• However, if the user uses custom limits then now such custom limits won't take any effect, because of limit_unlimited being enabled by default. So this is compat breaking, it requires the user to manually set limit_unlimited = false to restore the current behavior.

So the limits should be enabled by default to preserve the current behavior.

Regarding the limit_unlimited property name, I suggest renaming it to something simpler like limit_enabled (there are already many existing x_enabled properties across the engine, like CanvasItem.y_sort_enabled or Sprite2D.region_enabled), and of course swapping the meaning (AFAICT non-negated versions are preffered, and currently it's equivalent to limit_disabled).

[kleonc]

AFAICT no longer breaks compat. 👍

• Added limit_disabled, which allows the camera focus to move anywhere. Set to false by default.

As already mentioned in #101427 (comment), please rename it to limit_enabled and swap its default value/checks (even for the Camera2D itself there are already properties like enabled, drag_{horizontal|vertical}_enabled, {position|rotation}_smoothing_enabled).

[kleonc]

I suspect we don't want to add/bind such button as a property with PROPERTY_HINT_TOOL_BUTTON... Probably should be instead added using an EditorInspectorPlugin like e.g. Edit Region button for Sprite2D etc.? 🤔 cc @KoBeWi

[KoBeWi]

Yeah inspector plugin would avoid adding a property. The property is not needed for anything.

No longer compat-breaking.

[Lazy-Rabbit-2001]

Making the plugin of it seems a bit time-costing so I don't plan to add it in this pr. However, once i have available time for it, I'll try to add it in a continuation of this.
So I think it's time for reviewing the codes and the documentation.

[kleonc]

Tested a little in action, AFAICT works as expected. Overall LGTM, not sure about the exposed editor_draw_limits_color, see the comment I've added.

[kleonc]

For other reviewers: note there are other debug drawing colors which are still hardcoded, so this one being customizable/exposed is inconsistent in that regard. I'm not sure whether this is fine as is or if maybe something needs to be done about it.

godot/scene/2d/camera_2d.cpp

Line 375 in 1b7b009

Color area_axis_color(1, 0.4, 1, 0.63);

godot/scene/2d/camera_2d.cpp

Line 420 in 1b7b009

Color margin_drawing_color(0.25, 1, 1, 0.63);

[Calinou]

If it's to be made configurable, the limits color should be configurable in the editor settings rather than a per-node property, unless there's a demonstrable need for it to be configured on a per-node basis (like collision shapes).

That said, I would prefer we don't expose a setting at all if no need has been expressed by users beforehand.

[kleonc]

Don't forget to cleanup the commits, see the docs (there should be a single commit with descriptive title).

[AThousandShips]

There's no rush to do that until this is ready to be merged though

[Lazy-Rabbit-2001]

Don't forget to cleanup the commits, see the docs (there should be a single commit with descriptive title).

Thanks for informing that again but I've known this rule but I prefer to squash them until the final determination from you are almost fine to the pr. (except that sometimes there needs small tweaks after the determination is decided...)

Squashed for the first time. There will be continuing commits, but who knows...

[KoBeWi]

Is there still any use for limit_enabled? It looks like it was workaround for changed default limits, but since they were reverted, the property is not very much needed anymore 🤔 (unless you have a good reason to include it)

Also the default rectangle functionality has weird interactions with camera.

godot.windows.editor.dev.x86_64_9oHQES4Kh0.mp4

Trying to move the rect in Select mode will instead move the view. It's not something very bad, but a proper editor for the limits would be better.

[Lazy-Rabbit-2001]

Is there still any use for limit_enabled? It looks like it was workaround for changed default limits, but since they were reverted, the property is not very much needed anymore 🤔 (unless you have a good reason to include it)

If you disable it the camera can go anywhere without limits, even on single axis the camera can move to any x/y position over 100000000 pix. Basically it's kinda useful in this case...

Btw the problem in the video has been fixed

[KoBeWi]

There is a visible jump when dragging after resizing:

6WxRMFT5Sb.mp4

Also the camera can be selected by clicking inside it's rect. With camera using default limits, it means that clicking anywhere will select it.

If you disable it the camera can go anywhere without limits, even on single axis the camera can move to any x/y position over 100000000 pix

Not sure if going beyond 10000000 is a realistic use case.

[Lazy-Rabbit-2001]

There is a visible jump when dragging after resizing:

Fixed (maybe).

Also the camera can be selected by clicking inside it's rect. With camera using default limits, it means that clicking anywhere will select it.

I'm planning to propose a new pr to disallow all canvas items locked in the editor to be selectable, so basically I'm not considering fixing this.

[KoBeWi]

Creating Camera2D via right-clicking in the editor will result in awkward limits:

sxb8JjFov3.mp4

[Lazy-Rabbit-2001]

Reviewed what I made, I think I have to make a new shortcut for moving the rect:
Ctrl + Left
And dragging the view is still the direct left button held.

[KoBeWi]

If you hold Ctrl before dragging the rect, it will start rotation instead. Also the corner text is visible even if limits are disabled.

I still think that using rect for camera is not a very good idea. Now that you added a plugin, you can implement custom resizing functionality. The yellow lines that draw when editor_draw_limits is enabled could draw when the camera is selected and you could make them draggable (whole lines, not just gizmo points). It would solve the problem of accidental rotations, and selections I mentioned before.

[Lazy-Rabbit-2001]

If you hold Ctrl before dragging the rect, it will start rotation instead.

You missed a key condition: In moving mode. So if you are using default mode then yes it will rotate the rect.

As for the text, it will be fixed soon

I still think that using rect for camera is not a very good idea. Now that you added a plugin, you can implement custom resizing functionality. The yellow lines that draw when editor_draw_limits is enabled could draw when the camera is selected and you could make them draggable (whole lines, not just gizmo points). It would solve the problem of accidental rotations, and selections I mentioned before.

Thanks for noting, but I don't think it a good practice to remake a wheel to resize the rect in an plugin. At least it's quite time-consuming for me to do research on how to make a new dragging system.