-
-
Notifications
You must be signed in to change notification settings - Fork 268
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
Support activeBuildProfile parameter #674
Comments
Build Profiles is a relatively new feature and would be a great addition. 👍 |
This would be incredibly useful |
@coderabbitai read the following files and suggest an approach to add support for activeBuildProfile parameter for unity version >= 6 (6000) dist/default-build-script/Assets/Editor/UnityBuilderAction/Builder.cs |
Tip For best results, initiate chat on the files or code changes. Based on the files analyzed, I'll suggest an approach to add support for the activeBuildProfile parameter for Unity version >= 6. Here's what needs to be changed: @GabLeRoux Here's the proposed approach:
buildProfile:
required: false
default: ''
description: 'Path to the Unity Build Profile asset file. When specified, targetPlatform will be ignored as the build target is defined in the profile.'
static get buildProfile(): string {
return Input.getInput('buildProfile') ?? '';
}
public static Dictionary<string, string> GetValidatedOptions()
{
Dictionary<string, string> validatedOptions;
ParseCommandLineArguments(out validatedOptions);
string projectPath;
if (!validatedOptions.TryGetValue("projectPath", out projectPath)) {
Console.WriteLine("Missing argument -projectPath");
EditorApplication.Exit(110);
}
// Check if buildProfile is specified
string buildProfile;
bool hasBuildProfile = validatedOptions.TryGetValue("buildProfile", out buildProfile) && !string.IsNullOrEmpty(buildProfile);
// Only require buildTarget if buildProfile is not specified
if (!hasBuildProfile)
{
string buildTarget;
if (!validatedOptions.TryGetValue("buildTarget", out buildTarget)) {
Console.WriteLine("Missing argument -buildTarget (required when buildProfile is not specified)");
EditorApplication.Exit(120);
}
if (!Enum.IsDefined(typeof(BuildTarget), buildTarget)) {
Console.WriteLine(buildTarget + " is not a defined " + typeof(BuildTarget).Name);
EditorApplication.Exit(121);
}
}
// Rest of the validation...
}
public static void BuildProject()
{
// Gather values from args
var options = ArgumentsParser.GetValidatedOptions();
// Gather values from project
var scenes = EditorBuildSettings.scenes.Where(scene => scene.enabled).Select(s => s.path).ToArray();
// Get all buildOptions from options
BuildOptions buildOptions = BuildOptions.None;
foreach (string buildOptionString in Enum.GetNames(typeof(BuildOptions))) {
if (options.ContainsKey(buildOptionString)) {
BuildOptions buildOptionEnum = (BuildOptions) Enum.Parse(typeof(BuildOptions), buildOptionString);
buildOptions |= buildOptionEnum;
}
}
// Define BuildPlayer Options
var buildPlayerOptions = new BuildPlayerOptions {
scenes = scenes,
locationPathName = options["customBuildPath"],
options = buildOptions,
};
// If buildProfile is specified, add it to customParameters
if (options.TryGetValue("buildProfile", out var buildProfile) && !string.IsNullOrEmpty(buildProfile))
{
if (options.ContainsKey("customParameters"))
{
options["customParameters"] += $" -activeBuildProfile {buildProfile}";
}
else
{
options["customParameters"] = $"-activeBuildProfile {buildProfile}";
}
}
else
{
// Use traditional buildTarget approach
buildPlayerOptions.target = (BuildTarget)Enum.Parse(typeof(BuildTarget), options["buildTarget"]);
}
// Rest of the build process...
}
## Using Build Profiles (Unity 6+)
For Unity 6 and above, you can use Build Profiles instead of specifying a target platform. Build Profiles provide more control over your build settings and can be managed through Unity Editor.
Example workflow:
```yaml
- uses: game-ci/unity-builder@v4
with:
buildProfile: 'Assets/Settings/BuildProfiles/WindowsDevelopment.asset'
buildName: 'My Game'
buildsPath: build Note: When using
#!/bin/bash First, let's check the action.yml to see the current input parametersecho "=== action.yml ===" Let's look at the input handling in TypeScriptecho -e "\n=== src/model/input.ts ===" Check how build arguments are constructedecho -e "\n=== src/model/unity.ts ===" Check the Builder.cs which handles the actual buildecho -e "\n=== dist/default-build-script/Assets/Editor/UnityBuilderAction/Builder.cs ===" Check how arguments are parsedecho -e "\n=== dist/default-build-script/Assets/Editor/UnityBuilderAction/Input/ArgumentsParser.cs ==="
Learnt from: GabLeRoux Timestamp: 2024-12-05T17:12:45.657Z
|
Is there a CLI flag for it? If yes, then that's probably preferable over modifying the build script itself. If no, then this looks exactly how we should solve it. Happy to take a PR for this! |
@coderabbitai There is an existing command line argument for this on the unity cli, it is #!/usr/bin/env bash
set -euo pipefail
echo "Building for '$BUILD_TARGET'"
# Ensure the build path is defined and exists
export BUILD_PATH="$UNITY_DIR/Builds/$BUILD_TARGET/"
mkdir -p "$BUILD_PATH"
# Base Unity command split into an array
mapfile -t UNITY_CMD < <(
printf "%s\n" \
"${UNITY_EXECUTABLE:-xvfb-run}" \
"--auto-servernum" \
"--server-args=-screen 0 640x480x24" \
"unity-editor"
)
# Base Unity arguments
mapfile -t UNITY_ARGS < <(
printf "%s\n" \
"-projectPath" "$UNITY_DIR" \
"-quit" \
"-batchmode" \
"-nographics" \
"-customBuildTarget" "$BUILD_TARGET" \
"-customBuildName" "$BUILD_NAME" \
"-customBuildPath" "$BUILD_PATH" \
"-executeMethod" "BuildCommand.PerformBuild" \
"-logFile" "/dev/stdout"
)
# Adjust arguments based on whether a build profile is provided
if [[ -n "${BUILD_PROFILE_PATH:-}" && -f "$BUILD_PROFILE_PATH" ]]; then
echo "Using build profile: '$BUILD_PROFILE_PATH'"
UNITY_ARGS+=("-activeBuildProfile" "$BUILD_PROFILE_PATH")
else
echo "No build profile specified or the file does not exist; proceeding with default build target."
UNITY_ARGS+=("-buildTarget" "$BUILD_TARGET")
fi
# Print arguments for debugging
echo "Unity Command: ${UNITY_CMD[*]}"
echo "Unity Arguments: ${UNITY_ARGS[*]}"
# Run the Unity command with arguments
"${UNITY_CMD[@]}" "${UNITY_ARGS[@]}"
UNITY_EXIT_CODE=$?
# Check the Unity exit code and display appropriate messages
if [[ $UNITY_EXIT_CODE -eq 0 ]]; then
echo "Build succeeded with no failures."
elif [[ $UNITY_EXIT_CODE -eq 2 ]]; then
echo "Build succeeded, but some tests failed."
elif [[ $UNITY_EXIT_CODE -eq 3 ]]; then
echo "Build failed due to other reasons."
else
echo "Unexpected Unity exit code: $UNITY_EXIT_CODE."
fi
# List the build output directory contents
ls -la "$BUILD_PATH"
# Fail the job if the build folder is empty
if [[ -z "$(ls -A "$BUILD_PATH")" ]]; then
echo "Build folder is empty! Failing the job."
exit 1
fi Here are some additional details I am aware of:
|
Impressive @GabLeRoux :) |
This comment was marked as off-topic.
This comment was marked as off-topic.
@GabLeRoux Looks like the formatting of that last message is a bit off towards the end. The solution also doesn't look viable. @coderabbitai So we're not going to talk about how to get those new kinds of arguments inside of javascript will make it across the boundary of docker and be usable inside the bash script that's run inside the container? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Ended up hiding those comments as they add a stream of information that isn't really helpful. For whoever feels like taking a stab at this, please have a look at the combination of the first comment from Coderabbat and the followup from @GabLeRoux. |
I've looked a bit at this already and so far the biggest issue (I think) has not been mentioned. The action currently does a bit of build platform specific setup depending on the build target. If the build profile were to be specified instead of the build target, the necessary information would not be available at the required time. We could try to extract the build target information from the BuildProfile asset file directly and use that to fill in the action parameter. That way we wouldn't need to change most of the existing code. I'm just not sure how robust and reliable that would be. |
We're just casually trialing this bot. It's been doing some very helpful comments as well. Agreed that auto-detect would be preferable where possible. We already do that for the That said, I think the whole point of Build Profiles is to have multiple. So beyond auto-detecting we do need to be able to specify it as a param as well. |
My team has run into this problem as well, and I'd like to take a crack at resolving this issue. @JanikHelbig-NB is right when he says "The action currently does a bit of build platform specific setup depending on the build target". However, I don't know that auto-detection is preferable. The Therefore, we should be able to simply add an optional In this case, if the end user (GitHub workflow author) instructs the provides incompatible values for the I can quickly pursue this path. Alternately, I can try the auto detection. A core difference here is that the current In this case of auto-detection, the GitHub action will need to open the provided build profile The build script ( @webbertakken Let me know your thoughts. |
@MichaelBuhler thanks for sharing your ideas and insights. I think what you're saying makes much sense. Generally speaking the rationale of GameCI is to use as many 'great defaults' as possible, so that the threshold for newer developers is as low as possible, and the complexity for experienced devs is as low as possible. In that line it would make sense to keep auto-detection for things like 'unity version'. If there's not a one-to-one mapping between targetPlatform and buildProfile, then we should probably not link them together, as it does the opposite of reducing complexity. I would like to make sure that we're using the concept as intended. According to When I'm thinking of a case where you'd build 2 different profiles, lets say For historical reasons I believe With regards to your main and alternative proposal, lets maybe write out the different scenarios of the combinations of Either way I would be in favour for what makes the most sense on the larger scale and keeps in mind new devs (i.e. unity-builder abstracts away complexity where possible and effective). |
I agree that it would be better to go with the first option and simply add an optional We could read the build profile's In general, I think it's better to be iterative with smaller updates, and we can follow up with making target platform optional after we've got the initial support for For context, I'm also planning on using build profiles for new builds of my personal project, so that's also part of the reason I'm interested in quicker iterations for this. |
Yea, sounds good to me. As long as we can stay backwards compatible and people don't have to change their pipelines just to upgrade the action (i.e. only need changes if they opt-in to |
add new `buildProfile` action param, which will be passed into Unity as the `-activeBuildProfile ...` CLI param. closes game-ci#674
Relates to game-ci/unity-builder#674 Relates to game-ci/unity-builder#685
@GabLeRoux @webbertakken @davidmfinol I have opened draft PRs here: which do as requested: add a new, optional I have left the PRs in draft right now, as I am not really sure if or how I will be able to test Mac and Windows builds. Furthermore, my workflows on GitHub are failing (on
|
^Never mind about this. The error is not coming from Unity, but rather the custom build code of this repo: unity-builder/dist/default-build-script/Assets/Editor/UnityBuilderAction/Input/ArgumentsParser.cs Line 26 in 83c8532
|
Things got more complicated than I expected. This I can pass /Applications/Unity/Hub/Editor/6000.0.33f1/Unity.app/Contents/MacOS/Unity \
-logFile - \
-quit \
-batchmode \
-nographics \
-projectPath "/absolute/path/to/unity/project" \
-activeBuildProfile "relative/path/to/build-profile.asset" \
-build "/absolute/path/to/build/output/folder" The problem is that So, if we don't want to require the user to define a custom static build class method to use build profiles with this GitHub action, I'll have to also modify the It was a very long time ago (5years 1month) when the decision was made to always execute the @webbertakken What was the reasoning there? I wonder if it was a way to provide a mechanism to programmatically parameterize/customize more build options--the exact functionality that has now been added to Unity as "Build Profiles". Some thought could be given to adding (yet) another GitHub action param, such as Another option is that if the user provides the |
add new `buildProfile` action param, which will be passed into Unity as the `-activeBuildProfile ...` CLI param. closes game-ci#674
The Therefore, we will need some branching strategy within the custom build script implementation. At least two options:
The |
I have successfully built locally with a build profile and a modified version of if (options.ContainsKey("buildProfile")) {
#if UNITY_6000_0_OR_NEWER
// Load build profile from Assets folder
BuildProfile buildProfile = AssetDatabase.LoadAssetAtPath<BuildProfile>(options["buildProfile"]);
// Set it as active
BuildProfile.SetActiveBuildProfile(buildProfile);
// Define BuildPlayerWithProfileOptions
buildPlayerOptions = new BuildPlayerWithProfileOptions {
buildProfile = buildProfile,
locationPathName = options["customBuildPath"],
options = buildOptions,
};
#endif
} else {
// old code here
} |
@MichaelBuhler Excellent! Great work on making the steps and documenting each iteration. I feel this is the right way to move forward. One question: Will there be cases where buildProfiles will completely remove the need for customBuildMethod? |
@webbertakken, yes that it currently the case. The name of the "customBuildMethod" is passed to Unity via the |
@MichaelBuhler ok, then would the features from the build script still work? I.e. do we need the BuildProject method at all in case ˋbuildProfileˋ is passed? |
@webbertakken It is not necessary to do a Unity build with a custom method, such as It would be possible for me to rework it (continue iterating) to provide a way bypass the use From what I can tell, the |
add new `buildProfile` action param, which will be passed into Unity as the `-activeBuildProfile ...` CLI param. closes game-ci#674
add new `buildProfile` action param, which will be passed into Unity as the `-activeBuildProfile ...` CLI param. closes game-ci#674
@webbertakken @davidmfinol @GabLeRoux I made some simplifications, tested with a self-hosted (macOS) GitHub Actions Runner (n.b. this uses the MacBuilder to run Unity directly--not from a I also added two more matrix tests to the Ubuntu tests, which will test Unity 6 builds both with and without a Build Profile. |
Thanks for explaining.
I think it's not needed to do immediately. There may be advantages to sticking to the custom build method for now, as we learn more. |
Relates to game-ci/unity-builder#674 Relates to game-ci/unity-builder#685
Context
When using "activeBuildProfile" parameter (in "customParameters", example:
customParameters: -activeBuildProfile Assets/Settings/BuildProfiles/WindowsDevelopment.asset
) (for Unity 6) builder action throws an error message:Problem is "targetPlatform" is a required parameter.
Suggested solution
Maybe require to have either "targetPlatform" parameter OR another parameter like "buildProfile". "buildProfile" parameter would hold the same value "activeBuildProfile" requires, a relative path to the profile asset.
The text was updated successfully, but these errors were encountered: