-
Notifications
You must be signed in to change notification settings - Fork 54
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
base: development
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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) |
There was a problem hiding this comment.
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.");
}
There was a problem hiding this comment.
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.
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