canvas class javascript

Question

I'm having trouble with my javascript code. I'm trying to create a moving set of circles where each circle has their own attributes. So far I've managed to input all the needed values into an array, but I can't figure out how to use them properly for drawing on canvas.

Here's the javascript:

var radius = 10;
var step = x = y = 0;
var r = g = b = 255;
var circleHolder = [];
var loop = setInterval(function(){update();}, 30);

function Circle(x, y, radius, r, g, b)
{
    this.x = x;
    this.y = y;
    this.radius = radius;
    this.r = r;
    this.g = g;
    this.b = b;
    circleHolder.push(this);
}

Circle.prototype.draw = function()
{
    Circle.prototype.ctx = document.getElementById("MyCanvas").getContext("2d");
    Circle.prototype.ctx.clearRect(0,0,720,720); // clear canvas
    Circle.prototype.ctx.beginPath();
    Circle.prototype.ctx.strokeStyle = "rgb("+ this.r +", "+ this.g +", "+ this.b +")";
    Circle.prototype.ctx.arc(this.x, this.y, this.radius, 0, 2 * Math.PI);
    Circle.prototype.ctx.stroke();
}

Circle.prototype.update = function ()
{
    step += .02;
    step %= 2 * Math.PI;
    this.x = parseInt((Math.sin(step)) * 150) + 360;
    this.y = parseInt((Math.cos(step)) * 150) + 360;
    this.radius += 16;
    if (this.radius > 200)
    {
        for (i in circleHolder)
        {
            if (circleHolder[i]==this)
            {
                circleHolder.splice(i, 1);
            }
        }
    }
}

function update()
{
    var ci = new Circle(x, y, radius, r, g, b);
    for (i in circleHolder)
    {
        ci = circleHolder[i];
        ci.update();
        ci.draw();
    }
}

I'm pretty sure my problem lies within update() {} but I can't figure out how to do it properly.

---

EDIT: Okay, I've got it working with some changes! Check this Fiddle! I'm getting "ci not defined" error in the console though, and it has a strange bug: Changing the "if (this.radius > 128)" to higher integer it will make the circles spin faster, I don't know why. If you want you can try to change it to 256 and see what happens.

for (var i=0; i < allCircles; i++)
        {
            ci = circleHolder[i]; <----- This is causing the error
            ci.update();
            ci.draw();
        }

Answer 1

For starters, unless there's something else going on, outside of this code:

You are using for ... in ... on an array, for-in is for objects, when used on arrays, most browsers will include methods like .splice and .forEach, and not just the numeric 0...n index.

function splice () {}.draw(); doesn't end well.

Also, what is the colour of your page's background? You're setting the rgb colour of each circle to 100% white. You're also clearing the canvas... ...which might well mean that the whole thing is transparent. So if you've got a transparent canvas, white circles and a white background, chances are great you're not going to be seeing anything at all, if this is even working without spitting out an error.

It might make a lot more sense to move your logic around in a way that lets you follow what's going on.

If you make a circle constructor, don't have it do anything but make a new circle.

Inside of your update, create a circle.
THEN put it inside of your circle collection (not in the circle constructor).

In a large application, you will typically call update on ALL objects, and then call draw on ALL objects, rather than updating and drawing one at a time. Imagine a game that didn't bother to check if you had been hit by a bullet before drawing you and letting you move, for instance.

So inside of your loop, you should have an update and a draw. Inside of the update, create your circles add them to the list and update the positions of them. Inside of the draw, draw the circles.

In the future, this will give you the benefit of having things like collision-detection, without having to redraw everything, multiple times per frame.

Also, don't do DOM-access inside of a function that's going to be called many, many times (Circle.draw). That will demolish your framerate in the future.

Instead, pass the function a dependency (the canvas).

// inside of the main game's scope
var screen = document.getElementById(...).getContext("2d");


// inside of your new draw function, in the animation/game loop
var i = 0, allCircles = circleHolder.length, currentCircle;

for (; i < allCircles; i += 1) {
    currentCircle = circleHolder[i];
    currentCircle.draw(screen);
}

Answer 2

it's not 100% clear to me what you're trying to do, but I tried to fix the main problem

One problem is your for loop.. you shouldn't use for in for arrays, do this instead:

for (var i=0 ; i<circleHolder.length ; i++)
{
    ci = circleHolder[i];
    ci.update();
    ci.draw();
}

see this fiddle

Also I moved your get context and other things that should happen only once into the constructor, instead of having it in the update function.

You're also clearing the canvas before each draw, so the it will only show the last drawn circle per frame. (if you remove the clearRect it looks like one of those old spirographs).

You were also drawing the circles with (255,255,255)(white) so it wasn't showing until the color was changed.

Edit:

Really there are a few problems with this code:

The context shouldn't be inside a circle class if you plan on having many of them.

You should have some object which contains the canvas/context and an array of all circles.

Then have that object manage the updating/drawing.