Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graphics.drawCircle 3rd arg ("radius") is actually diameter #1454

Closed
HeatherSoron opened this issue Dec 14, 2014 · 8 comments
Closed

Graphics.drawCircle 3rd arg ("radius") is actually diameter #1454

HeatherSoron opened this issue Dec 14, 2014 · 8 comments

Comments

@HeatherSoron
Copy link

While trying to whip up a quick conveyor belt using the Graphics object, I discovered that the third argument (called "radius" in the docs) actually defines the circle's diameter. Not too difficult to deal with once I know about it, but guys, that's not what radius means :P.

Observed in Phaser v2.2.1.

Probably a Pixi bug? Want me to cross-file it over there?

Have some sample code below, which should demonstrate the issue nicely:

function addConveyor(x, y, width) {
    var rad = 10;
    var conveyor = game.add.graphics(x, y);
    conveyor.beginFill(0xdddddd, 1);

    // draw circles to represent wheels on both ends of the conveyor

    // circle 1: assume that arg #3 is radius (as per docs). Ends up drawing half the height of the conveyor.
    conveyor.drawCircle(rad, rad, rad); // offset circle by radius in both directions
    // circle 2: assume that arg #3 is diameter. Ends up drawing the same height as the conveyor.
    conveyor.drawCircle(rad + width, rad, rad * 2);

    // draw a rect for the body of the conveyor belt
    conveyor.drawRect(rad, 0, width, rad * 2);
}
@pnstickne
Copy link
Contributor

Might be related to #1288

@photonstorm
Copy link
Collaborator

This isn't technically a Pixi issue (it's a result of a change there, but not a bug they created).

Because PIXI.Circle uses x, y, radius where-as Phaser.Circle uses x, y, diameter.

Phaser replaces PIXI.Circle with Phaser.Circle - but the PIXI Graphics class now creates these objects every single time you draw something (erk) so the docs are out of whack. I'll over-ride drawCircle from Phaser.Graphics and all should be fine then.

photonstorm added a commit that referenced this issue Dec 17, 2014
…the docs are now correct re: diameter not radius (thanks @ethankaminski #1454)
@chrispine
Copy link

Just out of curiosity, why does Phaser.Circle use the diameter, when just about everyone everywhere uses radius to describe the size of a circle? HTML5 Canvas does. PIXI.Circle does. This feels like a mistake. :-/

@chrispine
Copy link

It's also inconsistent with how Phaser.Graphics.drawEllipse() works. Seems like it's just asking for confusion.

@photonstorm
Copy link
Collaborator

That comes from PIXI, not Phaser.

It is what it is. I've no intension of changing it for any v2 release. v3 yes.

@chrispine
Copy link

Understood @photonstorm and I wouldn't expect it to change for a v2 release.

But does this mean you are already planning on changing it to "radius" for v3? Because that would be awesome.

The reason I'm concerned about this is that I'm writing a book about learning to make games geared toward a beginner audience, and I'm using Phaser throughout the book (because I ❤️ Phaser). Newbies are often struggling with lots of new concepts, so providing as much consistency as possible is important. Since arcs and ellipses use radii, it would be great if circles did, too. One less thing to trip up a newbie. 😃

@pnstickne
Copy link
Contributor

@chrispine Maybe the documentation could have a 'warning' or 'note' attached to it about the possible 'unexpected' behavior - and such could pointed out in the book as well.

A PR for this should be simple; but it's not on my plate.

@photonstorm
Copy link
Collaborator

Yes it will be conformed to for v3. I could add a note to the docs, but by the time you're reading that it's already clear what the constructor expects.

Personally I have always hated the fact it's the radius - I've lost count of the number of times in Flash I saw it given as x / 2 in the constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants