Skip to content

Spawn Intake/Ejector in the middle of the robot [SYNTH-204]#1372

Open
RoushilS wants to merge 6 commits into
devfrom
roushils/204/spawn-config-center
Open

Spawn Intake/Ejector in the middle of the robot [SYNTH-204]#1372
RoushilS wants to merge 6 commits into
devfrom
roushils/204/spawn-config-center

Conversation

@RoushilS

@RoushilS RoushilS commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Task

SYNTH-204

Make intake/ejector config intially spawn in the center.

Symptom

When Intake/Ejector is spawned for the first time, it is often distanced from the robot itself (sometimes underground), making the user configuration period take slightly longer.

Solution

Set gizmo to spawn at center after doing a check to make sure the transformation delta is not the intial one.

Verification


  • Ejector and Intake config spawn in the middle of the robot
  • This only happens the first time.

Before merging, ensure the following criteria are met:

  • All acceptance criteria outlined in the ticket are met.
  • Necessary test cases have been added and updated.
  • A feature toggle or safe disable path has been added (if applicable).
  • User-facing polish:
    • Ask: "Is this ready-looking?"
  • Cross-linking between Jira and GitHub:
    • PR links to the relevant Jira issue.
    • Jira ticket has a comment referencing this PR.

@RoushilS RoushilS self-assigned this Jul 1, 2026
@RoushilS
RoushilS marked this pull request as ready for review July 1, 2026 23:27
@RoushilS
RoushilS requested review from a team as code owners July 1, 2026 23:27

@rutmanz rutmanz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like the idea of comparing the config to the default to see if it's been user-modified. Perhaps instead make the default array for the robot preferences empty or otherwise qualitatively different to make it clear it hasn't yet been configured, and use your bounding box computation then.

@RoushilS
RoushilS requested a review from rutmanz July 2, 2026 17:15

export function convertArrayToThreeMatrix4(arr: number[]) {
// DO NOT ask me why retrieving and setting the same EXACT data is done is two DIFFERENT majors
// biome-ignore-start format: We would prefer to visualize this as a matrix

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still true, the old formatting made it much clearer how the matrix works

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay, I'll revert it.

Comment thread fission/src/util/TypeConversions.ts Outdated
Comment on lines +18 to +21
// return identity matrix if array is not valid
if (!arr || arr.length !== 16) {
return new THREE.Matrix4()
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean I suppose this works, but I was thinking that when the robot preferences are loaded or when the intake zone prefs are accessed it automatically replace it with the centered position.

That would also have the effect of making the zone position be centered even before they open the intake config panel which isn't nice just for consistency.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the last sentence mean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to be sure I'm understanding you.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically I don't think it makes sense to have the intake be in one spot before you configure it and a different spot after if you don't move it. Right now it's possible (I think, I'm not able to run it right now) to intake before you configure the intake and the zone is just wherever the identity matrix puts it.

Alternatively, it might make sense to just return early from all the intake and ejector-related functions (the collision checker / button handlers) if the transform array is empty, or even have an explicit "isConfigured" flag in each subsystem so that they don't do anything until someone actually places them somewhere

} from "@/util/TypeConversions"

function isDefaultDelta(delta: number[]): boolean {
return !delta || delta.length === 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessarily a problem but !delta will always be false (empty and filled arrays are truthy*). Unless you're trying to do a null check in which case you should update the method type signature

https://dorey.github.io/JavaScript-Equality-Table/

@RoushilS
RoushilS requested a review from rutmanz July 8, 2026 17:55
@rutmanz

rutmanz commented Jul 8, 2026

Copy link
Copy Markdown
Member

It looks like if you press cancel while editing either intake or ejector, both of them reset to this weird position. If you press save instead on first edit it works fine
image

@rutmanz rutmanz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(see above)

@RoushilS
RoushilS requested a review from rutmanz July 8, 2026 21:09

@rutmanz rutmanz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh I mean it seems like this works but it's more addressing the symptom than the cause. I'm realizing that #1371 makes handling this much easier, but basically I think the settings need to be saved after this.endureDefaultZoneTransformations is called if things are modified. You could either do that with PreferencesSystem or wait for #1371 to get merged (which might take a while)

@RoushilS
RoushilS requested a review from rutmanz July 8, 2026 21:40

@rutmanz rutmanz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem appears to still be occuring

Screen.Recording.2026-07-08.at.4.43.21.PM.mov

@RoushilS

RoushilS commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

hmm. Is there any chance you could send me that robot? And also test on the other robots, as they all work for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants