Shapes refactor #323
Shapes refactor #323
Conversation
For readability, remove the ternary conditional operator, which was a long one-liner.
Remove the separate stroke line "edge_polygon" which was used only for control blocks. Since this line is connected to the "stroke_polygon" it can be joined with it. Also the "edge_polygon" was adding a redundant draw to non-control blocks. Also remove the outline_middle calculation. Taking screenshots as proof, this extra calculation has no effect in the drawing.
| var box_shape = _get_box_shape(size) | ||
| var bottom_knob_shape = _get_knob_shape(Vector2(Constants.KNOB_X, size.y)) | ||
| bottom_knob_shape.reverse() | ||
| return box_shape.slice(0, 3) + bottom_knob_shape + box_shape.slice(3) |
There was a problem hiding this comment.
The magic numbers you're passing to slice() here and in similar functions rely on the fact that _get_box_shape() returns exactly 5 co-ordinates in clockwise order from the top-left. It works but feels a bit error-prone.
I'm not sure what I'd do differently though. I guess you would end up open-coding those 4 coordinates in each of these places. That might be OK, it's just various combinations of 0 and box_size.[xy]. Anyway, no need to change it, it just feels a bit off to me.
There was a problem hiding this comment.
Good point. I added the "box shape" a couple minutes before submitting, and I'm not sure if it's worth it. Previously the only thing outside each draw function was the "get knob shape" and the rest was a straightforward list of points, although a bit of a boilerplate. I could go back to the boilerplate.
Another option would be to pass a dict to _get_box_shape() with optional tweaks for each side of the rectangle. Something like _get_box_shape(size, {"bottom": bottom_knob_shape}).
There was a problem hiding this comment.
I tried these 2 options and non of them looked better to me.
|
|
||
| if shift_top > 0: | ||
| stroke_polygon.append(Vector2(0.0, 0.0)) | ||
| if block_type == Types.BlockType.ENTRY: |
Refactor to have an easier to read separate draw function for each block type. Each draw function is now straightforward, without if/else conditionals. Instead of having multiple attributes like "shift_top", "show_top" to instruct the drawing, directly pass the block type. For control blocks that have 2 backgrounds, one for top and one for the bottom part, also expose a control_part attribute.
manuq
force-pushed
the
shapes-refactor
branch
from
29f1501 to
0118dcd
Compare
This is a refactor of the block background which has different shapes for the different block types. To make the work in #273 easier to rebase.