-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Add make_local_canvas_position_global, the inverse to make_canvas_position_local #36931
base: master
Are you sure you want to change the base?
Add make_local_canvas_position_global, the inverse to make_canvas_position_local #36931
Conversation
Make a local canvas position global
Made a test project. Please let me know if there's a formal test framework to add something to. |
We don't have built-in unit tests yet, but once #30743 is merged, there will be a way to write unit tests in C++ (which involve less moving parts than tests written in GDScript). |
I think the name of the function can be better, or at least it should follow the same naming pattern as the old function. How about "make local position canvas"? |
Hmm that's better. Honestly both are clumsy. I hate the original naming I choose, but I was trying to stick to the previous convention based on suggestions. I'd rather call both something like |
I prefer not to use local and global in this case, I think 'screen' and 'canvas' are better names since this code is specific to Canvas Item objects. I think something like this would be best: or even better, |
Hmm, but since it is called on a `Canvas` object, doesn't this make adding
the word `canvas` redundant? Like for a `Vector2.normalized()` , it seems
redundant to call it `Vector2.vector_normalized()`
…On Tue, Mar 10, 2020 at 10:39 AM m Kotob ***@***.***> wrote:
I prefer not to use local and global in this case, I think 'screen' and
'canvas' are better names since this code is specific to Canvas Item
objects.
I think something like this would be best:
make_canvas_position_to_screen_position and
make_screen_position_to_canvas_position
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36931?email_source=notifications&email_token=ACCZK7YE6N3LQDC4YTXB6OTRGZ3O3A5CNFSM4LEBLFK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOMNT6Y#issuecomment-597219835>,
or unsubscribe
</~https://github.com/notifications/unsubscribe-auth/ACCZK74JLOIZOTNCQM557SDRGZ3O3ANCNFSM4LEBLFKQ>
.
|
Updated PR: Ok what about
Also it won't break the old API |
074ccc2
to
260a2a3
Compare
@Razzlegames The new names sound fine to me, it's to have redundant function names exposed instead of unclear ones. Can potentially be improved but unless we find some existing naming convention to follow, this is probably the best we can think of for now. |
Needs rebase and style fixes. |
Transforms [code]local_point[/code] to a screen point. | ||
Basically the reverse of [code] make_canvas_position_local [/code] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transforms [code]local_point[/code] to a screen point. | |
Basically the reverse of [code] make_canvas_position_local [/code] | |
Transforms [code]local_point[/code] to a point in viewport's coordinate system. | |
See also [method get_global_transform_with_canvas]. |
Make a local canvas position transformed to global space.
Vector2 make_canvas_position_local(const Vector2 &screen_point) const;
Finally a follow up to #4854 , you know... 4 years later. :)
Test project: /~https://github.com/Razzlegames/GodotCanvasLocalToGlobalTest