Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Named robot controls #1034

Merged
merged 13 commits into from
Jul 24, 2024
Merged

Named robot controls #1034

merged 13 commits into from
Jul 24, 2024

Conversation

LucaHaverty
Copy link
Collaborator

@LucaHaverty LucaHaverty commented Jul 18, 2024

Description

Adds names and name tags to robots to help the user distinguish between robots when configuring controls and other settings like intake/ejector. You can create completely custom input schemes, or edit the default ones. Deleting a customized version of a default input scheme will reset it, not completely remove it.

Objectives

  • Refactor how inputs are stored and accessed for different robots to support named input schemes instead of only mira specific inputs
  • Add names to default input schemes and a list of other names to auto assign custom ones
  • Panel to select a name when a robot is spawned
  • Update intake and ejector config to work with the new system
  • Integrate with name tags

Note

When testing, make sure to mess around with configuring intakes and ejectors because the way robot control info is stored changed, which could potentially create issues.

image

image

JIRA Issue

@@ -96,7 +92,8 @@ function Synthesis() {
console.debug(`Selected Mirabuf File: ${mira_path}`)
}

const setup = async () => {
// TODO: do we actually want to spawn a robot right away when synthesis loads?
Copy link
Member

Choose a reason for hiding this comment

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

No we don't, but it's just kinda stuck around from when I was testing physics

@LucaHaverty LucaHaverty self-assigned this Jul 18, 2024
@a-crowell a-crowell mentioned this pull request Jul 19, 2024
6 tasks
@LucaHaverty LucaHaverty marked this pull request as ready for review July 19, 2024 17:47
@LucaHaverty LucaHaverty requested a review from a team as a code owner July 19, 2024 17:47
@LucaHaverty LucaHaverty requested review from a-crowell and Dhruv-0-Arora and removed request for a team July 19, 2024 17:47
@autodesk-chorus
Copy link

Chorus detected one or more security issues with this pull request. See the Checks tab for more details.

As a reminder, please follow the secure code review process as part of the Secure Coding Non-Negotiable requirement.

Copy link
Collaborator

@Dhruv-0-Arora Dhruv-0-Arora left a comment

Choose a reason for hiding this comment

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

Looks really good 👍
just have a few ui change requests

Copy link
Member

@HunterBarclay HunterBarclay left a comment

Choose a reason for hiding this comment

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

  1. You should be able to add new schemes in the controls menu
  2. You should prompt the user for a name for the new control scheme, but auto populate it with a name as a suggestion.

@LucaHaverty LucaHaverty added the gameplay Relating to the playability of Synthesis label Jul 22, 2024
Copy link
Collaborator

@Dhruv-0-Arora Dhruv-0-Arora left a comment

Choose a reason for hiding this comment

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

One thing to note for a potential future JIRA task is to be able to reassign names and keybinds for assemblies from the manage assemblies modal

Comment on lines 355 to 358
onAccept={() => {
PreferencesSystem.savePreferences()
InputSchemeManager.saveSchemes()
InputSystem.selectedScheme = undefined
}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you delete the robot onCancel() so if the user cancels their input scheme then the robot doesn't spawn

Copy link
Member

Choose a reason for hiding this comment

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

Would we maybe want them to be able to spawn a robot and not assign it a control scheme temporarily?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@HunterBarclay @Dhruv-0-Arora Is this something we could add to brain config down the line?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah

Copy link
Member

@HunterBarclay HunterBarclay left a comment

Choose a reason for hiding this comment

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

I'm liking this. Good work

@Dhruv-0-Arora Dhruv-0-Arora self-requested a review July 24, 2024 19:05
Copy link
Collaborator

@Dhruv-0-Arora Dhruv-0-Arora left a comment

Choose a reason for hiding this comment

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

Spectacular 🤩

@HunterBarclay HunterBarclay merged commit 94ed383 into dev Jul 24, 2024
13 checks passed
@HunterBarclay HunterBarclay deleted the haverty/1755/named-control-schemes branch July 24, 2024 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gameplay Relating to the playability of Synthesis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants