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

HITL - Highlight default receptacles #2075

Merged
merged 4 commits into from
Sep 7, 2024
Merged

Conversation

0mdc
Copy link
Contributor

@0mdc 0mdc commented Sep 7, 2024

Motivation and Context

This changeset adds a contextual UI that shows the default receptacle.

The following changes are included:

  • Add support for multiple contextual info labels.
  • Add default receptacle highlight and contextual info.
  • Add feature flag.
default_receptacle-2024-09-07_12.58.27.mp4

How Has This Been Tested

Tested on HITL application.

Types of changes

  • [Development]

Checklist

  • My code follows the code style of this project.
  • I have updated the documentation if required.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes if required.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 7, 2024
contextual_color = color_ui_valid
contextual_info.append(
(
"Double-click to pick up.",
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be color_ui_info?

Copy link
Contributor Author

@0mdc 0mdc Sep 7, 2024

Choose a reason for hiding this comment

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

The color theme is the following:

  • White: Generic information.
  • Yellow: Anything related to a pickable object.
  • Cyan: Selection.
  • Green: Anything related to a possible action.
  • Red: Anything related to an impossible action.

I made this label green to match with the green highlight that indicates that an object can be picked up.

selection_ui-2024-08-31_18.03.30.mp4

@@ -696,20 +705,29 @@ def _update_selected_object_ui(self):
# Get the contextual information.
color_ui_valid = [0.2, 1.0, 0.2, 1.0]
color_ui_invalid = [1.0, 0.2, 0.2, 1.0]
color_ui_info = None
color_ui_object_info = [0.8, 0.8, 0.2, 1.0]
Copy link
Contributor

Choose a reason for hiding this comment

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

hm maybe we should only have color_ui_info and use it in all places? rather than have color_ui_info and color_ui_object_info?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also let;s make these the same as the colors defined at the top of this file, rather than re-define them here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also let;s make these the same as the colors defined at the top of this file, rather than re-define them here?

I'll refactor that in a later pass.

  1. Those 2 colors are in different formats. This will be unified.
  2. The contextual information duplicates the real checks. This will be unified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm maybe we should only have color_ui_info and use it in all places? rather than have color_ui_info and color_ui_object_info?

I think that it's preferable to have contextual information related to pickable objects yellow, like the pickable object highlights.

not in self._world._opened_link_set
):
color_default_link = self._to_color_array(
COLOR_HIGHLIGHT
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ok for now, but maybe in the future, should be a config variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed!
I introduced UI configuration in this PR. I intend to make it fully configurable in the following weeks.

Copy link
Contributor

@akshararai akshararai left a comment

Choose a reason for hiding this comment

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

looks good, left minor comments. I think in the future, we should make these features behind flags that can be set in configs, so that its easy to turn them on or off, but looks good for now.

@0mdc 0mdc merged commit 1346eeb into main Sep 7, 2024
3 of 4 checks passed
@0mdc 0mdc deleted the 0mdc/hitl_default_receptacle branch September 7, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants