non-IEnumerable interface for action masking #5060
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change(s)
This fixes a few things that bugged me about
IDiscreteActionMask:
GetMask
andResetMask
methods - these were part of the implementation inActuatorDiscreteActionMask
, but they weren't meant for public use (and there was no safe way to use them in anOnActionReceived
callback).IEnumerable
. The previous approach required creating an object to pass, and also needed to allocate memory to iterate over it.WriteMask
" always felt vague, and I always had to look up whether I was enabling or disabling the actions.Things I haven't done yet but would like feedback on:
IDiscreteActionMask.WriteMask()
? This would be a larger interface change for anyone using it, but I think it's for the best.I'm leaning towards removing
WriteMask
but keeping the interface, but wanted to discuss it further.I haven't updated existing tests or documentation yet; wanted to get feedback on the general approach first.
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
https://jira.unity3d.com/browse/MLA-1831
https://jira.unity3d.com/browse/MLA-1728
Types of change(s)
Checklist
Other comments