iiYO Community

Hi!

I'm not very good at js, but after reading

Code:

<animation>

Code:

<imagepack>

and

Code:

<curtain>

sources I wrote this: https://github.com/lolbot-iichan/WebStor...5440b66271

It is a first version of particle generator that if future could be used for snow, cherry blossoms, autumn leaves, explosions, fountains, whatever. There are still lots of things to implement: not creating all the particle at once (first seconds of animation are strange), add particle image loading to initial progress bar, gravity support and so on. It also supports both text (like '*') and image source for particle.

However, there is one more thing I can't get. There is a problem with save/restore I cannot find for hours. Can you please take a look at my ugly code at say if there are any obvious mistakes? Full code review would be great, but I'm not sure if you have time for things like that.

First of all, nice idea and the snow looks pretty good.

I think your particles don't start again after restoring because you check whether isRunning is true in start() and return from the function when it is. You either need to set isRunning to false before calling start() in restore() or drop the check / change the code somehow.

I found another problem though: the particles use up almost 100% of my CPU (and that is a core i7 quad we're talking about here Wink

)... I think you shouldn't use Animation for this because it relies on timers being returned from the callbacks. When I use requestAnimationFrame() instead it seems to be much better. requestAnimationFrame() is a function that takes a callback and executes it once before the next re-paint of the rendering engine of the browser. Optimally it runs with 60 FPS. Also, when using requestAnimationFrame() no animations run when the browser tab is not visible. You can use it like this:

<pre>

Code:

function myDrawingFunction (timestamp) {

    // ...do your calculations here

    requestAnimationFrame(myDrawingFunction);

}

</pre>

Some more things about the code I think could be improved:

- Don't omit the braces for single-line ifs or elses. This can lead to really nasty bugs that can take days to find (no kidding, I had this problem already with other people's code, it's a maintenance nightmare). And it can be quite difficult for another person to find out what you really intended with your code, especially if you also get the indentation wrong...

- Don't use lookups in loop conditions; instead, use another local variable for this. This is just a minor thing, but it can improve performance --> for (var i = 0, len = this.nParticles; i < len; i += 1)

- In JavaScript, variables declared using the var keyword have function scope, we say that they get "hoisted". It basically means you should declare all your variables at the top of a function (like in C) even if you don't initialize them.

- Put a comment before the ParticleSnow() function describing the parameters of the function. I wouldn't know how to use it from just looking at the names you gave the parameters.

- If you want me to add your extension to the official repository, you should use the same style I'm using in the rest of the code, e.g. put spaces around operators like + and * and =, use Allman-style indentation for everything except when you return objects from a function (using Allman style is controversial in JS by the way (because of automatic semicolon insertion), and I personally switched to K&R some time ago, but for consistency it's probably better to use the same style as in the rest of the code). Use camelCase for variable names.

_lb_

Guest