Use AddComponentMenu for MonoBehaviours #3231
Conversation
| @@ -10,6 +10,7 @@ namespace MLAgents | |||
| /// Demonstration Recorder Component. | |||
| /// </summary> | |||
| [RequireComponent(typeof(Agent))] | |||
| [AddComponentMenu("ML Agents/Demonstration Recorder")] | |||
There was a problem hiding this comment.
Should we list this? I'd be willing to remove it.
There was a problem hiding this comment.
I think it makes sense
There was a problem hiding this comment.
Makes sense to have it or remove it?
| @@ -8,6 +8,7 @@ namespace MLAgents | |||
| /// Monitor is used to display information about the Agent within the Unity | |||
| /// scene. Use the log function to add information to your monitor. | |||
| /// </summary> | |||
| [AddComponentMenu("ML Agents/Monitor")] | |||
There was a problem hiding this comment.
I'm OK removing this one too.
There was a problem hiding this comment.
We should, this is supposed to be hidden from the user and automagic
| @@ -3,6 +3,7 @@ | |||
|
|
|||
| namespace MLAgents.Sensor | |||
| { | |||
| [AddComponentMenu("ML Agents/Sensors/Camera Sensor")] | |||
There was a problem hiding this comment.
Intentionally left "Component" off the sensor menu items; I think it's a bit wordy otherwise but willing to change it.
There was a problem hiding this comment.
The Unity UI guidelines recommend against nesting menus. We should just group the alike menu items using the componentOrder constructor variable.
There was a problem hiding this comment.
| [AddComponentMenu("ML Agents/Sensors/Camera Sensor")] | |
| [AddComponentMenu("ML Agents/Camera Sensor", kSensorMenuGroup)] |
Where kSensorMenuGroup is defined somewhere.
chriselion
changed the title
For now 😈 |
| @@ -8,6 +8,7 @@ namespace MLAgents | |||
| /// Monitor is used to display information about the Agent within the Unity | |||
| /// scene. Use the log function to add information to your monitor. | |||
| /// </summary> | |||
| [AddComponentMenu("ML Agents/Monitor")] | |||
There was a problem hiding this comment.
We should, this is supposed to be hidden from the user and automagic
| @@ -10,6 +10,7 @@ namespace MLAgents | |||
| /// Demonstration Recorder Component. | |||
| /// </summary> | |||
| [RequireComponent(typeof(Agent))] | |||
| [AddComponentMenu("ML Agents/Demonstration Recorder")] | |||
There was a problem hiding this comment.
I think it makes sense
There was a problem hiding this comment.
I am not sure we should do this now. We do not know yet how components will evolve and I would like to avoid maintaining new user entry points and associated documentation.
@vincentpierre I believe this is something we should be doing and keep up to date as components are changing. This way users have multiple ways to find out what they need. This is a common workflow for unity users. It's a small, yet impactful, step towards improving the discoverability of components (even if they are changing).
|
Without nesting and adding a menu group: |
| /// <summary> | ||
| /// Grouping for use in AddComponentMenu (instead of nesting the menus). | ||
| /// </summary> | ||
| public enum MenuGroup |
There was a problem hiding this comment.
Better way to do this / better place to put it?
There have been a few issues about missing menu items.
#2870
#3227
These were probably from following a now-outdated tutorial, but I think adding menu items for the MonoBehaviours that we have probably makes things a bit more discoverable:
Menu bar:
Add Component in the inspector
Unfortunately,
Agentdoesn't qualify for this because it's abstract.