Circular Obstacles #205
Circular Obstacles #205
Conversation
shueja
left a comment
shueja
left a comment
There was a problem hiding this comment.
obstacles are on a z-index between trajectories/initial guess lines and constraint targets/waypoints. Intended?
Dragging an obstacle snaps the center of the obstacle to the mouse. This was acceptable with waypoints, since the drag targets were small. Not so here, especially with large obstacles that only need to move a small amount.
The amount of code duplication here is partially inevitable, but could show need for some helper functions that would work on both the obstacles array and the waypoints array.
The small drag handle for radius is annoying to work with. Could the entire outer border of the circle be a resize handle? The existing handler for the drag event already supports this, the target for clicking just needs to be bigger.
The bright green for selected obstacles is visually jarring. Consider using var(--select-yellow) on the border and leaving the internals red (perhaps a more transparent red).
Should document schema 0.1.2 use the circleObstacles key instead of obstacles to avoid an annoying rename in the auto-upgrader later?
There was a problem hiding this comment.
Revert these changes before merge
src-tauri/src/main.rs
Outdated
| path_builder.set_bumpers(config.bumperLength, config.bumperWidth); | ||
|
|
||
| for o in circleObstacles { | ||
| println!("{:?}", o); |
| this.context.model.uiState.setSelectedNavbarItem(9); | ||
| }} | ||
| > | ||
| {React.cloneElement(<Circle></Circle>, { |
There was a problem hiding this comment.
The icon here is a circle, but is a circular (/) sign in the navbar. Use the latter here. Replace <Circle></Circle> with ObstacleData["CircleObstacle"].
| ", y: " + | ||
| this.props.obstacle.y.toFixed(2)} | ||
| </span> | ||
| <Tooltip disableInteractive title="Delete Waypoint"> |
There was a problem hiding this comment.
Tooltip here needs changing.
| className={styles.SidebarLabel} | ||
| style={{ display: "grid", gridTemplateColumns: "1fr auto auto" }} | ||
| > | ||
| {"Obstacle x: " + |
There was a problem hiding this comment.
Could there be a better way to distinguish obstacles? The coordinates in the title feels kind of clunky.
| pointerPos.x = event.x; | ||
| pointerPos.y = event.y; | ||
|
|
||
| this.props.obstacle.setX(pointerPos.x); |
There was a problem hiding this comment.
This snaps the obstacle center to the pointer, which does not work well on drag targets the size of the obstacle.
| /> | ||
|
|
||
| <Input | ||
| title="radius" |
There was a problem hiding this comment.
Consider leaving the title just r for visual lineup.
| WaypointScope, | ||
| } from "./ConstraintStore"; | ||
| import { SavedWaypointId } from "./previousSpecs/v0_1"; | ||
| import { SavedWaypointId } from "./previousSpecs/v0_1_2"; |
There was a problem hiding this comment.
This should actually be forward-exported from DocumentSpecTypes, but that's outside the scope of this PR
src/document/HolonomicPathStore.ts
Outdated
| }, | ||
| asSavedPath(): SavedPath { | ||
| let trajectory: Array<SavedTrajectorySample> = self.generated; | ||
| console.log( |
src/document/PathListStore.ts
Outdated
| import { HolonomicPathStore } from "./HolonomicPathStore"; | ||
| import { v4 as uuidv4 } from "uuid"; | ||
| import { ConstraintStores } from "./ConstraintStore"; | ||
| import { CircularObstacleStore } from "./CircularObstacleStore"; |
|
Approval is conditional on adjusting the dependency location of trajoptlib back to sleipnir's main now that SleipnirGroup/TrajoptLib#69 merged. Everything else looks good. |
2 participants
No description provided.