0

tl;dr: my function doesn't run while I've got setInterval() running another function every 3 seconds.

I'm making a text-based gardening game that runs "plant()" when I type plant. I've also got a setInterval(updatePlots, 3000) going.

both of these functions work fine on their own, but when I try to run plant() while setInterval() is going, it brings up the error Uncaught TypeError: plant is not a function

(I know it's the setInterval() because I tested planting without it running, and it worked fine.)

what I tried (didn't work):

if (command == "plant") {
  clearInterval(timer);
  plant(a, b);
  var timer = setInterval(updatePlots, 3000);
}

I'm not really sure what code I've got to show, since it seems more of a fundamental problem than single-line error... but here it is.

function updatePlots() {
  fullplots = [];
  for (i = 0; i < plots.length; i++) {
    if (plots[i].length) {
      fullplots.push(i);
    }
  }

  for (i = 0; i < fullplots.length; i++) {
    plant = plots[fullplots[i]][0];
    status = plots[fullplots[i]][1];
    growth = plots[fullplots[i]][2];

    if (growth < 100) {
        plots[fullplots[i]][2]++;
    }
  }

  if (document.getElementById('plots').style.display == 'block') {
    getPlots();
  }
}

...

function processTwo(command, a) {
  if (command == 'plant') {
    clearInterval(timer);
    console.log('about to plant 1'+a);
    plant(1, a);
    var timer = setInterval(updatePlots, 3000);
  }
  else { createError() }
}

update: solved!

  • 1
    If you would, please post all the code relevant to the question *in the question itself* - don't hide it behind a link. You shouldn't tell potential helpers who would otherwise love to help that they have to navigate offsite just to have an idea of what you're working with. If the link breaks, the question could be rendered useless to future readers. Please edit your code into the question in a [MCVE], or the question might get closed, thanks. – CertainPerformance Jan 18 '19 at 08:18
  • ah, sorry bout that, i'll fix it now – citywatcher Jan 18 '19 at 08:23
  • You are redefining plant in statement `plant = plots[fullplots[i]][0];` in `script.js`, Hence after first execution for `updatePlots` method you are getting the error – Satpal Jan 18 '19 at 08:24
  • oh my god. @Satpal you are glorious – citywatcher Jan 18 '19 at 08:27
  • You really need to learn about variable scope in JS and why global variables are evil. If you do know about these topics, you should really refactor your code on behalf of variable scope. – yunzen Jan 18 '19 at 08:28
  • you're right, my code needs a tidyin. thanks so much – citywatcher Jan 18 '19 at 08:36

2 Answers2

1
function updatePlots() {
  // this way, the global plant function is not overwritten
  // @see JavaScript variable scope
  var plant;
  // You might want to 'var' your other local (?) variables, too
  // var fullplots, i, status, growth;
  fullplots = [];
  for (i = 0; i < plots.length; i++) { //get fullplots
    if (plots[i].length) {
      fullplots.push(i);
    }
  }

  for (i = 0; i < fullplots.length; i++) {
    // at this line of code you overwrite the global plant, which is not a function anymore then
    plant = plots[fullplots[i]][0];
    status = plots[fullplots[i]][1];
    growth = plots[fullplots[i]][2];

    if (growth < 100) { //increment
        plots[fullplots[i]][2]++;
    }
  }

  if (document.getElementById('plots').style.display == 'block') {
    getPlots();
  }
}
yunzen
  • 32,854
  • 11
  • 73
  • 106
1

The problem arises in the updatePlots method called by setInterval. What you're doing in there is assigning a new value to "plant"

plant = plots[fullplots[i]][0];

Then, when plant is called, it points to the new value you assigned to it in updatePlots instead of the function you originally had. You have to declare a new variable in updatePlots to avoid changing the plant method. I'd use a different name to avoid confusion.

  • While your analysis is correct, the solution is not good. What the asker should do is encapsulate the inner variables of his functions by using `var`, `let` or `constant` – yunzen Jan 18 '19 at 08:31
  • Isn't that declaring a new variable? Even if it's the same identifier, the fact that it's in a different scope makes it a new variable. Correct me if I'm wrong here – tylermcwilliams Jan 18 '19 at 08:35
  • It is a new variable. The difference is, it has a local scope and won't pollute the global scope (unobtrusive) – yunzen Jan 18 '19 at 08:37
  • Right. So what is it that I said that is wrong then? – tylermcwilliams Jan 18 '19 at 08:38
  • It's not wrong and I won't downvote you on your answer. Your answer is completely correct. But it is not a good advise to create so many global variables. Global variables are evil and should be avoided at all costs. See https://stackoverflow.com/a/2613647/476951 – yunzen Jan 18 '19 at 08:42
  • Hmm i think we're misunderstanding each other then. I agree that global variables shouldn't be used, that's why I told him to declare a new variable in updatePlots – tylermcwilliams Jan 18 '19 at 08:54
  • That's totally fine. But using scoped variables is the better alternative – yunzen Jan 18 '19 at 08:54
  • Aaah, I've overread `declare` and the implications of that. Of course declare means using `var` or `let` or `const`. But you should state that in your answer, too, because it is like a hidden meaning and might not be clear to the asker or future readers. – yunzen Jan 18 '19 at 08:56