Agent implements lifecycle methods like OnEnable and OnDisable as private, so they mustn't be used, yet are easy to disable by a mistake

[scscgit]

The MLAgents.Agent is a MonoBehaviour, which includes private functions like OnEnable and OnDisable that always have be called, yet it's possible for the user to implement their own new private methods, which Unity calls using reflection instead. When the user does so, there's no visible error, yet the framework silently fails to initialize, and the Agents don't call any actions. My suggestions are as follows:

1. Always implement Unity lifecycle methods as protected, so that it's possible to call them using base superclass reference.
2. Use some boolean field to make sure that the methods have been called, and display a warning when they haven't.

This is especially important when the user uses APIs like NavMeshQuery that use an Allocator, which strictly require to be disposed and re-allocated when using Play Mode Options. An example use-case is adding a simple observation vector of a NavMesh Raycast.

There is currently a possible workaround to invoke those private functions using a reflection. Here is an example if anyone's interested:

private NavMeshQuery _navMeshQuery;

private void OnEnable()
{
    typeof(Agent)
        .GetMethod("OnEnable", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)
        .Invoke(this, new object[] { });
    // Due to Play Mode Options, this must get re-initialized under OnEnable after OnDisable (memory leak prevention)
    _navMeshQuery = new NavMeshQuery(NavMeshWorld.GetDefaultWorld(), Allocator.Persistent);
}

private void OnDisable()
{
    typeof(Agent)
        .GetMethod("OnDisable", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)
        .Invoke(this, new object[] { });
    _navMeshQuery.Dispose();
}

Environment:

• ML-Agents version: v0.14.0

[surfnerd]

HI @scscgit, This is great feedback. We have discussed this internally and will address the issue ASAP. We have logged the issue internally as MLA-739, and will update this thread when a fix is merged into master.

[surfnerd]

Hi, The fix for this was merged into master from #3590. Please open this issue again if a problem persists.

[github-actions[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.