sebas77 / Svelto.ECS

Svelto ECS C# Entity Component System
http://www.sebaslab.com
MIT License
1.2k stars 97 forks source link

Svelto.ECS on OpenUPM #45

Closed favoyang closed 4 years ago

favoyang commented 4 years ago

Hi @sebas77,

Thanks for creating the amazing project. I'm the creator of OpenUPM, a public upm registry with automatic build pipelines that publish new package bundle based on valid git tags. You can read the announcement on medium to get the basic idea, so far it hosts 90+ packages as of Jan 2020.

UPM offering better version and dependency management, is the future to delivery reusable component in the Unity Engine. I'd like to help you convert your packages to upm format, to be able to publish on git and the OpenUPM platform.

https://github.com/sebas77/Svelto.ECS

https://github.com/sebas77/Svelto.Tasks/

https://github.com/sebas77/Svelto.Common

Create GitHub release for all these repositories. (tag as x.y.z)

The changes will

Thought?

sebas77 commented 4 years ago

Hello,

yes I started to experiment with packages, but I had to stop as I had some difficulties. I will be happy to test OpenUPM, I am really curious about the unity integration and how it's going to work.

favoyang commented 4 years ago

Pull requests created:

After you merge the pull requests, Please create GitHub releases for each repository - v2.9.1

In the meanwhile, I'm setting up myside to add these to the platform. Later on we will continue on how to consume these packages (and how to keep developing them).

sebas77 commented 4 years ago

OK I have updated Svelto.ECS too, removed Svelto.Services (I had to do it anyway) and added the new package.json

favoyang commented 4 years ago

Cool.

The packages are added to OpenUPM platform

However to make it useful, you still need create new GitHub releases for all three repositories. The openupm build pipelines will watch the releases and build packages for you. The git tag name should be same as the package.json version - 2.9.1 for this time. It usually takes 5-10 mins for the build bot to finish the job.

sebas77 commented 4 years ago

I'll do it asap. It's an incredible job you did there. Looking forward to understanding how it's integrated to Unity and finding the time to read better your documentation.

favoyang commented 4 years ago

While waiting for GitHub releases and build pipelines. Some useful information.

  1. To learn how to use unity package manager (the basic stuff), please see https://docs.unity3d.com/Packages/com.unity.package-manager-ui@2.1/manual/index.html

  2. To consume your packages (unity integration), for now we have the openupm-cli, a command line tool to help you manage the Packages/manifest.json (the dependency file of the UPM system). If you have experience with Npm or Yarn, you will catch up quickly. Here's a quick tutorial.

    In future, we will create a dedicated way to integrate with Unity Editor UI. There maybe a quick integration first see https://github.com/openupm/openupm/issues/10, depends on how it goes, maybe the official support come after when I have more time.

  3. Because when you install a package via registry, the package is read only. To make it easier to keep developing the packages, two choices:

    3.1 use git subtree to place your packages directly under Packages folder. Unity treat them as local package which is mutable.

    3.2 checkout each of your repository. Go to an empty Unity project, use the Packages > Plus icon > Add package from disks... to point to your local checkout of these packages

    image

    your Packages/manifest.json will look like

{
  "dependencies": {
    "com.sebaslab.svelto.common": "file:PATH-OF-Svelto.Common",
    "com.sebaslab.svelto.ecs": "file:PATH-OF-Svelto.ECS/Svelto.ECS",
    "com.sebaslab.svelto.tasks": "file:PATH-OF-Svelto.Tasks/Svelto.Tasks",
sebas77 commented 4 years ago

will the system share only dll? Is it possible to distribute the code too?

favoyang commented 4 years ago

The system just bundle source code (same as you install via git url), no DLL is generated on cloud. Unity using asmdef files to compile and generate DLL on client side. Checkout the Library/ScriptAssemblies folders:

$ ls
BuiltinAssemblies.stamp             Unity.CollabProxy.Editor.pdb          Unity.TextMeshPro.pdb
Svelto.Common.dll*                  Unity.Collections.dll*                Unity.Timeline.dll*
Svelto.Common.pdb                   Unity.Collections.pdb                 Unity.Timeline.Editor.dll*
Svelto.ECS.dll*                     Unity.Mathematics.dll*                Unity.Timeline.Editor.pdb
Svelto.ECS.pdb                      Unity.Mathematics.Editor.dll*         Unity.Timeline.pdb
Svelto.Pool.Debugger.dll*           Unity.Mathematics.Editor.pdb          Unity.VSCode.Editor.dll*
Svelto.Pool.Debugger.pdb            Unity.Mathematics.pdb                 Unity.VSCode.Editor.pdb
Svelto.Tasks.dll*                   Unity.PerformanceTesting.dll*         UnityEditor.TestRunner.dll*
Svelto.Tasks.pdb                    Unity.PerformanceTesting.Editor.dll*  UnityEditor.TestRunner.pdb
Svelto.Tasks.StopEditorThread.dll*  Unity.PerformanceTesting.Editor.pdb   UnityEditor.UI.dll*
Svelto.Tasks.StopEditorThread.pdb   Unity.PerformanceTesting.pdb          UnityEditor.UI.pdb
Unity.Burst.dll*                    Unity.Rider.Editor.dll*               UnityEngine.TestRunner.dll*
Unity.Burst.Editor.dll*             Unity.Rider.Editor.pdb                UnityEngine.TestRunner.pdb
Unity.Burst.Editor.pdb              Unity.TextMeshPro.dll*                UnityEngine.UI.dll*
Unity.Burst.pdb                     Unity.TextMeshPro.Editor.dll*         UnityEngine.UI.pdb
Unity.CollabProxy.Editor.dll*       Unity.TextMeshPro.Editor.pdb
sebas77 commented 4 years ago

OK I will spend some time on it soon. I was just wondering why the building process is needed if dll are not distributed. I created the releases!

favoyang commented 4 years ago

In a short, an upm package is a tarball of package.json, source codes and asmdef files. A very loose structure. A build process is simply a npm publish command, depends where it publish to.

Choice for package owner

Certainly the docs cover more details about why OpenUPM designed this way.

sebas77 commented 4 years ago

sounds great, I will give you some feedback asap.

sebas77 commented 4 years ago

One thing I will need to add asap is that with Svelto.ECS 3.0, svelto will need to be dependent on System.Runtime.CompilerServices.Unsafe.dll. Would UPM be able to manage these kind of dependencies too?

favoyang commented 4 years ago

It sounds like a general asmdef reference DLL issue, not an upm issue. Upm is just a folder with asmdef. I assume it should be handled correctly by default. Because com.unity.collections@0.1.1-preview is referencing System.Runtime.CompilerServices.Unsafe.dll (see Library/PackageCache/com.unity.collections@0.1.1-preview after you installed the package). It just places the dll to the package root, and nothing more. Maybe it will just work, because ecs is already referencing the com.unity.collections. If you have question, please post on or search the sub-forum.

BTW, your packages are processed correctly: i.e. https://openupm.com/packages/com.sebaslab.svelto.ecs/

You can add a badge to help promote the OpenUPM service, recommended but totally optional. https://openupm.com/docs/adding-badge.html

You can also read https://www.patreon.com/posts/25070968, a tutorial for upm creator. It covers a tip to create a clean upm branch, for those repository doesn't have package in the root path. It's useful when you want to give your audience a second choice to install package via git.

Configurations on publishing your upm packages anyway. That's a good move!

I will close the issue, but feel free to use the thread to continue the conversation.

sebas77 commented 4 years ago

let me rephrase it. I obviously never used UPM (or NPM). I used a bit nuget and with nuget I could describe dependencies to download automatically external dll. I currently added the System.Runtime.CompilerServices.Unsafe.dll in my repository, but I would like to get rid of it if UPM could download it automatically from the official repo (nuget). It would be incredibly useful if it could also detect conflicts and not download it in case com.unity.collections@0.1.1-preview is used. At the moment there is a bit of manual process to make it work (which I have to document).

Thank you for all your effort!

favoyang commented 4 years ago

While I don’t think it will happen that unity helps you to download a nuget dll. If so they won’t place the dll to the root folder of the collections package.

You have to make it clear that if the com.unity.collections@0.1.1-preview package is a dependency, it must be installed, no matter manually (for existing user not using upm) or via the upm dependency system. So you can assume the the dll is already referenced because the com.unity.collections@0.1.1-preview package is installed.

But I agree that a better way is that the dll is a package that is part of Svelto.ECS dependencies. Then it become a trivial dependency resolving task.

When OpenUPM get stable, we will help convert some nuget to upm package. But we can not change how unity internal package works. So it may not solve the issue, but make install other nuget packages easier.

favoyang commented 4 years ago

I just found a post https://forum.unity.com/threads/please-update-com-unity-collections-reference-of-system-runtime-compilerservices-unsafe.732056/, complains about the issue of manually import the dll into the com.unity.collections@0.1.1-preview package. The thread poster need a higher version of the dll. The workaround is described #7, very manual solution.

https://github.com/xoofx/UnityNuGet is a project trying to solve the issue by packing the NuGet dlls into UPM packages. It is also something the OpenUPM will cover in the later stage.

sebas77 commented 4 years ago

Thank you for your reply. I have already a working work around, but the user must be aware of it and act accordingly depending if they use that package or not, hence I have to explain on the wiki what to do, which is not ideal. If upm would automatically manage it , it would be exceptional!

On Sat, 4 Jan 2020, 04:15 Favo Yang, notifications@github.com wrote:

I just found a post https://forum.unity.com/threads/please-update-com-unity-collections-reference-of-system-runtime-compilerservices-unsafe.732056/, complains about the issue of manually import the dll into the com.unity.collections@0.1.1-preview package. The thread poster need a higher version of the dll. The workaround is described #7 https://forum.unity.com/threads/please-update-com-unity-collections-reference-of-system-runtime-compilerservices-unsafe.732056/#post-5015006, very manual solution.

https://github.com/xoofx/UnityNuGet is a project trying to solve the issue by packing the NuGet dlls into UPM packages. It is also something the OpenUPM will cover in the later stage.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sebas77/Svelto.ECS/issues/45?email_source=notifications&email_token=AAHGZYZ7WYP6FHVUWKCAJOTQ4AEPZA5CNFSM4KB3VSDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICQQJA#issuecomment-570755108, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHGZY4HDXMX54YDKSPK44DQ4AEPZANCNFSM4KB3VSDA .

sebas77 commented 4 years ago

@favoyang there is a problem and that's the reason I actually stopped spending time with UPM in the first time: UPM needs me to put the meta files in the repository too. However Svelto is not a Unity centric project and doesn't need meta files. I could change that, but I very much dislike the idea. Thoughts?

favoyang commented 4 years ago

@sebas77, Sorry that I didn't recognized that the project is not Unity centric at the first place. Because of you mentioned the Unity.ECS, I thought it was an alternative. The UPM is totally a Unity thing. The meta files are required and should be consistent for all releases (Unity use the GUID stores in meta file to recognize file).

May I know what engine the project majorly designed for?

Workaround options:

  1. Commit all meta files to the repo to make the project “Unity centric". All Unity libraries did that, like or not.
  2. Disable the UPM support
  3. Disable the UPM support and Let community to support the upm by forking all your repo with the suffix "-upm", adding meta file, manage releases.
  4. Maintain an upm branch your own. Create a branch named upm, add meta files. Regular merge from master (via CI), adding meta files (manually). When do Github Releases, the x.y.z tag from master, the x.y.z-upm tag from upm branch (via CI). semantic-release-action could help some of the works. I just help a project convert to upm, the author use the tool to automaitc some workflows.
sebas77 commented 4 years ago

thanks, Svelto is not unity centric, but it's obviously used mainly with Unity. I could add the meta files, as at the end of the day they won't affect not Unity projects. The idea of using another repository would work, but it would be confusing and hard to maintain. I won't remove the UPM support because it's something I wanted to do anyway.

sebas77 commented 4 years ago

BTW, in my opinion this is a UPM design mistake. Unity should generate the .meta files when are not found.

favoyang commented 4 years ago

Unity did that for scripts in Asset folder. For UPM installed via registry, placed at Library/PackageCache/ folder, Unity treat it as read only assets, and keep warning the the missing meta files.

The major benefit for meta file (and the GUID behind) is that you can move script without breaking reference in editor or prefab or scriptable objects. So they unlikely to generate them for you without a warning.

sebas77 commented 4 years ago

yes sure, it's also needed because UPM support assets, but it shouldn't treat as an error the lack of meta files, because of cases like mine

favoyang commented 4 years ago

True. print warning once is enough.

BTW, 2.9.1.1 is not semantic version (the build pipeline will ignore it). 2.9.2 is. see https://semver.org/

sebas77 commented 4 years ago

ok fixing it thanks

sebas77 commented 4 years ago

Fixed, will it work?

favoyang commented 4 years ago

2.9.1.1 is not semantic version (the build pipeline will ignore it). 2.9.2 is. see https://semver.org/

sebas77 commented 4 years ago

I fixed it, I change the release tag, will it work?

sebas77 commented 4 years ago

I forgot to update the manifest :(

favoyang commented 4 years ago

add package.json also need meta file.

favoyang commented 4 years ago

. I just help a project convert to upm, the author use the tool to automaitc some workflows.

These links are worth to checkout to automate the whole process.

sebas77 commented 4 years ago

interesting thanks, I don't use these kind of automatism as Svelto is a simple project. I will need to keep in mind the process at the moment. BTW I think I have fixed it, not sure.

favoyang commented 4 years ago
sebas77 commented 4 years ago

OK, so moving the tag is not enough? I hoped it would have been enough

favoyang commented 4 years ago

I don't know what you mean by moving tag? You can rename a tag in GitHub UI, but it won't change the tagged commit. i.e. https://github.com/sebas77/Svelto.ECS/releases/tag/2.9.2 is point to https://github.com/sebas77/Svelto.ECS/commit/91beb508c4087ab57748c6f0ae5d6f6ae4804b67, your latest commit is https://github.com/sebas77/Svelto.ECS/commit/88d8fe61953a1ba9c0064ef179bc735ffc554c8d

However you can

1) delete github release, git tag and recreate a tag with same version (but this should be a rare practice, because of the DVCS system).

2) Or bump version and create new git tag/releases.

sebas77 commented 4 years ago

OK. I don't use git professionally and I don't even like it that much, so the little I know, I know from sourcetree, which is not that great. Source tree has a move tag, but it doesn't work anyway without deleting the original one from the remote repository. So I did it. Deleted, recreated and releases updated.

sebas77 commented 4 years ago

Now that the release works, I did this:

e:\node-v12.14.0-win-x64\openupm add com.sebaslab.svelto.ecs@2.9.2 added: com.sebaslab.svelto.ecs@2.9.2

which added this in the manifest:

"com.sebaslab.svelto.ecs": "2.9.2" }, "scopedRegistries": [ { "name": "package.openupm.com", "url": "https://package.openupm.com", "scopes": [ "com.openupm", "com.sebaslab.svelto.ecs" ] } ]

however it doesn't seem to have solved the common dependency, what is wrong now? Do I have to add common manually with another OUPM call?

favoyang commented 4 years ago

I will run a test soon. However the dependency is resolved by unity internally and not reflected into the manifest file.

sebas77 commented 4 years ago

OK I'll try again, the first time it didn't work (I add to import Svelto.Common manually)

sebas77 commented 4 years ago

OK second test worked. It's all good now then! Thank you! I'll do more research on UPM and current state and maybe write a little article about this update

favoyang commented 4 years ago

Ahh, it is a bug, not all dependencies was added to the scopes. So manually add Svelto.Common works for now until I fix the bug.

favoyang commented 4 years ago

You can track it here - https://github.com/openupm/openupm-cli/issues/1

In future, please always bump the version to fix an error. i.e. you find anything wrong you have to release 2.9.3.

sebas77 commented 4 years ago

OK. In future, if needed (and only if needed) I may create a UPM specialized branch, so that I will put the .meta files only there. would it be OK?

favoyang commented 4 years ago

It will work, as long as you create GitHub release based on the upm branch.

If you do want to release both master and upm branches, try this strategy:

sebas77 commented 4 years ago

I noticed you support package.json not in the root folder, how did you make this work? I don't think it's officially supported by UPM as an option.

favoyang commented 4 years ago

The build pipelines detect the path of the package.json and bundle the package from there.

Some package owners maintain an upm branch by using a git subtree to split the package folder (again more git magic). So they can install the package via git url. While there're more details to keep the subtree update/merge, as well as how to resolve dependencies from git url.

The links of https://github.com/MirrorNG/MirrorNG above, uses all the tips there. But it takes some time to understand every pieces of it. It's one benefit of OpenUPM platform, you focus on package itself, let the platform deal with the packaging.

sebas77 commented 4 years ago

OK I am understanding more and more about it. I create an UPM branch, which has the meta files, but I removed the Svelto.Common submodule and moved Svelto.ECS in the root folder. If I create a new release (later), will this new configuration create any problem?

favoyang commented 4 years ago

It shall just work.

sebas77 commented 4 years ago

I noticed that you set the type of the package as library, but Svelto is more a framework, does it matter if I change it?

Edit: I also noticed this:

name: com.sebaslab.svelto.ecs displayName: Svelto ECS description: 'Svelto ECS C# Lightweight Data Oriented Entity Component System Framework' repoUrl: 'https://github.com/sebas77/Svelto.ECS' repoBranch: master packageFolder: Svelto.ECS parentRepoUrl: null licenseSpdxId: MIT licenseName: MIT License topics:

should the packageFolder change now?

sebas77 commented 4 years ago

@favoyang an observation: even if you simplified it as much as possible, the process is frustrating as I always get something wrong; it can be the dependency version, can be the wrong branch, can be the package not updated correctly. Validating the package after it's uploaded is awkward, especially since to fix it I have to create a new release. It's absolutely necessary to have a tool to validate if what I have done is OK before the release is created. Alternatively I would need a tool to ask to your system to try to repackage a specific release after I fix what's wrong. I have created a ton of releases, probably it would be useful to be able to detect deleted releases from github, but I understand this should be an exception (like in my case, since I created 4 releases because of several mistakes I made). All that said, I created Svelto.ECS 2.9.5 with a dependency to Svelto.Common 2.9.4 and Svelto.Common is still not automatically installed, I have to add it manually (this time I didn't reuse tags).