Applying Magnet Alignment Algorithm to newly opened Component Browser #3366
Conversation
vitvakatu
force-pushed
the
wip/vitvakatu/magnet-algorithm-181076145
branch
2 times, most recently
from
b2a96c4
to
5f697df
Compare
vitvakatu
force-pushed
the
wip/vitvakatu/magnet-algorithm-181076145
branch
from
5f697df
to
fdcaf8e
Compare
vitvakatu
requested review from
MichaelMauderer,
wdanilo and
farmaazon
as code owners
vitvakatu
changed the title
| @@ -29,6 +29,12 @@ pub mod free_place_finder; | |||
| /// === New Node Positioning === | |||
| /// ============================ | |||
|
|
|||
| /// The maximum distance to the nearest grid line in Magnet Alignment algorithm. | |||
There was a problem hiding this comment.
Distance is in pixels, isn't it? Add the info to the docs (or to the constant name, e.g. in suffix _PX)
There was a problem hiding this comment.
No, the distance is in "scene units" (as coordinates). I mentioned it in the docs.
| fn magnet_alignment( | ||
| graph_editor: &GraphEditorModel, | ||
| position: Vector2, | ||
| only_horizontally: bool, |
There was a problem hiding this comment.
Instead of passing bool as argument, we could make an enum with explanatory names. Then we could have only one public magnet_alginment method instead of three (there will be no need for _x/_xy variants if enum names will be self-explanatory).
CHANGELOG.md
Outdated
| - [Magnet Alignment algorithm is used while placing new nodes][3366]. The new | ||
| node position now depends on the positions of the surrounding nodes. This | ||
| makes the resulting graph prettier. |
There was a problem hiding this comment.
This description does not really tell what is happening here. I'd add some info about empty space on the screen and why we do it.
There was a problem hiding this comment.
I'm not sure how to express it shortly, so I ask for help if you don't like an updated description.
| /// | ||
| /// This value defines the "sensitivity" of the Magnet Alignment for new nodes - greater values | ||
| /// lead to more "aggressive" snapping. | ||
| const MAGNET_ALIGNMENT_THRESHOLD: f32 = 150.0; |
There was a problem hiding this comment.
Looking at the videos I feel that this area is too big now. But to know exactly we need to test it live. Just saying, nothing to be fixed here for now.
There was a problem hiding this comment.
Yeah, totally, this requires some additional testing.
| // Perform Magnet Alignment if the node is pushed left to the free space. | ||
| if pos.x != align_x { |
There was a problem hiding this comment.
instead of comments, do variables. was_pushed_left = ...
Pull Request Description
Task link
This PR enables new node position adjustment using the Magnet Alignment algorithm for the following cases:
The size of the
alignment areaaround node is slightly enlarged, so that it's impossible to create a node that is being too close to other nodes.Videos with AC demonstration:
2022-03-29.11-05-11.mp4
2022-03-29.11-06-40.mp4
2022-03-29.11-09-38.mp4
Checklist
Please include the following checklist in your PR:
./run distand./run watch.