superunitybuild / buildtool

A powerful automation tool for quickly and easily generating builds with Unity.
MIT License
1.19k stars 121 forks source link

Dependency Fixes, New Settings, and UX Overhaul #112

Closed RobProductions closed 1 year ago

RobProductions commented 1 year ago

Hi, so as promised I've taken a look at quite a few things regarding UX and have a huge PR to present today. Apologies again for the long post but as usual I want to make sure each addition is reviewed and looks good before merging. I'm open to any feedback or better ways of handling certain things so definitely let me know if you have any suggestions! Though like before, I tried to follow the existing structure as best possible; hopefully it's mostly up to par :)

Bugfixes

First, this PR fixes #111 by manually implementing SkipLast() as an extension so that the package can be run on .NET Standard 2.0 in Unity 2020.3. I'd definitely want SuperUnityBuild to support 2020.3 for as long as possible as I see it as a landmark version and the 2022 LTS only just came out this week haha.

New: Scene List Overhaul

image

The UI has been revamped to show a non-editable label as the scene name and then a subtitle which shows the full path of the scene in case you have 2 of the same name somehow. This made more sense to me because all of the scene buttons give the user plenty of control already and you wouldn't want to be editing the path accidentally. I believe it's a bit more stable this way as it appears to the user as more of a guarantee that the scene list is holding actual scenes and not just some string.

There is now an additional bool in the scene data holder to track whether each scene is "active" or not. This is meant to mirror Unity's "Build Settings" window which lets you use the scene list as a sort of "workspace" to quickly disable scenes without having to remove and re-add them later. In order to make this work, I refactored the getter to only return scenes that are enabled like so:

 public string[] GetActiveSceneFileList()
        {
            List<string> scenes = new List<string>();
            for (int i = 0; i < releaseScenes.Count; i++)
            {
                var thisScene = releaseScenes[i];
                if(!thisScene.sceneActive)
                {
                    //Don't return inactive scenes
                    continue;
                }
                scenes.Add(SceneGUIDToPath(thisScene.fileGUID));
            }

            return scenes.ToArray();
        }

The "Add Scene Files From Build Settings" button now also takes into account the "active" status of your Build Scenes. New scenes that are added this way will be given the same "active" status they had in the build settings.

Additionally, there is now an "Add Current Scene" button which I thought was useful. This is similar to Unity's build window which lets you add open scenes quickly. I reordered the scene buttons to flow from most used to less used; another UX principle that helps to avoid people accidentally removing all their scenes.

One last aspect of this is in the special case for "Configure Editor Environment" where we don't just call BuildPlayer with the enabled scenes but actually have to set the Build Settings scenes according to what's in the release. In this case, Build Settings also retains the "active" data per scene and translates it back into an Editor Build Scene with the correct enabled status.

Unfortunately, this is already a breaking change which necessitates a new major version because of the new data involved with the scene list. When I tried it with my old settings, my scene list got cleared, so that's something to note.

Tweak: Delete Release Button

image

Following the convention of other parts of the package, releases now have a header button to remove them instead of the old "Delete Release" button which sat at the bottom of each release box.

New: Customize App Build (final binary) name

I really liked the idea proposed in #108 so I implemented it as well; definitely a useful feature since this functionality is possible in manual builds (by selecting the name of the final .exe file which can be different than your Product name).

image

Now there's a "sync app name" button similar to the version sync which is on by default. When it's on, the final exported binary/bundle will match the Product name, and when it's off you get to customize it. This allows you to create builds that have different file names than the PlayerSettings Product:

image

New: Customize BuildConstants File Location

image

There's now a setting in the BasicSettings which lets you choose the path where BuildConstants.cs will be generated. Previously, the package would search for BuildConstants.cs and use that location for the generated file, so if you moved it to another spot it would handle that. This feature still exists and had to be changed to account for the variable path. Now it checks for the exact file name BuildConstants.cs instead of the last part of the path:

            string[] fileSearchResults = Directory.GetFiles(Application.dataPath, FileName, SearchOption.AllDirectories);
            string filePath = null;
            char[] separatorChars = new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar };
            for (int i = 0; i < fileSearchResults.Length; i++)
            {
                var thisFilePath = fileSearchResults[i];
                var thisFilePathSplit = thisFilePath.Split(separatorChars);
                if(thisFilePathSplit.Length > 0)
                {
                    if(thisFilePathSplit[thisFilePathSplit.Length - 1].Equals(FileName))
                    {
                        filePath = thisFilePath;
                        break;
                    }
                }
            }

When the generator doesn't find an existing file, it uses the location specified in settings, which defaults to Assets/SuperUnityBuild like before. This means new users will still get the file in the previous place, but you can customize the path to get it to generate in a new location.

You can test this by running "Configure Editor Environment" on a specific configuration to generate the file. Like before, when the user runs a build, their old file will be copied into Temp and then restored which allows Editor runs to rely on consistent settings once you generate it for the first time.

This also means that the default "SuperUnityBuild" folder is no longer generated unless the user has the default settings applied for this path option. I might be wrong, but this was actually the reason why users kept seeing it pop up; the folder was wrongly generated even if you move the BuildConstants.cs into another place. Thus, this handles #76 and it handles #101.

Detour: Default Folder Generation

Let's talk about the SuperUnityBuild folder! With this PR, there is only one case where this is generated besides the BuildConstants generator, and that's in the BaseSettings CreateAsset function:

image

From what I can tell, this is a "default settings" creation step which occurs when users open the SuperUnityBuild window without a "current BuildSettings" set in the Editor Prefs. Therefore, the default folder will appear only when a user opens the window for the first time without any other settings to reference. On subsequent selection of other settings, you can safely delete the SuperUnityBuild folder without consequence.

There is one other case where this can happen though, and that's when a settings file you're working with gets deleted. In that case, the "default settings" will be created again since the window refuses to work with null. This is fine for now, but just thought I should mention it. Again, this handles #101 as the user can safely delete it once they have their own settings.

New: Double Click to Open BuildSettings

You can now double click on a BuildSettings to automatically set the "current BuildSettings" instance and open the SuperUnityBuild window! This was accomplished through the OnOpenAssetAttribute which loads the asset at the desired path and then sets BuildSettings.instance and shows the window.

Additionally, the Inspector button for BuildSettings now does the same:

image

This should eliminate confusion over working with the wrong build settings due to pressing on the button and expecting it to open the file you've selected.

Fix: Scripting Backend Restored after Build

One step I forgot to include in my last PR was to make sure the original scripting backend was restored after it's set by ConfigureEnvironment(). For example, if you had Mono selected in Player Settings but ran a build with IL2CPP, the Editor would be changed to IL2CPP.

I noticed the package was already doing settings restoration for things like the Product name so I just tacked it on like this:

// Save current environment settings
string preBuildDefines = PlayerSettings.GetScriptingDefineSymbolsForGroup(platform.targetGroup);
string preBuildCompanyName = PlayerSettings.companyName;
string preBuildProductName = PlayerSettings.productName;
string preBuildBundleIdentifier = PlayerSettings.GetApplicationIdentifier(platform.targetGroup);
ScriptingImplementation preBuildImplementation = PlayerSettings.GetScriptingBackend(platform.targetGroup);

And then applied it after the build was done like the other settings.

Tweak: Button styling and colors

I've streamlined a few UI elements like the Build buttons to use a consistent customizable color, and chose a color that's less... blinding. Less saturation makes it easier on the eyes :) Same for the Log header:

image

Various refactoring

I changed the Sanitize functions into string extensions which was a TODO comment, so hopefully that helped make the code a bit cleaner!

Conclusion

That's everything I've worked on for this PR so hopefully these are useful changes. I'm biased of course but I certainly think the package is way more viable for me as a user with these QOL adjustments. Feel free to let me know if there's anything I missed during refactoring or if there's any other issues.

Finally, I'd like to mention a few ideas for further enhancements down the road:

Maybe sometime in the future I can take at those ideas and any others proposed in the Issues list. For now though I think this is a good start to some cleaner UX. Let me know what you think! Thanks :)

robinnorth commented 1 year ago

Hi @RobProductions, thanks very much for this PR, it looks like you've put a lot of work into it and seems like there's some really useful stuff in it!

However, I would prefer it if you could split some of these changes out into smaller PRs, rather than sending one monolithic PR as it'll make reviewing and testing a lot easier, which ultimately means it'll be quicker for me to approve and merge them. Things like the fix for #111, restoring the scripting backend after build, changes to the build settings and build constants files and default folder generation could and should be made as separate PRs as they are separate to the UI/UX improvements that are your intended area of focus for this PR.

I also have some pending changes to push where I have reworked the string tokens system, which includes implementing the string sanitizing functions as string extensions, so you will also need to rebase your work on mine when that's done, if that's OK. I don't think that any merge conflicts will be too difficult to resolve, luckily.

Regarding the UI/UX changes themselves, I will take a closer look when I have a bit more time and see if I have any additional comments, but at first glance they all look like welcome improvements. I do have one question for now, which is to ask if the colour changes been tested with the dark Editor theme as well as the light theme as shown in your screenshots.

Your suggestions for future improvements all sound sensible and useful too, especially the change to the default build settings asset generation.

Once again, thank you very much for all of your effort on this so far, contributions like this are really appreciated! If you could look at splitting some of these changes out into separate PRs as a next step, that would be great. I will push my pending changes to the development branches of this repo and the Build Actions one so that you can pull them and rebase your work accordingly.

Cheers!

robinnorth commented 1 year ago

@RobProductions I have pushed my pending changes to the dev branch, so please rebase on that rather than master.

RobProductions commented 1 year ago

Hi @robinnorth , sorry about that; maybe it's just a quirk of the way I work because I don't think about individual features and instead make incremental progress on a bunch of different things at once when I get the idea for it. i.e. not all of the features have sequential progress and that's my mistake for being too lazy to switch branches haha.

Since some of my commits have fixes for earlier features it'll be tough to pull them out into separate PRs as I'll need to grab only some file changes within certain commits too, but I can definitely try. I'll get the fixes as their own PR first if that sounds okay and then make a few additional ones for the new stuff. In the meantime this PR can be used as reference for the different changes so that I don't have to write new descriptions :)

No worries about the sanitize stuff either, I'll keep your implementation intact when I handle the conflict. Just to let you know, I think for me the easier approach for splitting the PR will be to first grab your changes, put them into the new PR branches, and then cherry pick commits from my dev to recreate the new stuff. Since you might still have more to review on dev I'll target upstream/dev for the new PRs too and then you can merge that to main when you feel like it's ready. I'll try that out now and also make sure to test dark mode for the color adjustments too. Thanks!

RobProductions commented 1 year ago

@robinnorth , I believe I've handled all the changes as smaller PRs now. As mentioned in #113 , I had to include the fixes in all the other PRs because I couldn't compile to test otherwise. If you review that one first I believe the "changed files" of the other PRs will be reduced which should make it easier to review those. Once everything is reviewed and merged I can rebase upstream dev into my own dev branch and test that again to make sure it's the same as when I started. Thanks, and feel free to leave suggestions if you have any! :)

robinnorth commented 1 year ago

@RobProductions amazing work, thanks very much for all of this! I'll take a look through all of the new PRs and get back to you when I have done so.