fix: stop Replay3DModel self-advancing frames when the parent drives them#915
Open
Anexus5919 wants to merge 1 commit into
Open
fix: stop Replay3DModel self-advancing frames when the parent drives them#915Anexus5919 wants to merge 1 commit into
Anexus5919 wants to merge 1 commit into
Conversation
|
@Anexus5919 is attempting to deploy a commit to the somiljain2024-4175's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Author
|
@Somil450 @diksha78dev Kindly have a review on this pr. Thanks! |
…them When a parent passes currentFrameIdx (ReplayScreen, which runs its own setInterval to advance frames), Replay3DModel's render loop also advanced the frame via onFrameChange. Two drivers then fought over the same index at different rates, with conflicting end behaviour (the interval stops at the last frame, the render loop wraps around). Only self-advance when the frame index is uncontrolled. When controlled, render the parent's currentFrameIdx directly so the parent is the single frame driver.
febee32 to
4633bab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Related Issue
Fixes #878
📝 Description
While the replay played, two drivers advanced the same
currentFrameIdx:ReplayScreen's 66mssetInterval(about 15fps, stops at the last frame) andReplay3DModel's render loop (about 8fps, wraps around) viaonFrameChange. They competed, so playback ran faster and more erratically than intended, with contradictory end behaviour (one stops, the other loops).🔹 What has been changed?
src/components/Replay3DModel.tsx:Replay3DModelnow self-advances frames only when the frame index is uncontrolled (isFrameControlled = externalFrameIdx !== undefined). When a parent controls the frame index, the render loop renders the parent'scurrentFrameIdxdirectly, so the parent (ReplayScreen's interval) is the single driver. The flag is also added to the render-loop effect's dependency list.🔹 Why are these changes needed?
Replay3DModela pure renderer when controlled preserves ReplayScreen's intended 15fps + stop-at-end behaviour, and keepsReplay3DModelself-driving when it is used standalone.🛠️ Type of Change
🧪 Testing
✅ Tests Performed
npx tsc --noEmit:Replay3DModel.tsxclean (the only pre-existing tsc errors are inexerciseEngine.ts, unrelated).npx eslint src/components/Replay3DModel.tsx: 0 problems, including noreact-hooks/exhaustive-depswarning (the new flag is in the dependency list).frameFloatRefnor callssetCurrentFrameIdx, and renders the parent'scurrentFrameIdx.🔹 Note on automated tests
No runtime unit test:
Replay3DModelis a Three.js/WebGL component that does not run under the vitest/jsdom environment. Verification is bytsc,eslint, and the path review above.🌐 Browsers Tested
Not applicable (playback timing/lifecycle).
📷 Screenshots / Demo (if applicable)
Not applicable.
📋 Checklist
💬 Additional Notes
Branched from latest
upstream/main(5464425). This touchesReplay3DModel.tsx, as do a few other replay PRs (#906, #907, and #879); they change different lines and are independently mergeable, with at most a trivial rebase for whichever lands later.