Open buckle2000 opened 8 years ago
We're trying to limit breaking changes to major releases, and we just had one of those.
So how about the next release?
Hey there :) I was the one who wrote these classes originally and I use them extensively in Defender's Quest.
I suppose they could be refactored, but like Gama says we're trying to limit breaking changes, so anything that we do here, especially if it goes into flixel core, would need to be delayed until the next point release. I mean eventually we'll have to start talking about what's going to go into the next major point release and start a branch working towards that and all, so if we do agree to move forward on these changes it might be a while before they make it into a release, until then they'd live on a branch.
As for the actual points you brought up, seems like it could clean some things up but I want to make sure I'm clear on what the benefits are.
To start, the original design was based on the paradigm that a FlxShape should be an easy to use geometric object where the size and boundaries of the object matches the underlying shape, rather than just be a raw pixel canvas with a shape on it -- ie, if you have a 10x10 square with a line thickness of 4, it's flixel bounding box is 10x10, not 14x14. This is accomplished by drawing the shape and setting bounding box offsets. This way if the user wants such a square at X,Y, they just put it there rather than constantly doing X+myshape.linethickness/2, etc.
So my understanding is you want to change things so that instead of having a class for each shape, you make a sort of factory pattern out of FlxShape static functions, and then those just return FlxSprites with all the proper stuff done to them? If you need to change the radius of a circle or update its line thickness would you just pass your circle instance back to the FlxShape circle function with new parameters? What does your proposed workflow look like?
@larsiusprime @Gama11 glad to meet you too!
FlxShape
and so onFlxShape
is an abstract class, basically an interface between subclasses and {FlxSprite
and other users}.
FlxShape
should not extend anything (as far as I see), it even should never implement IFlxDestroyable
.
They should not have properties like line thickness, fill color or something else. These properties should be defined just in the time before drawn, and should be passed in as arguments.
Just think it as a geometry.
Every FlxShape should has a way to be drawn!
There are 2 possible ways (or more?) to draw a FlxShape
on a sprite:
Put a static method in FlxSpriteUtil
.
Disadvantage: has even no extendibility to people that do not want to change haxeflixel code.
Also, a lot of Std.is(shape, FlxPolygon)
will be written, for sure.
Let
FlxShape
havedraw(graphics, some arguments)
. But what type shouldgraphics
be? Perhapsflash.display.Graphics
?
Receiving attributes like border width will still be a pain. Suggestions:
drawShape(g: SomeGraphics, shape: FlxShape, linewidth: Float,linecolor: FlxColor,....
10+ more args perhaps. I must say that I have seen this kind of code in HaxeFlixel's code more than once. It is NOT the best way to deal with this problem.FlxShapeFormat
, FlxShapeGraphicsFormat
or something else. (complex)
e.g. drawShape(g: SomeGraphics, shape: FlxShape, format: FlxShapeFormat)
{color:0x0000FF, x: 0.3}
should be passed in.Same as its name, it is a polygon, but still a shape.
polygons have vertices: var vertices:Array<FlxPoint>
circles are polygons.
Since it has vertices, you can:
FlxPolygon
freely, if you know transformation matrix.using ..FlxSpriteUtil; // for sprite.drawShape(...)
import ..FlxPolygon;
import flixel.FlxSprite;
import flixel.math.FlxPoint;
class SomeClass {
var sprite: FlxSprite; // just pretend that it is pre-initialized
function somefunc() {
var a_circle = FlxPolygon.ellipse(radius, radius, center_position);
var format = SomeShapeFormat(linewidth, fillcolor, etc...);
sprite.drawShape(a_circle, format); // how we draw
a_circle.scale(scale_on_x, scale_on_y); // resizing
a_circle.rotate(angle_in_radians); // resizing
a_circle.applyMatrix(matrix); // advanced use: transfomation matrix. other methods like shape.scale should call this method.
var a_sqaure = FlxPolygon.regular_polygon(4, sidewidth);
var vertices = [];
vertices.push(new FlxPoint(0, 0));
vertices.push(new FlxPoint(0, 10));
vertices.push(new FlxPoint(20, 30));
vertices.push(new FlxPoint(50, 90));
var a_custom_polygon = new Polygon(vertices); // notice that a polygon is a close shape. so (50,90) and (0,0) should be connected when drawn.
var a_strange_shape=FlxPolygon.intersect(a_custom_polygon,a_circle); // get intersection
var overlaps:Bool=FlxPolygon.overlaps(a_custom_polygon,a_circle); // test overlapping
}
}
Also the first comment of this issue is updated.
I think its better to put helper methods in FlxPolygon
.
Check This First:: https://github.com/HaxeFlixel/flixel-addons/blob/dev/flixel/addons/display/shapes/FlxShape.hx
Main Problem
I know this will break API capability. But the earlier it is fixed, less problem will appear in the future!
For several reasons,
FlxShape
should not be like it is now. e.g. it is not good for MVC separation.FlxShape
should NOT be a subclass ofFlxSprite
. But drawing a shape becomes a problem. See Questions below.FlxPolygon
(or namedFlxPoly
or else), which is a subclass ofFlxShape
should be placed. Not all shapes are polygons! e.g. Line,FlxCircle
should not be child ofFlxShape
, but ofFlxPolygon
. For some seldom later use, a new classFlxFastCircle
may take place, which can only be drawn, and does not have features ofFlxPolygon
, which will be discussed later.flixel.addons.display.shape
exceptFlxShape
shall be removed!Instead,
_==== UPDATED ====_ on 2016.3.15 0:28 UTC+8 Put some static methods in _FlxPolygon_ (simple shapes like box, triangle) or
FlxShapeUtil
(complex shapes likeFlxShapeDoubleCircle
).Methods in
FlxShape
will generate aFlxShape
(basically aFlxPolygon
).Methods in
FlxShapeUtil
will generate aFlxShapeGroup
(just likeFlxTypedGroup<FlxShape>
, but is a subclass ofFlxShape
, with additional implements for easy printing and etc..More classes = more mess.
flixel.shape
orflixel.util
(FlxShapeUtil
).I believe that it is important enough to be moved into main Flixel distributed package!
After you agree with this change, I will add the class
FlxPolygon
and when I have time.FlxPolygon
FlxPolygon will have following features:
FlxCircle
is aFlxPolygon
with many vertices.getArea()
andoverlaps(other:FlxPolygon)
.subtract(other:FlxPolygon)
,union(other:FlxPolygon)``,
intersection(other:FlxPolygon)`` and so on!What's more operators should be overriden. A.subtract(B) can be written as A - B.
PS: A + B == A & B
Disadvantages / Questions
I have some questions and unmade decisions. This proposal also has some possible disadvantages.
There are multiple solutions.
FlxSpriteUtil
have a unified static method to print aFlxShape
on aFlxSprite
. Bad.FlxShape
havedraw(graphics, some arguments)
. But what type shouldgraphics
be? Perhapsflash.display.Graphics
?Create a haxelib called shape2d, with shapes which can be used in many projects (like physics engines and game graphics), and then wrap
shape2d.*
around withFlxShape
. It is rather complex, but its benefit will be brought to the surface, I believe.~~-
FlxShape
is probably redundanceSince
FlxShape
seems to be only a helper class of creatigFlxPolygon
, and no other class extends from it currently, this middle-class might shall be dropped.~~