Handling of repeats with browser tab sleep #564

MasatoMakino · 2020-10-03T04:52:14Z

Related Issues

#557
When the browser tab sleeps, the delay of repeated tweens is not keep

Purpose of this Pull Request

This fixes handling of repeats.

Details of the changes

When interval for TWEEN.update() is more than one repeat

Continuous elapsed
Multiple tween._repeat subtractions

This is a test to confirm that the tween._repeat has been successfully reduced. I'm worried about whether I should test private variables. But I added this test because I would change the subtraction process for tween._repeat.

Tests for a TWEEN.update() interval of more than 1 repeat duration.

calculateElapsed() outputs a continuous elapsed, even if you give it a multi-repeat time interval.

trusktr · 2020-10-19T08:14:05Z

Thanks for doing this! Will review it soon.

trusktr · 2023-04-23T08:18:34Z

src/Tween.ts

+			}
+
+			const timesRepeated = Math.trunc(elapsedTime / durationAndDelay)
+			const timeIntoCurrentRepeat = elapsedTime - timesRepeated * durationAndDelay


@MasatoMakino Hello! I renamed some vars for easier reading. F.e. I renamed elapsedTime to timeIntoCurrentRepeat.

I agree with what @trusktr said.

I feel like the naming isn't quite right yet. 🤔 EDIT: On second thought, I think naming is fine now.

src/Tween.ts

trusktr · 2023-04-23T18:25:01Z

src/Tween.ts

 				if (isFinite(this._repeat)) {
-					this._repeat--
+					this._repeat -= completeCount


Down below, regarding the chained tweens, there is this:

this._chainedTweens[i].start(this._startTime + this._duration)

Does this need to be updated too? Should it now be only

this._chainedTweens[i].start(this._startTime)

because we already advanced _startTime to where the next repeat would have been?

This chained tween logic happens on the next update right? I wonder if it needs to happen immediately in the updated where _repeat > 0, f.e. when we run this._repeat -= completeCount and _repeat becomes 0.

Looks like we should add a unit test for repeat + chain used together.

EDIT: Aha! This issue #397 shows the problem of chained tweens starting on the next update.

This is getting complicated. I think that later we can remove some complexity from here, and instead adding something like a Timeline feature would make everything a lot easier to understand:

#647

Another issue with repeat(N) is that we cannot make the time go backwards, because we lose the initial _startTime every time we increment it. Timeline would make it easy to go forwards/backward anywhere on the timeline.

It is complicated to use "repeat" and "chain" at the same time. I agree with the need for additional unit tests.

* main: (107 commits) v22.0.0 examples: add an example showing we can import into Node.js projects in ESM format fix import for Node.js CommonJS projects v21.1.1 eslint started having an error with the latest environment, so delete it for now chore: ignore output .js files in typescript example examples: add two example projects, one plain JS modules, the other TS compiled to plain JS modules examples: simplify the createGraph function, and make it crispier on devices with devicePixelRatio higher than 1 refactor: remove requestAnimationFrame polyfill, it has been out for a long time in all browsers update formatting update prettier v21.1.0 Update package.json, remove duplicate entry so we can merge fix: Use type definition in ESM change maintainers list and remove dead link Add missing types file to package.json exports docs: update tutorial link fix(package.json): fix export map types Fix CJS entry point v21.0.0 ...

trusktr · 2024-01-15T04:32:36Z

Merged! All existing tests pass, plus the new tests pass, so this is an improvement even if we know it is not working with all features.

What we should do now is start to prevent more complexity, and next we should focus on simplifying, and then Timeline.

trusktr · 2024-01-15T04:45:11Z

Ah shoot, I'm doing some manual testing, and the 13_relative_start_time example broke. Will revert. But I think 13_relative_start_time will provides some clues as to what is missing

trusktr · 2024-01-15T04:56:49Z

Hmmm, actually, the 13_relative_start_time example uses .delay(-500). It is not intuitive what "delay negative 500" means, and the improvement of this PR is a lot more significant.

I'm going to change that example so it only uses positive values, "remove" the undocumented "feature", and keep the change from this PR because it is more valuable. The next release will be a major version bump.

trusktr · 2024-01-15T07:17:11Z

Released in v23.0.0

MasatoMakino added 4 commits October 2, 2020 16:03

add : test for tween._repeat

4fbced8

This is a test to confirm that the tween._repeat has been successfully reduced. I'm worried about whether I should test private variables. But I added this test because I would change the subtraction process for tween._repeat.

add : tests for browser sleep

21cc033

Tests for a TWEEN.update() interval of more than 1 repeat duration.

change : calculate elapsed

f6c8ac1

calculateElapsed() outputs a continuous elapsed, even if you give it a multi-repeat time interval.

change : Subtracting tween._repeat and updating tween_startTime.

bc5aad5

trusktr added this to Opened Pull Requests in Pull Requests via automation Oct 19, 2020

rename variables for readability

2a4195a

trusktr reviewed Apr 23, 2023

View reviewed changes

src/Tween.ts Show resolved Hide resolved

trusktr reviewed Apr 23, 2023

View reviewed changes

trusktr linked an issue Apr 23, 2023 that may be closed by this pull request

When the browser tab sleeps, the delay of repeated tweens is not keep #557

Open

trusktr mentioned this pull request Apr 23, 2023

chained tweens cannot be stopped in onComplete callback (chained tweens start in the NEXT update after onComplete) #397

Closed

trusktr changed the base branch from master to main July 8, 2023 06:47

trusktr changed the base branch from main to test-557-tab-sleep January 15, 2024 04:00

trusktr merged commit bb608dd into tweenjs:test-557-tab-sleep Jan 15, 2024

Pull Requests automation moved this from Opened Pull Requests to Done Jan 15, 2024

trusktr temporarily deployed to github-pages January 15, 2024 04:33 — with GitHub Pages Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handling of repeats with browser tab sleep #564

Handling of repeats with browser tab sleep #564

MasatoMakino commented Oct 3, 2020

trusktr commented Oct 19, 2020

trusktr Apr 23, 2023

MasatoMakino Apr 23, 2023

trusktr Apr 23, 2023 •

edited

trusktr Apr 23, 2023 •

edited

trusktr Apr 23, 2023 •

edited

trusktr Apr 23, 2023 •

edited

trusktr Apr 23, 2023

MasatoMakino Apr 24, 2023 •

edited

trusktr commented Jan 15, 2024

trusktr commented Jan 15, 2024 •

edited

trusktr commented Jan 15, 2024 •

edited

trusktr commented Jan 15, 2024

Handling of repeats with browser tab sleep #564

Handling of repeats with browser tab sleep #564

Conversation

MasatoMakino commented Oct 3, 2020

Related Issues

Purpose of this Pull Request

Details of the changes

trusktr commented Oct 19, 2020

trusktr Apr 23, 2023

Choose a reason for hiding this comment

MasatoMakino Apr 23, 2023

Choose a reason for hiding this comment

trusktr Apr 23, 2023 • edited

Choose a reason for hiding this comment

trusktr Apr 23, 2023 • edited

Choose a reason for hiding this comment

trusktr Apr 23, 2023 • edited

Choose a reason for hiding this comment

trusktr Apr 23, 2023 • edited

Choose a reason for hiding this comment

trusktr Apr 23, 2023

Choose a reason for hiding this comment

MasatoMakino Apr 24, 2023 • edited

Choose a reason for hiding this comment

trusktr commented Jan 15, 2024

trusktr commented Jan 15, 2024 • edited

trusktr commented Jan 15, 2024 • edited

trusktr commented Jan 15, 2024

trusktr Apr 23, 2023 •

edited

trusktr Apr 23, 2023 •

edited

trusktr Apr 23, 2023 •

edited

trusktr Apr 23, 2023 •

edited

MasatoMakino Apr 24, 2023 •

edited

trusktr commented Jan 15, 2024 •

edited

trusktr commented Jan 15, 2024 •

edited