fix: prevent wires from breaking or adding extra nodes unexpectedly #616
Conversation
|
Warning Rate limit exceeded@blazethunderstorm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
""" WalkthroughThe Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@ThatDeparted2061 @Arnabdaz @niladrix719 pls review |
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/simulator/src/wire.ts (1)
1-270: Consider adding unit tests for the wire stability fixes.While all the changes look good and directly address the PR objectives, consider adding unit tests to verify:
- Wire type assignment for diagonal wires
- Deferred deletion behavior
- Hit detection with pixel tolerance
- Prevention of self-convergence
- Prevention of redundant node creation
This would help ensure these fixes remain stable in future updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/simulator/src/wire.ts(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/simulator/src/wire.ts (1)
src/simulator/src/node.js (1)
Node(150-1034)
🔇 Additional comments (7)
src/simulator/src/wire.ts (7)
23-23: Good type safety improvement!Explicitly typing the
typeproperty as a union type prevents invalid wire orientations and improves code maintainability.
44-46: Correct handling of wire orientation.The explicit
else ifcondition prevents diagonal wires from being incorrectly typed as horizontal, which aligns with the PR objective of preventing unexpected wire behavior.
57-59: Smart deferral of wire deletion.Using
setTimeoutto defer deletion prevents potential race conditions during event handling, which helps maintain wire stability during user interactions.
103-111: Improved hit detection with pixel tolerance.The 2-pixel threshold for coordinate comparison makes wire selection more user-friendly and prevents precision issues that could cause wires to break unexpectedly.
120-120: Critical fix preventing self-convergence.The early return prevents a wire from converging with its own nodes, which directly addresses the issue of adding extra nodes unexpectedly.
212-217: Clean parameter organization.Grouping related coordinate parameters improves code readability and aligns with the updated
checkAndCreateNodesignature.Also applies to: 221-226
237-251: Excellent fix preventing redundant node creation.The position checks before convergence prevent creating unnecessary nodes when the new node would be at the same location as an existing node. This directly solves the "adding extra nodes unexpectedly" issue mentioned in the PR objectives.
Fixes #610
Describe the changes you have made in this PR -
This pull request fixes a bug in the Vue simulator where wires connecting nodes would sometimes break or add extra nodes on their own when users clicked or dragged near them. The update improves the wire handling logic to keep connections stable and prevent unwanted changes, making the circuit layout cleaner and easier to use.
Screenshots of the changes (If any) -
After --
wirefixed.mp4
Before --
wiresnotfixed.mp4
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
Bug Fixes
Improvements