Add snap to floor functionality to the editor #20022

EIREXE · 2018-07-07T19:25:57Z

UE4 style floor snapping.

Default binding is page down.

Zylann · 2018-07-08T00:03:36Z

I was nearly going to make a plugin for that because I need it, but glad to see an initiative to integrate into core :)
Several notes after reading the code:

You should implement it using UndoRedo, currently it cannot be undone
Don't just set the translation of the Spatial, it would be better to take its AABB into account, and snap from its bottom
It won't work for scenes having no colliders (not sure if that's a problem though)

EIREXE · 2018-07-08T08:12:04Z

@Zylann

You should implement it using UndoRedo, currently it cannot be undone

I will do that

Don't just set the translation of the Spatial, it would be better to take its AABB into account, and snap from its bottom

That's what I wanted to do at first, but I haven't found any way to get the AABB of a node and it's children combined, I could try iterating through the children but I feel like that would get messy.

It won't work for scenes having no colliders (not sure if that's a problem though)

If there's no floor to snap to it just doesn't snap to anything.

EIREXE · 2018-07-08T08:35:39Z

I just realised that maybe we could simply get the AABB from all VisualInstances and use that, can someone think of a better way?

Zylann · 2018-07-08T12:41:57Z

If the thing to snap has physics, use its collision shapes. If it doesn't, use its VisualInstance AABBs. If none of those, use origin.

EIREXE · 2018-07-08T14:03:25Z

I will do that then.

EIREXE · 2018-07-08T17:56:56Z

How do you guys suggest I get the bounds of collision physics?

toger5 · 2018-07-08T19:08:00Z

git reset HEAD~4
git commit -a -m "commit message"
git push -f

;)

it's really nice to have snap to floor!

EIREXE · 2018-07-08T19:57:44Z

@toger5 I will merge commits when it's done, at the moment it's lacking snap to floor for collision shapes, which I still need someone to help me with.

EIREXE · 2018-07-09T14:08:16Z

I have merged the commits and it now works with collisionshapes too.

EIREXE · 2018-07-21T07:15:54Z

fails on travis only, anyone knows why this is?

Zylann · 2018-07-21T15:25:14Z

It looks like it doesn't like templates in your get_child_nodes function. Maybe it needs the typename prefix before T.
Also, did you try to not make it a member function since it's private? (and just make it a normal function in the .cpp)

EIREXE · 2018-07-21T22:52:44Z

@Zylann thanks mate, fixed now

akien-mga · 2018-07-21T22:53:40Z

editor/plugins/spatial_editor_plugin.cpp

+	for (int i = 0; i < parent_node->get_child_count(); i++) {
+		Node *child_node = parent_node->get_child(i);
+		Set<T *> child_nodes = _get_child_nodes<T>(child_node);
+		for (typename Set<T *>::Element *I = child_nodes.front(); I;


Please don't wrap forcefully when it makes things less readable, like here (several other occurrences). In general we don't have max char limit because the wrap it imposes it almost always bad for readability.

Must have been a clang format misconfiguration on my end

mhilbrunner · 2018-07-24T11:07:55Z

I build & tested this locally, it seems to be working fine. Thanks for the contribution :)

mhilbrunner · 2018-07-24T11:28:00Z

reduz pointed out it would be good for discoverability to have this added to the Editor "Edit" menu so people can easily find it's there and learn the shortcut.

Besides that, this should be good to go. :)

Zylann · 2018-07-24T12:36:04Z

Hmm I wonder something: in my terrain editor, terrain chunks are not nodes. They are mesh instances that exist in VisualServer only. Collision is also not generated in editor for performance reasons (and is not very useful to keep updating it as it gets edited constantly and had no use so far). But how would snap to floor work in this situation?

mhilbrunner · 2018-07-24T12:42:02Z

@Zylann The PR does check VisualInstance.size. No idea beyond that.

Zylann · 2018-07-24T12:59:35Z

@mhilbrunner the code searches for VisualInstance nodes. But my terrain doesn't use nodes, it only uses VisualServer directly. That means it won't work and I wonder what to do about it :/

mhilbrunner · 2018-07-24T15:50:40Z

@Zylann Maybe your terrain could use nodes, e.g. one per chunk? Other than that, we'd need to discuss with reduz/core devs how to do that, or see if there is a way to add to what this PR does, see if the needed information is exposed somewhere in VisualServer.

But that really is orthagonal to this PR :) We should aim to get this merged so it works in 90% of cases, then see about how it could work with terrain.

But such integration (or lack thereof) is really why I feel terrain should be supported in core.

Zylann · 2018-07-24T16:17:28Z

@Zylann Maybe your terrain could use nodes, e.g. one per chunk?

That would be a big waste to only do this :/ And even if I did, collision is the only way to go, visual meshes get displaced in shader.
I thought there would be a way to simply ask VisualServer instead of the scene tree when looking for VisualInstances (selection does that I think? so why not snap), but I'm curious what reduz thinks about it.

EIREXE · 2018-07-25T05:42:45Z

@mhilbrunner What edit menu? You mean the top bar?

EIREXE · 2018-07-25T08:32:20Z

I made it like this, let me know what you think.

Two-Tone · 2018-07-25T09:52:33Z

Side the tool tip for the lock is "Lock the selected object... etc", this should probably say "Snap the selected object to the floor."

…

On Wed, Jul 25, 2018, 3:32 AM Alex Roman Nuñez ***@***.***> wrote: [image: image] <https://user-images.githubusercontent.com/3101690/43189061-fe3e850a-8ff5-11e8-86a2-cb3e33359792.png> I made it like this, let me know what you think. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#20022 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEJESzId5U5ztaUk7WP_4kCZ2pbmHfyBks5uKC0ugaJpZM4VGdzM> .

mhilbrunner · 2018-07-25T19:21:22Z

reduz> I would put that in the Transform menu

Can you move the option there? :) Sorry for not being precise initially.

EIREXE · 2018-07-26T06:35:56Z

Like this?

mhilbrunner · 2018-07-26T20:30:31Z

Yes. Thanks for making those changes, and thanks for the making the contribution in the first place!

seadra · 2018-07-27T00:26:39Z

This doesn't seem to be working correctly with a mesh collider; it snaps the object to the level of highest place in the mesh.

For example, if you have a terrain with hills, and if you try to snap an object in a valley, snapping make it hovers over the valley (at the height of the hills).

mhilbrunner · 2018-07-27T09:22:06Z

Well, it uses the extents, I'm not sure what the alternative would be for a Mesh Collider - use Raycasts downwards?

Zylann · 2018-07-27T09:22:47Z

@seadra does your terrain have a concave trimesh collider? AFAIK this PR uses raycast so the only way for this to happen is the collider is convex.
@mhilbrunner it should use AABBs only for the snapped objects, not the environment. If AABBs are used for environment then it's not good (should use mesh data to intersect manually after broad AABB detection)

mhilbrunner · 2018-07-27T09:25:07Z

Ah, right.

AndreaCatania · 2018-07-27T10:21:07Z

Just found it for case, Thanks you for this "so useful" addition!

EIREXE · 2018-07-27T11:05:25Z

@Zylann Yes it uses a raycast, unless there's a better way I'm not familiar with.

seadra · 2018-07-27T23:47:19Z

@Zylann The terrain I tested with uses convex static body, which was generated by Godot.

Zylann · 2018-07-28T00:28:12Z

@seadra that explains it then. Convex means Godot will generate an encompassing mesh collider. Try using concave trimesh instead.

seadra · 2018-07-28T01:41:23Z

@Zylann Thanks, that was it! But there's still some considerable margin/gap between the object collider and the ground...

vnen added feature proposal topic:editor usability labels Jul 7, 2018

vnen added this to the 3.1 milestone Jul 7, 2018

akien-mga reviewed Jul 21, 2018

View reviewed changes

Add snap to floor functionality to the editor

af72509

mhilbrunner merged commit 6cf5eb8 into godotengine:master Jul 26, 2018

Zylann mentioned this pull request Mar 24, 2020

The collision seems to be lost in the Editor but not when playing the Scene Zylann/godot_heightmap_plugin#129

Open

Add snap to floor functionality to the editor #20022

Add snap to floor functionality to the editor #20022

Conversation

EIREXE commented Jul 7, 2018 • edited Loading

Zylann commented Jul 8, 2018 • edited Loading

EIREXE commented Jul 8, 2018

EIREXE commented Jul 8, 2018

Zylann commented Jul 8, 2018

EIREXE commented Jul 8, 2018

EIREXE commented Jul 8, 2018

toger5 commented Jul 8, 2018 • edited Loading

EIREXE commented Jul 8, 2018 • edited Loading

EIREXE commented Jul 9, 2018

EIREXE commented Jul 21, 2018

Zylann commented Jul 21, 2018 • edited Loading

EIREXE commented Jul 21, 2018

akien-mga Jul 21, 2018

Choose a reason for hiding this comment

EIREXE Jul 21, 2018 • edited Loading

Choose a reason for hiding this comment

mhilbrunner commented Jul 24, 2018

mhilbrunner commented Jul 24, 2018

Zylann commented Jul 24, 2018 • edited Loading

mhilbrunner commented Jul 24, 2018

Zylann commented Jul 24, 2018

mhilbrunner commented Jul 24, 2018 • edited Loading

Zylann commented Jul 24, 2018 • edited Loading

EIREXE commented Jul 25, 2018

EIREXE commented Jul 25, 2018

Two-Tone commented Jul 25, 2018 via email

mhilbrunner commented Jul 25, 2018 • edited Loading

EIREXE commented Jul 26, 2018 • edited Loading

mhilbrunner commented Jul 26, 2018

seadra commented Jul 27, 2018

mhilbrunner commented Jul 27, 2018

Zylann commented Jul 27, 2018 • edited Loading

mhilbrunner commented Jul 27, 2018

AndreaCatania commented Jul 27, 2018

EIREXE commented Jul 27, 2018

seadra commented Jul 27, 2018

Zylann commented Jul 28, 2018

seadra commented Jul 28, 2018

EIREXE commented Jul 7, 2018 •

edited

Loading

Zylann commented Jul 8, 2018 •

edited

Loading

toger5 commented Jul 8, 2018 •

edited

Loading

EIREXE commented Jul 8, 2018 •

edited

Loading

Zylann commented Jul 21, 2018 •

edited

Loading

EIREXE Jul 21, 2018 •

edited

Loading

Zylann commented Jul 24, 2018 •

edited

Loading

mhilbrunner commented Jul 24, 2018 •

edited

Loading

Zylann commented Jul 24, 2018 •

edited

Loading

mhilbrunner commented Jul 25, 2018 •

edited

Loading

EIREXE commented Jul 26, 2018 •

edited

Loading

Zylann commented Jul 27, 2018 •

edited

Loading