Uncomment obsolete attributes, fix warnings (#4771) (#4774)

Chris Elion · andrewcoh · web-flow · commit a2b81c3b6620 · 2020-12-17T18:57:06.000-08:00
* uncomment obsolete attributes, fix warnings

* Apply suggestions from code review

Co-authored-by: andrewcoh &lt;54679309+andrewcoh@users.noreply.github.com&gt;

Co-authored-by: andrewcoh &lt;54679309+andrewcoh@users.noreply.github.com&gt;

Co-authored-by: andrewcoh &lt;54679309+andrewcoh@users.noreply.github.com&gt;
diff --git a/com.unity.ml-agents/Runtime/Actuators/IActionReceiver.cs b/com.unity.ml-agents/Runtime/Actuators/IActionReceiver.cs
@@ -156,7 +156,7 @@ public override int GetHashCode()
         /// <param name="destination">A float array to pack actions into whose length is greater than or
         /// equal to the addition of the Lengths of this objects <see cref="ContinuousActions"/> and
         /// <see cref="DiscreteActions"/> segments.</param>
-        /// [Obsolete("PackActions has been deprecated.")]
+        [Obsolete("PackActions has been deprecated.")]
         public void PackActions(in float[] destination)
         {
             Debug.Assert(destination.Length >= ContinuousActions.Length + DiscreteActions.Length,
diff --git a/com.unity.ml-agents/Runtime/Agent.cs b/com.unity.ml-agents/Runtime/Agent.cs
@@ -886,7 +886,11 @@ public virtual void Initialize() { }
         /// <seealso cref="IActionReceiver.OnActionReceived"/>
         public virtual void Heuristic(in ActionBuffers actionsOut)
         {
-            // For backward compatibility
+            // Disable deprecation warnings so we can call the legacy overload.
+#pragma warning disable CS0618
+
+            // The default implementation of Heuristic calls the
+            // obsolete version for backward compatibility
             switch (m_PolicyFactory.BrainParameters.VectorActionSpaceType)
             {
                 case SpaceType.Continuous:
@@ -904,6 +908,8 @@ public virtual void Heuristic(in ActionBuffers actionsOut)
                     actionsOut.ContinuousActions.Clear();
                     break;
             }
+#pragma warning restore CS0618
+
         }
 
         /// <summary>
@@ -1151,7 +1157,10 @@ public virtual void WriteDiscreteActionMask(IDiscreteActionMask actionMask)
             {
                 m_ActionMasker = new DiscreteActionMasker(actionMask);
             }
+            // Disable deprecation warnings so we can call the legacy overload.
+#pragma warning disable CS0618
             CollectDiscreteActionMasks(m_ActionMasker);
+#pragma warning restore CS0618
         }
 
         /// <summary>
@@ -1232,7 +1241,10 @@ public virtual void OnActionReceived(ActionBuffers actions)
             {
                 m_LegacyActionCache = Array.ConvertAll(actions.DiscreteActions.Array, x => (float)x);
             }
+            // Disable deprecation warnings so we can call the legacy overload.
+#pragma warning disable CS0618
             OnActionReceived(m_LegacyActionCache);
+#pragma warning restore CS0618
         }
 
         /// <summary>
diff --git a/com.unity.ml-agents/Runtime/Agent.deprecated.cs b/com.unity.ml-agents/Runtime/Agent.deprecated.cs
@@ -9,6 +9,7 @@ public partial class Agent
         /// Deprecated, use <see cref="WriteDiscreteActionMask"/> instead.
         /// </summary>
         /// <param name="actionMasker"></param>
+        [Obsolete("CollectDiscreteActionMasks has been deprecated, please use WriteDiscreteActionMask.")]
         public virtual void CollectDiscreteActionMasks(DiscreteActionMasker actionMasker)
         {
         }
@@ -17,6 +18,7 @@ public virtual void CollectDiscreteActionMasks(DiscreteActionMasker actionMasker
         /// Deprecated, use <see cref="Heuristic(in ActionBuffers)"/> instead.
         /// </summary>
         /// <param name="actionsOut"></param>
+        [Obsolete("The float[] version of Heuristic has been deprecated, please use the ActionBuffers version instead.")]
         public virtual void Heuristic(float[] actionsOut)
         {
             Debug.LogWarning("Heuristic method called but not implemented. Returning placeholder actions.");
@@ -27,6 +29,7 @@ public virtual void Heuristic(float[] actionsOut)
         /// Deprecated, use <see cref="OnActionReceived(ActionBuffers)"/> instead.
         /// </summary>
         /// <param name="vectorAction"></param>
+        [Obsolete("The float[] version of OnActionReceived has been deprecated, please use the ActionBuffers version instead.")]
         public virtual void OnActionReceived(float[] vectorAction) { }
 
         /// <summary>
@@ -36,7 +39,7 @@ public virtual void OnActionReceived(float[] vectorAction) { }
         /// The last action that was decided by the Agent (or null if no decision has been made).
         /// </returns>
         /// <seealso cref="OnActionReceived(ActionBuffers)"/>
-        // [Obsolete("GetAction has been deprecated, please use GetStoredActionBuffers, Or GetStoredDiscreteActions.")]
+        [Obsolete("GetAction has been deprecated, please use GetStoredActionBuffers instead.")]
         public float[] GetAction()
         {
             var storedAction = m_Info.storedVectorActions;
diff --git a/com.unity.ml-agents/Runtime/Communicator/GrpcExtensions.cs b/com.unity.ml-agents/Runtime/Communicator/GrpcExtensions.cs
@@ -101,6 +101,8 @@ public static List<ObservationSummary> GetObservationSummaries(this AgentInfoAct
         /// <param name="isTraining">Whether or not the Brain is training.</param>
         public static BrainParametersProto ToProto(this BrainParameters bp, string name, bool isTraining)
         {
+            // Disable deprecation warnings so we can set legacy fields
+#pragma warning disable CS0618
             var brainParametersProto = new BrainParametersProto
             {
                 VectorActionSpaceTypeDeprecated = (SpaceTypeProto)bp.VectorActionSpaceType,
@@ -116,6 +118,7 @@ public static BrainParametersProto ToProto(this BrainParameters bp, string name,
             {
                 brainParametersProto.VectorActionDescriptionsDeprecated.AddRange(bp.VectorActionDescriptions);
             }
+#pragma warning restore CS0618
             return brainParametersProto;
         }
 
diff --git a/com.unity.ml-agents/Runtime/Policies/BrainParameters.cs b/com.unity.ml-agents/Runtime/Policies/BrainParameters.cs
@@ -78,7 +78,7 @@ public ActionSpec ActionSpec
         /// the action.
         /// For the discrete action space: the number of branches in the action space.
         /// </value>
-        /// [Obsolete("VectorActionSize has been deprecated, please use ActionSpec instead.")]
+        [Obsolete("VectorActionSize has been deprecated, please use ActionSpec instead.")]
         [FormerlySerializedAs("vectorActionSize")]
         public int[] VectorActionSize = new[] { 1 };
 
@@ -91,7 +91,7 @@ public ActionSpec ActionSpec
         /// <summary>
         /// (Deprecated) Defines if the action is discrete or continuous.
         /// </summary>
-        /// [Obsolete("VectorActionSpaceType has been deprecated, please use ActionSpec instead.")]
+        [Obsolete("VectorActionSpaceType has been deprecated, please use ActionSpec instead.")]
         [FormerlySerializedAs("vectorActionSpaceType")]
         public SpaceType VectorActionSpaceType = SpaceType.Discrete;
 
@@ -102,7 +102,7 @@ public ActionSpec ActionSpec
         /// <summary>
         /// (Deprecated) The number of actions specified by this Brain.
         /// </summary>
-        /// [Obsolete("NumActions has been deprecated, please use ActionSpec instead.")]
+        [Obsolete("NumActions has been deprecated, please use ActionSpec instead.")]
         public int NumActions
         {
             get
@@ -117,6 +117,8 @@ public int NumActions
         /// <returns> A new BrainParameter object with the same values as the original.</returns>
         public BrainParameters Clone()
         {
+            // Disable deprecation warnings so we can read/write the old fields.
+#pragma warning disable CS0618
             return new BrainParameters
             {
                 VectorObservationSize = VectorObservationSize,
@@ -126,13 +128,16 @@ public BrainParameters Clone()
                 VectorActionSize = (int[])VectorActionSize.Clone(),
                 VectorActionSpaceType = VectorActionSpaceType,
             };
+#pragma warning restore CS0618
         }
 
         /// <summary>
         /// Propogate ActionSpec fields from deprecated fields
         /// </summary>
         private void UpdateToActionSpec()
         {
+            // Disable deprecation warnings so we can read the old fields.
+#pragma warning disable CS0618
             if (!hasUpgradedBrainParametersWithActionSpec
                 && m_ActionSpec.NumContinuousActions == 0
                 && m_ActionSpec.BranchSizes == null)
@@ -149,13 +154,17 @@ private void UpdateToActionSpec()
                 }
             }
             hasUpgradedBrainParametersWithActionSpec = true;
+#pragma warning restore CS0618
         }
 
         /// <summary>
         /// Sync values in ActionSpec fields to deprecated fields
         /// </summary>
         private void SyncDeprecatedActionFields()
         {
+            // Disable deprecation warnings so we can read the old fields.
+#pragma warning disable CS0618
+
             if (m_ActionSpec.NumContinuousActions == 0)
             {
                 VectorActionSize = (int[])ActionSpec.BranchSizes.Clone();
@@ -170,6 +179,7 @@ private void SyncDeprecatedActionFields()
             {
                 VectorActionSize = null;
             }
+#pragma warning restore CS0618
         }
 
         /// <summary>
diff --git a/com.unity.ml-agents/Tests/Editor/MLAgentsEditModeTest.cs b/com.unity.ml-agents/Tests/Editor/MLAgentsEditModeTest.cs
@@ -711,8 +711,16 @@ public void TestHeuristicPolicyStepsSensors()
             Assert.AreEqual(numSteps, agent1.sensor1.numWriteCalls);
             Assert.AreEqual(numSteps, agent1.sensor2.numCompressedCalls);
 
+            // Disable deprecation warnings so we can read/write the old fields.
+#pragma warning disable CS0618
+
             // Make sure the Heuristic method read the observation and set the action
             Assert.AreEqual(agent1.collectObservationsCallsForEpisode, agent1.GetAction()[0]);
+            Assert.AreEqual(
+                agent1.collectObservationsCallsForEpisode,
+                agent1.GetStoredActionBuffers().ContinuousActions[0]
+            );
+#pragma warning restore CS0618
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ public override int GetHashCode()`
`156`	`156`	`/// <param name="destination">A float array to pack actions into whose length is greater than or`
`157`	`157`	`/// equal to the addition of the Lengths of this objects <see cref="ContinuousActions"/> and`
`158`	`158`	`/// <see cref="DiscreteActions"/> segments.</param>`
`159`		`- /// [Obsolete("PackActions has been deprecated.")]`
	`159`	`+ [Obsolete("PackActions has been deprecated.")]`
`160`	`160`	`public void PackActions(in float[] destination)`
`161`	`161`	`{`
`162`	`162`	`Debug.Assert(destination.Length >= ContinuousActions.Length + DiscreteActions.Length,`
Original file line number	Diff line number	Diff line change
`@@ -886,7 +886,11 @@ public virtual void Initialize() { }`
`886`	`886`	`/// <seealso cref="IActionReceiver.OnActionReceived"/>`
`887`	`887`	`public virtual void Heuristic(in ActionBuffers actionsOut)`
`888`	`888`	`{`
`889`		`- // For backward compatibility`
	`889`	`+ // Disable deprecation warnings so we can call the legacy overload.`
	`890`	`+#pragma warning disable CS0618`
	`891`	`+`
	`892`	`+ // The default implementation of Heuristic calls the`
	`893`	`+ // obsolete version for backward compatibility`
`890`	`894`	`switch (m_PolicyFactory.BrainParameters.VectorActionSpaceType)`
`891`	`895`	`{`
`892`	`896`	`case SpaceType.Continuous:`
`@@ -904,6 +908,8 @@ public virtual void Heuristic(in ActionBuffers actionsOut)`
`904`	`908`	`actionsOut.ContinuousActions.Clear();`
`905`	`909`	`break;`
`906`	`910`	`}`
	`911`	`+#pragma warning restore CS0618`
	`912`	`+`
`907`	`913`	`}`
`908`	`914`
`909`	`915`	`/// <summary>`
`@@ -1151,7 +1157,10 @@ public virtual void WriteDiscreteActionMask(IDiscreteActionMask actionMask)`
`1151`	`1157`	`{`
`1152`	`1158`	`m_ActionMasker = new DiscreteActionMasker(actionMask);`
`1153`	`1159`	`}`
	`1160`	`+ // Disable deprecation warnings so we can call the legacy overload.`
	`1161`	`+#pragma warning disable CS0618`
`1154`	`1162`	`CollectDiscreteActionMasks(m_ActionMasker);`
	`1163`	`+#pragma warning restore CS0618`
`1155`	`1164`	`}`
`1156`	`1165`
`1157`	`1166`	`/// <summary>`
`@@ -1232,7 +1241,10 @@ public virtual void OnActionReceived(ActionBuffers actions)`
`1232`	`1241`	`{`
`1233`	`1242`	`m_LegacyActionCache = Array.ConvertAll(actions.DiscreteActions.Array, x => (float)x);`
`1234`	`1243`	`}`
	`1244`	`+ // Disable deprecation warnings so we can call the legacy overload.`
	`1245`	`+#pragma warning disable CS0618`
`1235`	`1246`	`OnActionReceived(m_LegacyActionCache);`
	`1247`	`+#pragma warning restore CS0618`
`1236`	`1248`	`}`
`1237`	`1249`
`1238`	`1250`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -101,6 +101,8 @@ public static List<ObservationSummary> GetObservationSummaries(this AgentInfoAct`
`101`	`101`	`/// <param name="isTraining">Whether or not the Brain is training.</param>`
`102`	`102`	`public static BrainParametersProto ToProto(this BrainParameters bp, string name, bool isTraining)`
`103`	`103`	`{`
	`104`	`+ // Disable deprecation warnings so we can set legacy fields`
	`105`	`+#pragma warning disable CS0618`
`104`	`106`	`var brainParametersProto = new BrainParametersProto`
`105`	`107`	`{`
`106`	`108`	`VectorActionSpaceTypeDeprecated = (SpaceTypeProto)bp.VectorActionSpaceType,`
`@@ -116,6 +118,7 @@ public static BrainParametersProto ToProto(this BrainParameters bp, string name,`
`116`	`118`	`{`
`117`	`119`	`brainParametersProto.VectorActionDescriptionsDeprecated.AddRange(bp.VectorActionDescriptions);`
`118`	`120`	`}`
	`121`	`+#pragma warning restore CS0618`
`119`	`122`	`return brainParametersProto;`
`120`	`123`	`}`
`121`	`124`
Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ public ActionSpec ActionSpec`
`78`	`78`	`/// the action.`
`79`	`79`	`/// For the discrete action space: the number of branches in the action space.`
`80`	`80`	`/// </value>`
`81`		`- /// [Obsolete("VectorActionSize has been deprecated, please use ActionSpec instead.")]`
	`81`	`+ [Obsolete("VectorActionSize has been deprecated, please use ActionSpec instead.")]`
`82`	`82`	`[FormerlySerializedAs("vectorActionSize")]`
`83`	`83`	`public int[] VectorActionSize = new[] { 1 };`
`84`	`84`
`@@ -91,7 +91,7 @@ public ActionSpec ActionSpec`
`91`	`91`	`/// <summary>`
`92`	`92`	`/// (Deprecated) Defines if the action is discrete or continuous.`
`93`	`93`	`/// </summary>`
`94`		`- /// [Obsolete("VectorActionSpaceType has been deprecated, please use ActionSpec instead.")]`
	`94`	`+ [Obsolete("VectorActionSpaceType has been deprecated, please use ActionSpec instead.")]`
`95`	`95`	`[FormerlySerializedAs("vectorActionSpaceType")]`
`96`	`96`	`public SpaceType VectorActionSpaceType = SpaceType.Discrete;`
`97`	`97`
`@@ -102,7 +102,7 @@ public ActionSpec ActionSpec`
`102`	`102`	`/// <summary>`
`103`	`103`	`/// (Deprecated) The number of actions specified by this Brain.`
`104`	`104`	`/// </summary>`
`105`		`- /// [Obsolete("NumActions has been deprecated, please use ActionSpec instead.")]`
	`105`	`+ [Obsolete("NumActions has been deprecated, please use ActionSpec instead.")]`
`106`	`106`	`public int NumActions`
`107`	`107`	`{`
`108`	`108`	`get`
`@@ -117,6 +117,8 @@ public int NumActions`
`117`	`117`	`/// <returns> A new BrainParameter object with the same values as the original.</returns>`
`118`	`118`	`public BrainParameters Clone()`
`119`	`119`	`{`
	`120`	`+ // Disable deprecation warnings so we can read/write the old fields.`
	`121`	`+#pragma warning disable CS0618`
`120`	`122`	`return new BrainParameters`
`121`	`123`	`{`
`122`	`124`	`VectorObservationSize = VectorObservationSize,`
`@@ -126,13 +128,16 @@ public BrainParameters Clone()`
`126`	`128`	`VectorActionSize = (int[])VectorActionSize.Clone(),`
`127`	`129`	`VectorActionSpaceType = VectorActionSpaceType,`
`128`	`130`	`};`
	`131`	`+#pragma warning restore CS0618`
`129`	`132`	`}`
`130`	`133`
`131`	`134`	`/// <summary>`
`132`	`135`	`/// Propogate ActionSpec fields from deprecated fields`
`133`	`136`	`/// </summary>`
`134`	`137`	`private void UpdateToActionSpec()`
`135`	`138`	`{`
	`139`	`+ // Disable deprecation warnings so we can read the old fields.`
	`140`	`+#pragma warning disable CS0618`
`136`	`141`	`if (!hasUpgradedBrainParametersWithActionSpec`
`137`	`142`	`&& m_ActionSpec.NumContinuousActions == 0`
`138`	`143`	`&& m_ActionSpec.BranchSizes == null)`
`@@ -149,13 +154,17 @@ private void UpdateToActionSpec()`
`149`	`154`	`}`
`150`	`155`	`}`
`151`	`156`	`hasUpgradedBrainParametersWithActionSpec = true;`
	`157`	`+#pragma warning restore CS0618`
`152`	`158`	`}`
`153`	`159`
`154`	`160`	`/// <summary>`
`155`	`161`	`/// Sync values in ActionSpec fields to deprecated fields`
`156`	`162`	`/// </summary>`
`157`	`163`	`private void SyncDeprecatedActionFields()`
`158`	`164`	`{`
	`165`	`+ // Disable deprecation warnings so we can read the old fields.`
	`166`	`+#pragma warning disable CS0618`
	`167`	`+`
`159`	`168`	`if (m_ActionSpec.NumContinuousActions == 0)`
`160`	`169`	`{`
`161`	`170`	`VectorActionSize = (int[])ActionSpec.BranchSizes.Clone();`
`@@ -170,6 +179,7 @@ private void SyncDeprecatedActionFields()`
`170`	`179`	`{`
`171`	`180`	`VectorActionSize = null;`
`172`	`181`	`}`
	`182`	`+#pragma warning restore CS0618`
`173`	`183`	`}`
`174`	`184`
`175`	`185`	`/// <summary>`