Skip to content

Manage CameraSensors in CameraSensorComponent - #6324

Open
montplaisir wants to merge 5 commits into
developfrom
viviane/camera-sensor-textures
Open

Manage CameraSensors in CameraSensorComponent#6324
montplaisir wants to merge 5 commits into
developfrom
viviane/camera-sensor-textures

Conversation

@montplaisir

@montplaisir montplaisir commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Proposed change(s)

For CameraSensorComponent, have a list of CameraSensors instead of a single one.
For RenderTextureSensorComponent, have a list of RenderTextureSensors instead of a single one.

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Fixes #6320

Types of change(s)

  • Bug fix

Checklist

  • Added tests that prove my fix is effective or that my feature works
  • Updated the changelog

…single one.

For RenderTextureSensorComponent, have a list of RenderTextureSensors instead of a single one.
@montplaisir

Copy link
Copy Markdown
Contributor Author

/review

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

LGTM

The change correctly gives concurrently initialized agents distinct camera and render-texture sensors, but the new component-owned collections are not reconciled with the agent lifecycle.

Reviewed commit 0421fc0

🤖 Helpful? 👍/👎

Dispose();
m_Sensor = new CameraSensor(m_Camera, m_Width, m_Height, Grayscale, m_SensorName, m_Compression, m_ObservationType);
var sensor = new CameraSensor(m_Camera, m_Width, m_Height, Grayscale, m_SensorName, m_Compression, m_ObservationType);
m_Sensors.Add(sensor);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2

When an agent is disabled, Agent.OnDisable() disposes the ISensor instances it received but never calls SensorComponent.Dispose(). Re-enabling that agent initializes it again and calls CreateSensors(), so this append retains the already-disposed sensor indefinitely; repeated enable/disable cycles grow this list (and make every runtime property update iterate stale entries). The same issue exists in RenderTextureSensorComponent. Deregister each sensor when its owning agent disposes it, or otherwise prune disposed entries without disposing sensors still owned by other agents.

🤖 Helpful? 👍/👎

@montplaisir

Copy link
Copy Markdown
Contributor Author

/review

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

Ship it

The change now gives each agent its own visual sensor and disposes stacked wrappers so the wrapped texture is released during agent cleanup. I examined the creation, cleanup, stacking, and inspector-temporary-sensor paths and found no new actionable issues beyond the previously reported finding.

Reviewed commit 6ccc12c

🤖 Helpful? 👍/👎

@montplaisir
montplaisir marked this pull request as ready for review August 14, 2026 16:28

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

Ship it

The change now gives each agent its own camera or render-texture sensor and ensures stacked wrappers release their owned concrete sensors. I examined the agent cleanup/reinitialization path, component update and disposal behavior, inspector-created sensor cleanup, and other stacking call sites; no new actionable defect was identified beyond the previously reported issue.

Reviewed commit 2f0c204

🤖 Helpful? 👍/👎

@Codengineer Codengineer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Please address UPR issue. We need to make sure no memory leaks can be introduced by any path and that everything is properly disposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CameraSensor NullReferenceException & Black Screen

2 participants