Added play/pause functionality #644
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds a simulationPlaying boolean, UI play/pause checkbox, keyboard space toggle, toggleSimulationPlaying() utility, and runtime guards that prevent clock ticks and play() execution when simulationPlaying is false. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/simulator/src/engine.js (2)
404-410: Paused guard should also flush the simulation queue to match “discard queued updates” promiseRight now, play() returns before clearing pending work. This will let already-queued resolves run immediately after resuming, contradicting the PR description (“including updates already queued”). Reset the queue before returning while paused.
Apply this diff:
if (errorDetected) return // Don't simulate until error is fixed if (loading === true) return // Don't simulate until loaded - if (!simulationArea.simulationPlaying) return // Don't simulate when paused + // Don't simulate when paused — also discard any pending work as per PR requirements + if (!simulationArea.simulationPlaying) { + simulationArea.simulationQueue.reset() + return + }
549-571: Resume may no-op: update() clears updateSimulation even when pausedupdate() unconditionally sets updateSimulation=false after calling play(). If the app is paused, play() exits early and we still clear the flag. Toggling Play back on (per current summary) only schedules an update; it doesn’t guarantee updateSimulation becomes true again, so the simulation may not actually resume.
Gate clearing the flag behind an actual simulation run.
Apply this diff:
- if (updateSimulation) { - play() - } + const didSimulate = updateSimulation && simulationArea.simulationPlaying + if (didSimulate) { + play() + } @@ - updateSimulationSet(false) + if (didSimulate) updateSimulationSet(false)Follow-up (alternative/complement outside this file):
- When turning Play on, explicitly set updateSimulationSet(true) in toggleSimulationPlaying().
- When turning Play off, also clear the queue there for consistency with the behavior added above.
Example (in utils.ts):
export function toggleSimulationPlaying() { simulationArea.simulationPlaying = !simulationArea.simulationPlaying; if (simulationArea.simulationPlaying) { updateSimulationSet(true); scheduleUpdate(1); } else { simulationArea.simulationQueue.reset(); } updateCanvasSet(true); }
🧹 Nitpick comments (2)
src/simulator/src/interface/simulationArea.ts (1)
9-9: New playback state flag looks good; clarify interplay with clockEnabled.Adding
simulationPlaying: booleanis a clean, minimally invasive way to gate simulation. Consider documenting how it differs from and composes withclockEnabled(e.g., both must be true to tick), so future contributors don’t conflate the two.src/components/Panels/PropertiesPanel/ModuleProperty/ProjectProperty/ProjectProperty.vue (1)
39-51: UI toggle works; consider controlled update flow and a11y label.Current pattern uses
:checkedbound to state and@changeto toggle. It works, but two refinements help:
- Accessibility: add an explicit label for screen readers.
- Optional: move from “toggle on change” to an idempotent setter, so alternate callers (keyboard, future APIs) can pass a desired state without guessing.
Minimal improvement (adds ARIA without changing behavior):
<input type="checkbox" class="objectPropertyAttributeChecked" name="changeSimulationPlaying" :checked="simulationArea.simulationPlaying" - @change="toggleSimulationPlaying" + aria-label="Play or pause the simulation" + title="Play/Pause (Space)" + @change="toggleSimulationPlaying" />Optional refactor (if you introduce
setSimulationPlaying(next: boolean)in utils):
- Bind
:checked="simulationArea.simulationPlaying"- Replace handler with
@change="(e) => setSimulationPlaying((e.target as HTMLInputElement).checked)"This avoids accidental double-toggles if other code also flips the flag.
Also applies to: 121-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/components/Panels/PropertiesPanel/ModuleProperty/ProjectProperty/ProjectProperty.vue(2 hunks)src/simulator/src/engine.js(1 hunks)src/simulator/src/interface/simulationArea.ts(1 hunks)src/simulator/src/listeners.js(2 hunks)src/simulator/src/simulationArea.ts(1 hunks)src/simulator/src/utils.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/simulator/src/engine.js (2)
src/simulator/src/simulationArea.ts (2)
simulationArea(70-70)simulationArea(71-71)src/simulator/src/embedListeners.js (1)
simulationArea(104-104)
src/simulator/src/utils.ts (1)
src/simulator/src/engine.js (2)
updateCanvasSet(114-116)scheduleUpdate(466-485)
src/simulator/src/listeners.js (1)
src/simulator/src/utils.ts (1)
toggleSimulationPlaying(54-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
src/simulator/src/utils.ts (1)
44-45: Correct early-return to pause clock ticks.The extra guard in
clockTick()cleanly halts ticks while paused without affecting layout mode or error handling. Nice.src/simulator/src/simulationArea.ts (1)
12-12: Good default: start in playing state.Initializing
simulationPlaying: truealigns with current simulator behavior (auto-running after load). If you adopt interval clearing on pause, ensuresetup()or resume flow re-arms the clock viachangeClockTime(timePeriod).Please verify resume after page load and after toggling pause→play still ticks by confirming
ClockIntervalis non-null and increasing cycles.src/simulator/src/engine.js (1)
404-407: Simulation resume, space‐key handling, and clockTick guards verified—no changes needed
- toggleSimulationPlaying (src/simulator/src/utils.ts: lines 54–59) flips simulationArea.simulationPlaying and immediately calls scheduleUpdate(1), so resuming never “sticks.”
- Space key handler (src/simulator/src/listeners.js: lines 563–567) invokes toggleSimulationPlaying() then e.preventDefault(), correctly suppressing scrolling.
- clockTick (src/simulator/src/utils.ts: lines 1–8) early-returns when simulationPlaying is false, so no further scheduleUpdate calls fire while paused.
| :checked="simulationArea.simulationPlaying" | ||
| @change="toggleSimulationPlaying" | ||
| /> |
2 participants
Fixes #564
Describe the changes you have made in this PR -
Screenshots of the changes (If any) -
Screencast.from.08-23-2025.10.28.17.PM.webm
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
New Features
Improvements