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

Offset drag instantiated scenes that result in a collision by its bounds to mitigate overlap #88511

Conversation

ryevdokimov
Copy link
Contributor

@ryevdokimov ryevdokimov commented Feb 18, 2024

  • Offset drag-instantiated scenes to place them on top of the targeted surface, raised along the surface's normal.
  • Fix dragging behavior and appearance of scenes that have top-level set to true. Now, they can be positioned like non-top-level nodes while being dragged. Once they're dropped, they will remain at the same global position with their top-level flag set to true.

@cooperra

Before:

2024-02-18.13-38-22.mp4

After:

2024-02-18.13-37-09.mp4

@ryevdokimov ryevdokimov requested a review from a team as a code owner February 18, 2024 18:46
@ryevdokimov ryevdokimov changed the title Offset drag instantiated scenes that result in a collision by its bounds to prevent overlap Offset drag instantiated scenes that result in a collision by its bounds to mitigate overlap Feb 18, 2024
@ryevdokimov ryevdokimov marked this pull request as draft February 18, 2024 21:11
@ryevdokimov ryevdokimov force-pushed the offset-drag-to-instantiate-scene branch from d4eed20 to e046f54 Compare February 19, 2024 02:08
@Cammymoop
Copy link
Contributor

Could have a proposal since this is added functionality, if so I would also suggest adding this functionality to the move and/or select mode tools because this could be quite useful for them as well and isn't easily recreated by any other existing functionality afaik.

Regarding the implementation, how does it interact with non-axis aligned surfaces?

@ryevdokimov
Copy link
Contributor Author

ryevdokimov commented Feb 24, 2024

Regarding the implementation, how does it interact with non-axis aligned surfaces?

I need to get back into this, but I believe that preview nodes are packaged into a temporary node that is not rotated, so the bounds are always axis aligned. My current issue is that the scenes origin isn't always the center of the AABB, so that needs to be factored into the offset, if that even makes sense.

I would also suggest adding this functionality to the move and/or select mode tools because this could be quite useful for them as well and isn't easily recreated by any other existing functionality afaik.

Would be cool.

@Cammymoop
Copy link
Contributor

I need to get back into this, but I believe that preview nodes are packaged into a temporary node that is not rotated, so the bounds are always axis aligned. My current issue is that the scenes origin isn't always the center of the AABB, so that needs to be factored into the offset, if that even makes sense.

Yeah, I mean, if the the surface you're instancing onto is not axis aligned, not the scene you're instancing. Does it stick the center of the closest aabb side to the surface or something?

@ryevdokimov
Copy link
Contributor Author

ryevdokimov commented Feb 24, 2024

Does it stick the center of the closest aabb side to the surface or something?

The current mechanism for Godot is to use the intersection of a viewport ray and a collider (which doesn't have an AABB), to determine the global position of where to instantiate the scene. This means that currently you can't snap to meshes without colliders, and the collider shape doesn't necessarily have to match the mesh.

I do have this PR: #87969 that does factor in the target AABB by using meshes instead of colliders, but it has its own issues. The answer for an ideal solution that encompasses flexible snapping to whatever layer is probably somewhere in between. An interesting and maybe dumb idea is to temporary build a collision sibling similar to how you can manually do in the editor via:

image

and then snap to that. It's a tricky problem.

@ryevdokimov ryevdokimov force-pushed the offset-drag-to-instantiate-scene branch from e046f54 to 42f738d Compare April 1, 2024 13:28
@ryevdokimov ryevdokimov force-pushed the offset-drag-to-instantiate-scene branch from 42f738d to a60ab6c Compare April 1, 2024 13:57
@ryevdokimov ryevdokimov marked this pull request as ready for review April 1, 2024 13:59
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 3576e84), it works as expected. Code looks good to me.

This will be good to go alongside #96740 to improve its usability 🙂

@akien-mga akien-mga merged commit 2135346 into godotengine:master Oct 4, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@ryevdokimov ryevdokimov deleted the offset-drag-to-instantiate-scene branch October 4, 2024 21:24
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.

7 participants