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

feat(upm,tool) : make modular tool in into its own window #645

Draft
wants to merge 5 commits into
base: development
Choose a base branch
from

Conversation

arthur740212
Copy link
Collaborator

This is a PR for the modular UPM build tool.
This version is made with the Editor Windows changes on version 3.1.0
And instead of applying it on top of the existing upm creation window, it has own window such that no conflicts will occur.
Also to have flexibility during this transition phase

image

Copy link
Collaborator

@paulhazen paulhazen left a comment

Choose a reason for hiding this comment

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

There are a few concerns I have about this PR:

Overlap with Create Package Window

I'm concerned this PR could complicate our support matrix. Introducing a new way to create modular packages might lead to discrepancies in package functionality between the CreatePackageWindow and the new modular method, potentially introducing bugs. For instance, packages created on different platforms through different tools could behave inconsistently.

Additionally, updating package descriptions would require changes across multiple JSON files if new files are added to the plugin source. However, these issues could be mitigated by modifying the existing CreatePackageWindow to include an expandable section for platform selection, ensuring uniformity in package creation code.

Clarity around use cases

I need further clarification on when and why users might limit platform support for plugins. For example, if a package is created for Windows and later needs iOS support, this could increase our support tasks significantly. Despite potential file size benefits, it seems deploying games only includes necessary binaries for the chosen platform, so what's the added value of this modular approach?

Changes to UnityPackageCreationUtility

Regarding the changes to make certain methods public in the utility, I believe this could bypass important common tasks handled in the CreatePackage function. It might be more prudent to refactor CreatePackage to incorporate desired flexibility rather than altering method access.

Sorry for being so long-winded 😆

}

const int BYTES_TO_READ = sizeof(Int64); //check 4 bytes at a time
static bool FilesAreEqual(FileInfo first, FileInfo second)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function can (and I think should) be removed - its functionality was put into a FileInfoExtensions class. You can find it at Assets/Plugins/Source/Core/Utility/Extensions/FileInfoExtensions.cs. Once you have that class included via a using statement that includes it, all you need to do is use it:

FileInfo fileOne = new FileInfo(path1);
FileInfo fileTwo = new FileInfo(path2);

if (fileOne.AreContentsSemanticallyEqual(fileTwo))
{
    Debug.Log("The contents of the files are semantically identical.");
}
else
{
    Debug.Log("The contents of the files are _not_ semantically identical.");
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like this feature might be a good candidate for our internal new feature runbook.

@paulhazen paulhazen marked this pull request as draft June 10, 2024 22:16
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