Skip to content

refactor: Nullable and Modernization for Stride.Core.Serialization#2266

Merged
Eideren merged 25 commits into
stride3d:masterfrom
NexStandard:nullable
May 29, 2024
Merged

refactor: Nullable and Modernization for Stride.Core.Serialization#2266
Eideren merged 25 commits into
stride3d:masterfrom
NexStandard:nullable

Conversation

@IXLLEGACYIXL

@IXLLEGACYIXL IXLLEGACYIXL commented May 22, 2024

Copy link
Copy Markdown
Collaborator

PR Details

Modernizing Stride.Core.Reflection

Description

  1. Should enable Nullable instead of Strides NotNull attribute
  2. Should reduce the usage of reflection, mainly used in collection Descriptors ( thats now prototyped for Dictionary, if its in interested i can convert set and list and the others too )
  3. Using new C# features ie collection initialization, Pattern Matching etc

Related Issue

#2156
#2155
Removes all [NotNull] annotations and uses the new Nullable
Removes a bug taht System.Collections anyway crash the editor as serializers are missing

Motivation and Context

Types of changes

  • Docs change / refactoring / dependency upgrade ( some comments changed )
  • 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.

@Eideren Eideren marked this pull request as draft May 22, 2024 22:50
Comment thread sources/core/Stride.Core.Reflection/MemberPath.cs
@IXLLEGACYIXL IXLLEGACYIXL changed the title WIP: Nullable for Stride.Core.Serialization Nullable and Modernization for Stride.Core.Serialization May 23, 2024
@IXLLEGACYIXL

Copy link
Copy Markdown
Collaborator Author

would be ready for review

dont know what oldcollection descriptor does to be still required

some nullable warnings are still existent but i dont know how to resolve them

@IXLLEGACYIXL IXLLEGACYIXL marked this pull request as ready for review May 24, 2024 13:06

@Eideren Eideren left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great, thanks a bunch for your work @IXLLEGACYIXL

Comment thread sources/core/Stride.Core.Reflection/MemberDescriptors/PropertyDescriptor.cs Outdated
Comment thread sources/core/Stride.Core.Reflection/MemberPath.cs Outdated
Comment thread sources/core/Stride.Core.Reflection/TypeDescriptors/ObjectDescriptor.cs Outdated
Comment thread sources/core/Stride.Core.Reflection/TypeDescriptors/SetDescriptor.cs Outdated
Comment thread sources/core/Stride.Core.Reflection/TypeDescriptors/SetDescriptor.cs Outdated
Comment thread sources/core/Stride.Core.Yaml.Tests/DescriptorTests.cs Outdated
Comment thread sources/core/Stride.Core.Reflection/TypeDescriptorFactory.cs
Comment thread sources/core/Stride.Core.Reflection/MemberPath.cs
@IXLLEGACYIXL

Copy link
Copy Markdown
Collaborator Author

ran tests and they fail currently, maybe its a compilation issue... lets see..

@IXLLEGACYIXL

Copy link
Copy Markdown
Collaborator Author

WELL sample tests fail but on all branches with latest main, but the descriptor tests and all other engien tests pass so it seems good

@Eideren

Eideren commented May 29, 2024

Copy link
Copy Markdown
Collaborator

Yep, it's good, TC reported 4 failing tests but those are unrelated, see #2296

@Eideren Eideren merged commit f449a87 into stride3d:master May 29, 2024
@Eideren

Eideren commented May 29, 2024

Copy link
Copy Markdown
Collaborator

Thanks !

@Eideren Eideren changed the title Nullable and Modernization for Stride.Core.Serialization refactor: Nullable and Modernization for Stride.Core.Serialization May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants