Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UnmanagedSystem support in ECS Integration #467

Merged
merged 8 commits into from Oct 21, 2023

Conversation

stenyin
Copy link
Contributor

@stenyin stenyin commented Jan 22, 2023

This pull request adds support for UnmanagedSystem in the ECS Integration feature of VContainer. The changes include new extension methods for ContainerBuilder that allow for easy registration of UnmanagedSystems from a specific World, as well as new classes for handling the creation and disposal of UnmanagedSystem references.

This update also includes the ability to add a system to a specific system group using IntoGroup method.

This should be a useful addition for those using UnmanagedSystems in their projects, and will make it easier to manage the lifetimes of these systems in conjunction with VContainer.

I've tested the changes and it's working well. Please let me know if there's anything else needed.

Note: this feature requires the VCONTAINER_ECS_INTEGRATION_1_0 preprocessor symbol to be defined in order to use it.

…nyin/VContainer into feature/ecs-unmanaged-system

# Conflicts:
#	VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs
#	VContainer/Assets/VContainer/Runtime/Unity/InstanceProviders/UnmanagedSystemInstanceProvider.cs
#	VContainer/Assets/VContainer/Runtime/Unity/UnmanagedSystemReference.cs
@vercel
Copy link

vercel bot commented Jan 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vcontainer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2023 6:23am

@@ -1,4 +1,5 @@
using System;
using Unity.Entities;
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we need the #if directive ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my mistake. i had remove this line.

@hadashiA
Copy link
Owner

@stenyin Very interesting and appreciated as I have not been able to catch up on the new features of the ecs. thanks for the PR.

I would like to merge, but it would be very helpful if you could confirm two points.

  1. If VCONTAINER_ECS_INTEGRATION_1_0 and VCONTAINER_ECS_INTEGRATION are undefined, does it work as it was?
  2. Note: this feature requires the VCONTAINER_ECS_INTEGRATION_1_0 preprocessor symbol to be defined in order to use it.

    • Do we need to do this manually? It was assumed that if the project had a newer version of com.unity.entities, this preprosessor would be added automatically.
      • スクリーンショット 2023-01-31 13 53 38

I would be happy to have it confirmed. Thanks.

@stenyin
Copy link
Contributor Author

stenyin commented Feb 1, 2023

  1. if VCONTAINER_ECS_INTEGRATION_1_0 and VCONTAINER_ECS_INTEGRATION are undefined, it work as well.
  2. Entities 1.0 is compatible with Unity version 2022.2.0b8 and later. Can we check unity version instead of VCONTAINER_ECS_INTEGRATION_1_0 preprocessor symbol?

@hadashiA
Copy link
Owner

hadashiA commented Feb 6, 2023

Thanks for the checking.

  1. Entities 1.0 is compatible with Unity version 2022.2.0b8 and later. Can we check unity version instead of VCONTAINER_ECS_INTEGRATION_1_0 preprocessor symbol?

Sounds like a good idea.
The management the preproseccor symbol myself is complicated.
However, if unity 2022.2 can use less than ecs 1.0, then the unity version check is incomplete. How about this?

@stenyin
Copy link
Contributor Author

stenyin commented Feb 6, 2023

Thank you for your response.

I have confirmed that versions lower than ecs 1.0 cannot be used normally in 2022.2, so I believe using the Unity version as a check is a feasible solution (unless consideration is needed for those using 2022.2.0a1-2022.2.0b7 versions).

@hadashiA
Copy link
Owner

hadashiA commented Feb 8, 2023

Ok, Could you fix this PR to checking the Unity version?

@Borningstar
Copy link

Are there any additional changes needed before this can be merged in @hadashiA?

@stenyin
Copy link
Contributor Author

stenyin commented Jul 10, 2023

Sorry I haven't replied in a while, but for work and personal reasons. I'll test it again.

@stenyin stenyin force-pushed the feature/ecs-unmanaged-system branch from 26e588e to 8f434a1 Compare July 16, 2023 13:18
Additionally, references to Unity.Collections have been added in `VContainer.Tests.asmdef` and `VContainer.asmdef`, to support ECS update.
Remove "VCONTAINER_ECS_INTEGRATION_1_0"
@hadashiA
Copy link
Owner

Sorry for late reply. Looks good for me. Thanks!

@hadashiA hadashiA merged commit 6a904ef into hadashiA:master Oct 21, 2023
2 checks passed
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.

None yet

3 participants