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

Combine Camera2D "limit_top", "limit_bottom", "limit_left" and "limit_right" to a Rect2i "limit" #3604

Closed
Mickeon opened this issue Nov 26, 2021 · 3 comments

Comments

@Mickeon
Copy link

Mickeon commented Nov 26, 2021

Describe the project you are working on

A typical 2D Godot game

Describe the problem or limitation you are having in your project

The Camera2D's limit variables (limit_left, limit_top, limit_right, limit_bottom) are all separate integers, which feels odd, considering they refer to the same thing. Even editor_draw_limits property, when enabled, draws a rectangle of them.
image

Furthermore, passing these as arguments of Vector2 or Rect2 functions becomes a somewhat repetitive task, especially noticeable if the camera itself is referenced in a variable.

func something(camera: Camera2D):
	var limit_rect := Rect2(
			camera.limit_left, camera.limit_top, 
			camera.limit_right, camera.limit_bottom
	)
	
	limit_rect.grow(2)
	update_generic_boundaries(limit_rect)
	generic_enemy.act_area = limit_rect

Even though these values could be effectively expressed as a top-left and bottom-right Vector2.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Combining all limit properties into a single Rect2i limit would make the variables less redundant while keeping its functionality relatively intact.

This would be a breaking change that takes advantage of Godot 4's new Rect2i (Rect2 for integers)

Notes, Drawbacks and other Proposals

  • I am unsure about the new name of the property, whether it should be limit, limits or something else entirely.
  • This proposal was mentioned in this comment and in [TRACKER] Methods, properties and signals to consider for renaming in next planned compatibility breakage godot#16863
  • An alternative to this proposal would be to have two Vector2i properties expressing the top-left and bottom-right points, which may be preferable to the current solution, considering all of the following proposals it would require to make this one more user-friendly.
    • I am aware that there's a difference between the Rect2i size and end. The exported values in the Inspector would actually be the position and the size, something often not desirable for Camera2D's use cases. For this reason, the ability to export Rect2i's end is something that's worth proposing elsewhere.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

	# Example code
	var limit := camera.limit
	update_generic_boundaries(camera.limit)
	
	limit.end.x += 20
	limit.position.x -= 50
	limit.expand(point_of_interest)

image

If this enhancement will not be used often, can it be worked around with a few lines of script?

It roughly can. Not really a few lines of script, and it is somewhat redundant.

extends Camera2D
tool

export var limit: Rect2 setget _set_limit

func _set_limit(new: Rect2):
	limit = new
	
	limit_left = limit.position.x
	limit_top = limit.position.y
	limit_right = limit.end.x
	limit_bottom = limit.end.y

image

Is there a reason why this should be core and not an add-on in the asset library?

The Camera2D class is a core feature.

@golddotasksquestions
Copy link

golddotasksquestions commented Nov 26, 2021

I get what you are saying, but I'm not sure if this is a good idea tbh.
limit.x, limit.y, limit.w, limit.h is just by far not as explicit self-explanatory and intuitive as limit_left, limit_right, limit_bottom, limit_top.

Dealing with camera movement and controls can be tough, confusing and challenging for beginners already. This is definitely a feature a beginner would use. I would be in favor of keeping it more explicit and self-explanatory and have it feel somewhat "odd", than making it more tough and less intuitive for beginners, but also less "odd".

@KoBeWi
Copy link
Member

KoBeWi commented Nov 27, 2021

The problem is that Rect2 has width and height for right and bottom properties, so the camera limit would need to be camera bounds (i.e. top-left and size) to be intuitive as rect.

@Mickeon
Copy link
Author

Mickeon commented Aug 3, 2022

Closing, at least for now, as for this to be at the very least nicely doable there would need to be changes to the core class Rect2, or a more personalised way this very Variant could be exported to the editor.

@Mickeon Mickeon closed this as completed Aug 3, 2022
@KoBeWi KoBeWi added the archived label Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants