UX Proposal: Eliminate reliance on SuperUnityBuild folder in /Assets
Closed this issue · 4 comments
Hi, as we discussed in the PR #91 , I had a few suggestions regarding UI/UX which I believe could make the project more user-friendly. I had some other ideas for UI improvements which shouldn't be too controversial such as improving the "reordering" system and making some buttons more consistent which I can just take a look at in a new PR, so for this Issue I'd like to focus on a specific UX adjustment as it'd be nice to make sure everything sounds right.
Current Implementation
From what I understand, the following folder is generated when you install/use buildtool for the first time:
This folder is used to create a default settings file, a place for your BuildActions to sit, and the BuildConstants file which user code can reference at a later time. Based on my testing, if the folder is missing, SuperUnityBuild will regenerate it once you start a Build which means it always exists under /Assets.
The Main Idea
As a user I'm actually not keen on this folder being forced into my Assets directory especially when there's no customization for it. The behavior I'm expecting as derived from Unity "official" packages is to be able to manually define the location of the files the impact the package, i.e. the InputSystem uses an InputActionAsset which can be located anywhere you want and the other package settings are hidden away somewhere that doesn't mess with /Assets. This is mainly my own bias speaking haha but my preference is that the /Assets folder hierarchy should be left completely up to the user (apart from Unity's Special Named folders) and files that are needed by packages should be placed anywhere the user wants.
In summary, I'd like to get rid of this folder entirely and let the user control where the files in here are located, if they truly are necessary. As an alternative, it'd still be nice to include an option to generate this folder (as it helps first-timers to understand what they may need to work with buildtool), but it shouldn't be forced to generate on each build imo.
Adjustments needed for this idea
So to break this down there are 3 obstacles standing in the way of this change and each one has its own quirks/considerations which I want to discuss individually.
Build Actions
According to the docs,
To create a new BuildAction:
Create a class that inherits from BuildAction. You must place this class in an 'Editor' folder or include it in an assembly that supports the 'Editor' platform for it to run.
So theoretically the Editor folder contained in here isn't needed at all and the user can make their BuildActions wherever they want in a different Editor folder. I know the folder exists an example but with the "generate SuperUnityBuild folder" option I proposed above, I think that should be enough to cover that case and users that know what they're doing won't need this.
Default Settings
As its entirely possible to create your settings from the Asset dropdown and you can customize the current settings as seen here:
it stands again that the default settings are simply an example and share the same fate as the BuildActions folder. So perhaps if no settings are found when you open the window, a button could appear saying "Generate Default Settings" and it will make it if it's missing. Otherwise it shouldn't be generated automatically as a user may have their own settings file which is stored somewhere else and there's no need to confuse them with multiple settings files.
Additionally, I've noticed that when you click this button on a specific setting file:
It simply opens the SuperUnityBuild window without actually associating it with the settings file you clicked on. i.e. it opens the last settings file you linked to the window using that "selector" at the top. This has definitely confused me before and I started editing the wrong file by mistake. A great enhancement would also be that when you double-click or use this Open button, it automatically lets you start editing that file specifically.
Build Constants
This is the most complicated adjustment but also an important one, because I have a few nitpicks on this file in general:
- Perhaps it should use the SuperUnityBuild namespace to clarify the source of these constant values? I'm not too certain about this one but it's also possible that BuildConstants is a pretty common class name that a user may already have, so a namespace makes sense to further abstract it, though I realize that messes with ease of use.
- The file is only generated once you make a build, which means you can't pull from those constant values in your code until it exists. If I'm understanding correctly, user code will fail to compile until they make at least one build.
- Even after a user makes a build, it's locked in until you make the next build. That means if you run the game in the Editor and user code relies on these constants, they're working off of outdated info. Ideally a user accounts for these situations with editor checks or writes a post-action that grabs the latest build info and updates when necessary, but it's a little inconvenient to do that.
I'm just spitballing here but to solve that last issue it may pay off to have some sort "Polling API" which the user can use to grab the current environment/distribution even in Editor mode, and that might necessitate some sort of "editor environment" setting in the build settings. And I realize that constants are faster and more preferred than some sort of state management but that's something to think about for the future. An alternative could be to generate BuildConstants on build AND when build is done so that Editor environment can be recognized too.
Now for actually removing the reliance on this file location, I see a few possible approaches:
- Let BuildConstants generate within the package files itself and .gitignore it. This would be quite challenging because it's difficult for the package to understand where exactly it's installed; it can be installed in 3 different ways: Remote Install (git), /Packages embed, or directly into Assets. The package wouldn't know which installation type the user chose so it would have to search for the location or be given the location from the user or some file check.
- Rework BuildConstants so that it's no longer a generated file and simply passes data to the user in some other way, maybe through a local state or through Scripting Define Symbols(?). This is pretty vague so I understand why this isn't preferred either.
- Add a new param to BuildSettings which lets the user define where BuildConstants is generated, maybe with a user-friendly file picker too. I believe this is the best option as it provides the most customization and isn't too destructive to the way BuildConstants already works. It also lets you specify different locations per Settings file which may be useful for some people, though I realize it would cause a compile error if they share the same name. I think it should be fine though as people will understand this fact. This would be a great opportunity to add in more management options in the UI for this file though like a "generate now" button so that users won't have to build to see it appear.
Conclusion
With those 3 obstacles solved I believe that eliminates all reliance on the SuperUnityBuild folder and it could stop automatically generating at that point. As explained, we could still have a button that lets users generate it as an example and default locations for BuildConstants and DefaultSettings could still point to that folder so that there's a smoother transition from the way it previously worked. But these new customization options would go a long way to making the project feel more viable for me as I'd really love to keep my folder structure intact :-)
Apologies for the long post but I definitely wanted to be thorough here. Let me know what you think! If it all sounds good and option number 3 is preferred for BuildConstants, I can worry about the implementation details and just go ahead with these changes in a new PR. Thanks and I hope this suggestion helps to improve SuperUnityBuild even further!
I wrote this entire post and then realized that #76 is pretty similar haha. So if we go ahead with this idea I believe it will solve that issue as well as users will be able to simply move the folder and have everything point to the new location.
Thanks for taking the time and effort to write this up, @RobProductions. There's a bunch to digest here, so I will need to get back to you with a more thorough response when I have the time, but I broadly agree with the motivation behind this: as you say, the precedent set by Unity's recent packages is to allow serialized settings files to be placed anywhere in the Assets directory, but often combined with a sensible default location. I definitely think SuperUnityBuild should follow that convention.
I will write up some thoughts on your different adjustment proposal options and let you know what I think would work best, so that you can then work up a PR. Cheers!
Hi @robinnorth , I believe this issue is solved now thanks to #116 and can be closed if there's no other suggestions! Thanks for reviewing the code :)
Nevermind I can close it lol. Didn't realize that