Making some fields and properties internal #3342
Conversation
| @@ -710,7 +710,7 @@ void _AgentReset() | |||
| AgentReset(); | |||
| } | |||
|
|
|||
| public void UpdateAgentAction(AgentAction action) | |||
| internal void UpdateAgentAction(AgentAction action) | |||
There was a problem hiding this comment.
Should we just remove this and UpdateVectorAction? I don't think they're used anywhere currently.
|
Looks good aside from Academy.EnvironmentStep and the whitespace. |
| @@ -11,7 +11,7 @@ namespace MLAgents | |||
| /// </summary> | |||
| [RequireComponent(typeof(Agent))] | |||
| [AddComponentMenu("ML Agents/Demonstration Recorder", (int)MenuGroup.Default)] | |||
| public class DemonstrationRecorder : MonoBehaviour | |||
| internal class DemonstrationRecorder : MonoBehaviour | |||
There was a problem hiding this comment.
If we make the class internal, can users still add it to the GameObjects in the editor?
There was a problem hiding this comment.
Yes (but I don't know why)
There was a problem hiding this comment.
I think it makes it possible to add the component but not possible to get the component via script (gameObject.GetComponent<TheComponentIsInternalSoICannotDoThis>())
There was a problem hiding this comment.
Let's talk about it more tomorrow. This feels like the sort of thing that someone might want to access from code; I think we should err on the side of keeping it accessible.
There was a problem hiding this comment.
Can you hold off on this one for now? I'd like to do a refactor pass on the DemonstrationRecorder/Store separately from this.
| @@ -24,7 +24,7 @@ public class WriteAdapter | |||
| /// <param name="data">Float array or list that will be written to.</param> | |||
| /// <param name="shape">Shape of the observations to be written.</param> | |||
| /// <param name="offset">Offset from the start of the float data to write to.</param> | |||
| public void SetTarget(IList<float> data, int[] shape, int offset) | |||
| internal void SetTarget(IList<float> data, int[] shape, int offset) | |||
There was a problem hiding this comment.
I think we need to keep this class public; it would be impossible to test a custom sensor without these methods.
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| namespace MLAgents | |||
| { | |||
| public class Startup : MonoBehaviour | |||
| internal class Startup : MonoBehaviour | |||
There was a problem hiding this comment.
Do we use this anywhere? It feels like it should be in the examples code.
There was a problem hiding this comment.
ah yeah. That's my bad. I can move it after this PR lands.
There was a problem hiding this comment.
We use this script for switching the scenes before the training starts. So not really belong to the examples code. Should live where the Startup scene lives.
There was a problem hiding this comment.
But the startup scene is in the example environments
There was a problem hiding this comment.
yeah, above I said I should have moved it to the Project folder.
…rite adapter and the demorecorder
No description provided.