Reputation: 33996
I need to queue snippets and execute them all at a given time (Once a second). The snippets are actually player actions like jump(), attack(), walk('left') etc.
When a user presses a key I need to queue his action and execute all actions once a second.
Right now my approach, for lack of a better idea is to add each snippet to an array and loop through it with eval(). This is my code:
var queue = [];
// On player or AI action
queue.push('attack()'); // Could be walk('left'), jump() etc.
// On new frame
for(var i=0;i<queue.length;i++){
eval(queue[i]);
}
queue = [];
I'm sure my approach is terrible but I wanted to clarify what I want to do, I suppose a problem such as this is not so uncommon. Any info/ideas?
Upvotes: 1
Views: 107
Reputation: 198436
It is terrible - eval
is evil. Instead, use closures:
var queue = [];
queue.push(function() { attack(); });
while (queue.length) {
queue.shift().call();
}
This is a general case, but it can in this case be simplified: function() { attack(); }
is a more complicated and slower way to write attack
, so queue.push(attack)
would work just as well (if you have a a function attack() { ... }
defined somewhere accessible). You can't do that, obviously, if your function takes parameters (or rather, if each function takes different parameters; you can supply unified parameter list in the call(thisobj, param)
call).
EDIT for your additional query:
In the code that you wrote, this should have captured the variable into the closure; if you change the variable, that value will be what you'll get when you execute it. If you got undefined
, I guess you executed closerEnemy = undefined
somewhere later. This is a common source of errors when, for example, people try to bind a click handler to several elements in a loop, and use the loop counter in the handler - the handler captures the counter and not its value, and will always later evaluate to the number of elements (the last value it was left with when the loop finished).
To capture the value and not the variable, use this trick:
(function(capturedCloserEnemy) {
queue.push(function() {
attack(capturedCloserEnemy);
});
})(closerEnemy);
(The variables are named differently just for readability; they could all be named the same and it would not matter, because shadowing.)
Upvotes: 3
Reputation: 119867
The problem with your approach is that:
You are using eval
You have already heard about the risks in using eval. If not, check this document from MDN.
You are using a loop
When using the loop, JavaScript tries to finish the loop as fast as possible. This will cause blocking in the UI. This also means that when you start looping in this manner, your animation will appear to only move at the first animation, freeze, and end at the last animation in your queue.
As a remedy for both issues, why not push to the queue a reference to the function that you intend to execute. Also, to prevent UI blocking, use timers instead:
var queue = [];
//actions as functions
function attack(params){...}
function block(params){...}
function walk(params){...}
//our animation timer
var animationTimer = setInterval(function(){
//remove the action from the queue and execute
var nextAction = queue.shift();
//if we shifted something, execute
if(nextAction){
nextAction.call();
}
},1000);
//to insert into the queue, push the reference of the function instead.
queue.push(attack);
Upvotes: 1