waterbearlang / waterbear

Visual block syntax for programming languages
http://waterbearlang.com/
357 stars 88 forks source link

#898, add shape blocks #1241

Closed ghost closed 8 years ago

ghost commented 8 years ago

Not ready to merge yet, will add a few more shapes first.

Ready for review.

kotarCreative commented 8 years ago

can you post a screen shot of the actual block please. The html looks a little funny.

ghost commented 8 years ago
screen shot 2015-10-24 at 11 47 11 am
ghost commented 8 years ago
screen shot 2015-10-24 at 11 48 51 am
CelticMinstrel commented 8 years ago

I'm not entirely happy with how this looks...

There are several ways to uniquely specify a triangle without reference to points. Two angles and a side, two sides and an angle, three sides, or base+height with one additional piece of information. (And if you know the triangle is isosceles, base and height are enough.) If you have a polygon block though, is it necessary that the triangle block(s) can represent every possible triangle? (Might be easier, mind you.)

For polygons, I'm not so sure how to uniquely specify one without reference to points. One way would be to specify an n-sided polygon with n-1 vectors, but I think that would likely confuse people. (It's n-1 because with n vectors you can't guarantee a closed perimeter; the starting point is implicitly (0,0), and the side that ends at (0,0) is not explicitly given in any way.) Another way is the "turtle graphics way" - a sequence of side lengths and external angles (again, you'd need n-1 elements to ensure a closed path). I think that could also be confusing, though. Maybe it's fine for polygons to be location-dependent? I dunno.

ghost commented 8 years ago

I have to say I disagree with your comments on how to define a triangle. Yes there are many ways you can do it such as specifying angles and what type of triangle, but the most straight forward way is to define 3 points. I would argue that this way is much more clear for a user. Also you can get every type of triangle this way and it doesn't over complicate it. It also isn't really that inconsistent with the other shapes, the circle is also defined by its location as well as a radius. The polygon stuff hasn't been pushed yet. The blocks you see there are where I started out but since then I have moved on to merging paths and shapes and will be treating a polygon as a closed path. I agree that this will make the triangle block redundant, but we can decide that once everything else is done. Issue #1238 has details on merging paths into shapes.

ghost commented 8 years ago

I guess I should also be clear in saying that I am treating a polygon as its proper definition, a finite set of connected line segments that start and end at the same point. In my first implementation it implicitly joins the last point to the first to close the path, but once I have merged in the paths we can can just have one block where the user specifies if its closed or not, if we want that.

CelticMinstrel commented 8 years ago

Ah, you're right, I missed that. Perhaps I'm not entirely clear on what a "shape" represents. There's nothing wrong with the shape knowing its own location, so I suppose the interface you've implemented is okay. I haven't looked at the actual implementation of the blocks, though. I think it's good if the definition of the shape is location-agnostic (so the shape can be represented as a location plus a definition of its form). On the other hand, translation isn't that hard if you have a list of points, so maybe it's not a big deal. (Speaking of which, unless I'm missing something there are no blocks for transformations of shapes...? That's probably out of scope for this PR though.)

For something completely unrelated... I could easily do this myself, but I'm not sure if it would create merge conflicts with your work. I think the rectangle block should say "anchored at" instead of "orientation", since orientation implies rotation.

ghost commented 8 years ago

I personally have no issue with the shapes knowing their own locations, many other libraries implement shapes the same way. For example the p5js library uses points to define all of its shapes, and adding similar API's to that library is what this issue is about. It is possible that "Shapes" isn't the best word to describe these blocks anymore, p5js refers to the group as "2D primitives".

And sorry for the confusion about the polygons. Those blocks will be with the other work I am doing to merge paths and shapes in PR #1246. I have not been working on this PR this week since we decided issue #1238 was higher priority.

You are welcome to change "orientation" to "anchored at", it will not conflict with any of my work (or if it does I know how to merge conflicts so it won't be an issue) and I agree that wording would make a lot more sense.

dethe commented 8 years ago

@CelticMinstrel I think our current method for transformation of shapes (translate and rotate, doesn't look like we have scale there yet) is to assign the shape to sprite and transform the sprite. Going forward, I'm tempted to make the sprite library more implicit and let any image, set of images, or shape be treated as a sprite without having to create a sprite explicitly. Not 100% sure about this, but if we do keep sprites we should probably move scaling from images to sprites so all transformations are in once place and can be applied to shapes as well as to images.

dethe commented 8 years ago

@AdriennePind I added shapes for regular polygon and star in PR #1276 because I wanted them for testing, so don't worry about those.

kotarCreative commented 8 years ago

So where is this branch at? is it ready for review yet?

ghost commented 8 years ago

No this had been put on hold for a couple weeks while I worked on a higher priority task. I will make sure to change the status of the PR when it is ready for review and post it in slack. It is my intention to get back to work on this this week, but as I have other assignments to finish first, I likely won't have time till later in the week.

ghost commented 8 years ago

I have added a couple more of the p5js blocks for bezierPoint and bezierTangent. Here is a gist them in use: 35fe369767c4e2ff6a04.

I'm not sure that I want to add any of the curve functions, but would like a second opinion on that. If anyone wants to take a look at the list in #898 to see the remaining shapes that p5js supports and let me know what you think about them, that would be great.

samuel-massinon commented 8 years ago

Code looks good and gist works. :shipit: