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

Timer Bug Fix - Fixed indeterminate behavior of new timers. #94

Merged
merged 1 commit into from
Apr 8, 2018

Conversation

oniietzschan
Copy link
Contributor

@oniietzschan oniietzschan commented Feb 16, 2018

Fixed bug where behavior of new timers added during iteration through timer handles was indeterminate.

On master, consider that while iterating through self.functions, we use pairs.

  -- timer.lua on master

  for handle in pairs(self.functions) do
    -- <update handle...>
  end

So, if we have a timer that does something like this:

local timerOne = Timer.after(1, function()
  local timerTwo = Timer.after(1, function() print('does something else') end)
end

Now, if we run `Timer.update(1), there are two possibilities.

A. timerTwo is updated later.

  1. timerOne will finish.
  2. timerOne will execute and create timerTwo, it will be added to somewhere considered "before" timerOne in pairs(self.functions).
  3. timerTwo will not be updated in this call of Timer.update.

B. timerTwo is updated immediately.

  1. timerOne will finish.
  2. timerOne will execute and create timerTwo, it will be added to somewhere considered "after" timerOne in pairs(self.functions).
  3. timerTwo will be updated in this call of Timer.update, thus printing "does something else" immediately.

To fix this issue, before starting to iterate through self.functions, we gather together all of the timer handles which currently exist, so that new timers will not be updated until the next call of Timer.update().

@vrld vrld merged commit dde34f9 into vrld:master Apr 8, 2018
@vrld
Copy link
Owner

vrld commented Apr 8, 2018

Thanks again :)

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

Successfully merging this pull request may close these issues.

2 participants