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 paket-unity UPM mode [ATLAS-1273] #96

Merged
merged 4 commits into from
Jul 27, 2023
Merged

Conversation

Joaquimmnetto
Copy link
Contributor

@Joaquimmnetto Joaquimmnetto commented Jul 25, 2023

Description

Paket packages can now be configured to be installed as UPM packages. This can be useful when your paket package contains an UPM one (with a package.json file), and you want it to be integrated with Unity's packages folder.

This mode sets the installation directory (paketOutputDirectoryName) to the unity project's Packages folder, and ensures that the package.jsonfile for each dependency is in the installed package root.

If an installed package is not UPM-compatible, that is, it doesn't have a package.json file, such file will be created. By default, a package.json is generated with com.wooga.nuget.${paketPackage.toLowerCase()} name, and version 0.0.0, but those can be overridden and more properties can be added on using the mappings in paketUpmPacakgeJson.

UPM mode can be enabled through the paketUpmPackageEnabled property, or using the paketUnity.enablePaketUpmPackages() extension method. It is disabled by default.

Changes

  • NEW Packages can now be installed in UPM mode in paket-unity plugin
  • ADD UPM mode in PaketUnityInstall task
  • ADD paketUpmPackageEnabled and paketUpmPacakgeJson properties to PaketUnityPluginExtension

@Joaquimmnetto Joaquimmnetto self-assigned this Jul 25, 2023
@Joaquimmnetto Joaquimmnetto changed the title Adds paket-unity UPM mode Adds paket-unity UPM mode [ATLAS-1273] Jul 25, 2023
@Joaquimmnetto Joaquimmnetto changed the title Adds paket-unity UPM mode [ATLAS-1273] Add paket-unity UPM mode [ATLAS-1273] Jul 25, 2023
Comment on lines 17 to 21
@Input
@Optional
MapProperty<String, Map<String, Object>> getPaketUpmPackageManifests() {
return paketUpmPackageManifest
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo, an s at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its not a typo, there may be many manifests. Its a map of [paket_name: upm manifest] after all.

Copy link
Member

@pletoss pletoss left a comment

Choose a reason for hiding this comment

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

lgtm.

I have one concern i'd like to clarify first before merging. please see comment directly in code.

if (isPaketUpmPackageEnabled().get()) {
def references = new PaketUnityReferences(getReferencesFile())
def locks = new PaketLock(getLockFile())
def packages = locks.getAllDependencies(references.nugets).collect {
Copy link
Member

Choose a reason for hiding this comment

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

one thing that we need to make sure is, that ONLY whatever was being installed in Assets/Paket.Unity3D gets installed in Packages/ too, and nothing more. So if there is any dependency of something referenced in the unity3d reference file but it would have NOT be installed in there, it should not be installed in Packages either.

My concern is to remind you that paket actually only copies the .net dll variants that are defined/supported inside Paket.Unity3D.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pletoss this task is the exact same one the installs stuff in Assets/Paket.Unity3D. All I'm doing there is changing the installation directory and making sure that the package.json file is in the right place. So don't worry about this ;)

@Joaquimmnetto Joaquimmnetto merged commit 79d4391 into master Jul 27, 2023
This pull request was closed.
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.

3 participants