Skip to content

Conversation

@azeno
Copy link
Collaborator

@azeno azeno commented Oct 23, 2025

PR Details

Makes code easier to read (as it was before a122c71). The SetCount implementation looks very similar to the previously used Resize method of FastList. For value types it just sets the internal count, for reference types it clears the pointers. Both what was done before as well.

Related Issue

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

Makes code easier to read (as it was before a122c71).
The SetCount implementation looks very similar to the previously used Resize method of FastList.
For value types it just sets the internal count, for reference types it clears the pointers. Both what was done before as well.
@Kryptos-FR
Copy link
Member

Kryptos-FR commented Oct 23, 2025

Does it zero the new data? If not that's a behavioral change. I can only assume that the code that was explicitly clearing the data was here for a reason.

And if it is not needed, it should be documented in the code why.

@Ethereal77
Copy link
Contributor

It seems that if it is going to shrink, and the type is a reference type or contains references, it clears. You can see it here.

If it needs to grow the list, it does a new T[] (which always is zeroed out), then a copy. Here.

For value types that don't contain references, it does nothing to them (so they can expose stale data).

@Kryptos-FR
Copy link
Member

On second read it looks fine. Thanks.

@azeno
Copy link
Collaborator Author

azeno commented Oct 24, 2025

What @Ethereal77 said - for value types stale data could be observed. But only the third case (RenderSystem) in this commit is about value types and when we read a few lines further down each entry in the re-sized list gets written (no read).

@Eideren Eideren merged commit 24d3660 into stride3d:master Oct 27, 2025
7 checks passed
@Eideren
Copy link
Collaborator

Eideren commented Oct 27, 2025

Thanks !

@Eideren Eideren added the area-Core Issue of the engine unrelated to other defined areas label Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Core Issue of the engine unrelated to other defined areas

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants