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

Enable unity licensing server for Windows #638

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

Latylus
Copy link
Contributor

@Latylus Latylus commented Mar 14, 2024

Hello

A simple version of unity licensing server support for Windows using the same logic as the existing Linux implementation.

Changes

  • Modify the step files activate.ps1 and return_license.ps1 to replicate what is done in activate.sh and return_license.sh
  • Update validate-windows.ts to allow this option to pass through for a Windows build
  • Update docker.ts to correctly copy the unity config file necessary for unity licensing server request

Related PRs

  • 468 : the Linux implementation of this feature

Checklist

  • Read the contribution guide and accept the
    code of conduct
  • Docs (If new inputs or outputs have been added or changes to behavior that should be documented. Please make a PR
    in the documentation repo)
  • Readme (not needed)
  • Tests (not needed)

Copy link

Cat Gif

Copy link
Member

@webbertakken webbertakken left a comment

Choose a reason for hiding this comment

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

LGTM. Can you confirm this works as expected?

if (!(process.env.UNITY_EMAIL && process.env.UNITY_PASSWORD)) {
throw new Error(`Unity email and password must be set for Windows based builds to
authenticate the license. Make sure to set them inside UNITY_EMAIL
if (!((process.env.UNITY_EMAIL && process.env.UNITY_PASSWORD) || buildParameters.unityLicensingServer)) {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would be helpful to simplify the if statement by adding something like

const { unityLicensingServer } = buildParameters
const hasValidLicensingCredentials = process.env.UNITY_EMAIL && process.env.UNITY_PASSWORD
const hasValidLicensingStrategy = validLicensingCredentials || unityLicensingServer 

if (!hasValidLicensingStrategy) { /* stuff */ }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I've added these variables for clarification.

@Latylus
Copy link
Contributor Author

Latylus commented Mar 15, 2024

LGTM. Can you confirm this works as expected?

We have tested this in our private repo and it works as intended (basically identical to the linux version)

@Latylus
Copy link
Contributor Author

Latylus commented Mar 26, 2024

Hello @webbertakken , if any other changes are needed, I'd be glad to discuss or implement them.
In the meantime, I'll try to keep this branch up to date to prevent any merging issue.

@webbertakken webbertakken merged commit f2250e9 into game-ci:main Mar 26, 2024
1 check passed
@webbertakken
Copy link
Member

Released in v4.3.0.

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.

2 participants