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

[4.2] Fix SubViewport physics picking #85665

Merged

Conversation

Sauermann
Copy link
Contributor

Apply the logic of handle_input_locally for physics picking (as done in #77730).

resolve in 4.2 issue #84258

For master I consider #77926 the better approach, but in order to not break compatibility in 4.2, I consider this a viable option to fix this in 4.2.

@Sauermann Sauermann added this to the 4.2 milestone Dec 2, 2023
@Sauermann Sauermann requested a review from a team as a code owner December 2, 2023 16:13
@AThousandShips
Copy link
Member

AThousandShips commented Dec 2, 2023

Also considering this copies the code from #77730 I'd say a co-author would be appropriate (it's the same code in a different place)

@Sauermann
Copy link
Contributor Author

Initially this code-section was introduced by reduz in #37317:
https://github.com/godotengine/godot/blame/d76c1d0e516fedc535a2e394ab780cac79203477/scene/main/viewport.cpp#L3657-L3673
Also the style with the multiple break; conditionals comes from that PR.

Please let me know, if I should add reduz as co-author. I'm not yet very familiar with when to use co-author.

@AThousandShips
Copy link
Member

Then no need for co-author IMO, thought the other PR introduced this specific code

I'd suggest compressing the code regardless of the other blocks but it's either way 🙂

Apply the logic of `handle_input_locally` for physics picking.
@Sauermann Sauermann force-pushed the fix-4.2-subviewport-physics-picking branch from a2fd4bf to ab7b662 Compare December 3, 2023 22:35
@akien-mga akien-mga changed the title Fix SubViewport physics picking for 4.2 [4.2] Fix SubViewport physics picking Dec 5, 2023
@akien-mga akien-mga merged commit 289472d into godotengine:4.2 Dec 5, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

3 participants