Show playback #221
Show playback #221
Conversation
Co-authored-by: Gabriel Burbach <[email protected]>
Needed to downgrade due to this issue I encountered when installing Framer Motion: yarnpkg/yarn#7807
Co-authored-by: Gabriel Burbach <[email protected]>
Co-authored-by: Colin Wong <[email protected]>
* Editor scaled 1:60 Messed around with editor scaling to watch the bots move. Is it correct? probably not Should be reviewed by jason before being merged back into lol-bs may also just want to delete this later idk. * style: format * refactor: store scale factor as constant * fix: readd ts expect error directive --------- Co-authored-by: ymmot239 <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR implements a new show designer interface (/editor) with timeline-based robot choreography capabilities and adds new robot movement commands for spline-based pathfinding and show playback.
- Implements a comprehensive editor interface with grid-based spline editing, timeline management, and real-time preview
- Adds new robot TCP packet types for advanced movement: cubic/quadratic splines, spin commands, and tick-based driving
- Extends server API with show execution capabilities and robot configuration updates
Reviewed Changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/utils/tcp-packet.ts | Adds new packet types for spline movement and spin commands |
| src/server/robot/robot.ts | Implements methods for advanced robot movement commands |
| src/server/command/move-command.ts | Creates command classes for spline movement and timing |
| src/common/spline.ts | Adds spline evaluation and manipulation utilities |
| src/common/show.ts | Defines showfile schema and timeline event types |
| src/client/editor/* | Complete editor interface implementation with timeline and grid editing |
| src/client/router.tsx | Adds editor route |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/server/robot/robot.ts
Outdated
| console.log(tileDistance); | ||
| await tunnel.send({ type: PacketType.DRIVE_TANK, left: 1, right: 1 }); |
There was a problem hiding this comment.
The sendDrivePacket method ignores the tileDistance parameter and always sends a hardcoded DRIVE_TANK packet with left: 1, right: 1. This breaks the expected behavior of driving a specific distance.
| console.log(tileDistance); | |
| await tunnel.send({ type: PacketType.DRIVE_TANK, left: 1, right: 1 }); | |
| await tunnel.send({ type: PacketType.DRIVE_TANK, left: tileDistance, right: tileDistance }); |
| "startHeading": 90 | ||
| }, | ||
| "robot-15": { |
There was a problem hiding this comment.
The attributes for robot-15 were removed, but this could be a breaking change if these motor multipliers were being used to calibrate that specific robot. Consider documenting why these calibration values were removed.
| <Button | ||
| onClick={async () => { | ||
| post("/do-big", { show: JSON.stringify(show) }); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
This appears to be debug code with a hardcoded API endpoint. Consider removing this before production or making it conditional based on a debug flag.
| <Button | |
| onClick={async () => { | |
| post("/do-big", { show: JSON.stringify(show) }); | |
| }} | |
| /> | |
| {/* Debug button removed */} |
| // @ts-expect-error: chessbots client is a CommonJS module, but this library is a ES Module, so we need to tell TypeScript that it's okay | ||
| import { decode as cborDecode } from "cbor-x"; |
There was a problem hiding this comment.
Using @ts-expect-error to suppress module system compatibility issues is fragile. Consider configuring TypeScript module resolution properly or using dynamic imports for better type safety.
| // @ts-expect-error: chessbots client is a CommonJS module, but this library is a ES Module, so we need to tell TypeScript that it's okay | |
| import { decode as cborDecode } from "cbor-x"; | |
| // Use dynamic import to handle CommonJS module compatibility for cbor-x | |
| async function getCborDecode() { | |
| const cborX = await import("cbor-x"); | |
| return cborX.decode; | |
| } |
| timelineLayerIndex < show.timeline.length; | ||
| timelineLayerIndex++ | ||
| ) { | ||
| // TODO: make a way to map robot ids to timeline layers, so we can choose which physical robot to use for each timeline layer |
There was a problem hiding this comment.
This TODO indicates a significant limitation where robot assignment to timeline layers is based on array index rather than explicit mapping. This could cause unexpected behavior if robots disconnect or are added/removed.
5 participants
This PR:
/editor) to support designing showsthis code needs HEAVY refactoring which we will do in future issues because these branches have been open for too long 👍🏾