feat(manager): Adds Unity3D #28189

ViMaSter · 2024-03-30T22:28:45Z

Changes

Adds a manager for updating the runtime of a project using the Unity game engine.

Context

See #27219, closes #27295.

Documentation (please check one with an [x])

I have updated the documentation, or
No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

Code inspection only, or
Newly added/modified unit tests, or
No unit tests but ran on a real repository, or
Both unit tests + ran on a real repository
- Ran tests locally and via PR on my fork: feat(manager/unity3d): Add Unity3D support ViMaSter/renovate#7

secustor · 2024-03-31T10:09:15Z

lib/modules/manager/unity3d/index.ts

+  packageRules: [
+    {
+      groupName: 'Unity Editor',
+      matchPackageNames: ['m_EditorVersion', 'm_EditorVersionWithRevision'],
+    },
+  ],


This should be rather part of a preset, as it is hard to find out what is happening if the config is set here.

If we are talking about the same version in different places you can set depName to the same value. This will by default result too in a single PR

secustor · 2024-03-31T10:12:57Z

lib/modules/manager/unity3d/extract.ts

+    return null;
+  }
+
+  if (version !== version.trim()) {


Why is this necessary? 🤔

secustor · 2024-03-31T10:15:07Z

lib/modules/manager/unity3d/extract.ts

+  'm_EditorVersion',
+];
+const parseLine = (line: string): PackageDependency | null => {
+  const matches = regEx(/^(?<depName>.+): (?<currentValue>.+)/g).exec(line);


Move the regex definition to the module scope.

lib/modules/manager/unity3d/extract.ts

secustor · 2024-03-31T10:17:03Z

lib/modules/manager/unity3d/extract.ts

+  if (!fileMatchRegex.every((regex: string) => regEx(regex).test(fileName))) {
+    return { deps: [] };
+  }


This isn't necessary, extractPackageFile will only be called on fitting file names

secustor · 2024-03-31T10:17:49Z

lib/modules/manager/unity3d/extract.ts

+    autoReplaceStringTemplate:
+      '{{depName}}{{#if newValue}}:{{newValue}}{{/if}}',
+    currentDigest: undefined,
+    currentValue: version,
+    datasource: 'unity3d',
+    depName: key,
+    depType: 'final',
+    replaceString: matches[0],


Suggested change

autoReplaceStringTemplate:

'{{depName}}{{#if newValue}}:{{newValue}}{{/if}}',

currentDigest: undefined,

currentValue: version,

datasource: 'unity3d',

depName: key,

depType: 'final',

replaceString: matches[0],

currentValue: version,

datasource: 'unity3d',

depName: key,

depType: 'final',

These vales are set by default.

Also why final as depType, if there is only one?

secustor · 2024-03-31T10:22:20Z

lib/modules/manager/unity3d/extract.ts

+    deps: content
+      .split('\n')
+      .map((line) => line.trim())
+      .filter((line) => line.length > 0)
+      .map(parseLine)
+      .filter((dep) => dep !== null) as PackageDependency[],
+  };


Chaining these makes it hard to debug it, please use a for each loop.
https://github.com/renovatebot/renovate/blob/main/docs/development/best-practices.md#iterating-objects--containers

and return null for empty deps

viceice

☝️

viceice · 2024-03-31T11:57:08Z

lib/constants/category.ts

@@ -24,6 +24,7 @@ const Categories = [
  'rust',
  'swift',
  'terraform',
+  'unity3d',


🤔 is it really worth to have an own category? 🤔

Unity has a different package manager (Unity Package Manager) for its sub-dependencies and while I've read #19300, I haven't got it working using the proposed NPM workaround: There's a lock file different from normal NPM lock files that would have to be generated and currently isn't. This might be something worth adding under the unity3d umbrella.

Let me know if you find any existing ones more appropriate.

Makes sense to add the category then, if support for this manager is added too.

Please remove it for now.

viceice · 2024-03-31T11:57:31Z

lib/modules/manager/unity3d/__fixtures__/beta.ProjectVersion.txt

@@ -0,0 +1,2 @@
+m_EditorVersion: 2023.3.0b5


please inline those small fixtures, use codeBlock string template helper for indention

viceice · 2024-03-31T12:01:27Z

lib/modules/manager/unity3d/extract.ts

+    deps: content
+      .split('\n')
+      .map((line) => line.trim())
+      .filter((line) => line.length > 0)
+      .map(parseLine)
+      .filter((dep) => dep !== null) as PackageDependency[],
+  };


and return null for empty deps

Co-authored-by: Sebastian Poxhofer <secustor@users.noreply.github.com>

HonkingGoose · 2024-04-03T21:35:49Z

lib/modules/manager/unity3d/extract.spec.ts

+      ]);
+    });
+
+    it('skips if casing is incorrect', () => {


Should Renovate raise a warning about the malformed casing? Or should Unity warn about malformed files?

HonkingGoose · 2024-04-03T21:36:08Z

lib/modules/manager/unity3d/extract.spec.ts

+      expect(res).toBeEmpty();
+    });
+
+    it('skips abnormal spacing', () => {


Same question about Renovate warning or Unity warning here.

HonkingGoose · 2024-04-03T21:43:10Z

lib/modules/manager/unity3d/readme.md

@@ -0,0 +1,10 @@
+Handles version numbers inside `ProjectSettings/ProjectVersion.txt` files used by the Unity game engine.


I see two ways to read this:

Renovate can read ProjectSettings.txt and ProjectVersion.txt files

Renovate can read a ProjectVersion.txt file if its in the ProjectSettings directory

I don't know which interpretation is correct, as I don't use Unity.

Please update the text so it explains precisely what Renovate can update. 🙂

HonkingGoose · 2024-04-03T21:46:00Z

lib/modules/manager/unity3d/readme.md

@@ -0,0 +1,10 @@
+Handles version numbers inside `ProjectSettings/ProjectVersion.txt` files used by the Unity game engine.
+
+`ProjectSettings/ProjectVersion.txt` always contains two version references in a yml-like syntax; one with and one without a hash, both of which are updated:


Suggested change

`ProjectSettings/ProjectVersion.txt` always contains two version references in a yml-like syntax; one with and one without a hash, both of which are updated:

`ProjectSettings/ProjectVersion.txt` always have two version references in a yml-like syntax; one with and one without a hash, both are updated:

Also, is it YML-like syntax or YAML-syntax? I lean towards YAML-syntax, but not sure. 🙂

HonkingGoose · 2024-04-03T21:47:36Z

lib/modules/manager/unity3d/readme.md

+m_EditorVersionWithRevision: 2020.3.15f2 (6cf78cb77498)
+```
+
+Relies on the corresponding `unity3d` datasource and versioning.


I don't see any changes to a datasource directory in this PR? Maybe that was added in a different PR? Or I'm misunderstanding something here. 🙃

In any case, please check if this sentence is correct.

rarkins · 2024-05-07T04:54:26Z

@ViMaSter do you have time to address the PR review comments?

feat: Adds unity3d manager

a25d75d

ViMaSter mentioned this pull request Mar 30, 2024

refactor: Adresses review notes for Unity3D datasource #28190

Merged

6 tasks

fix: Applies lint-fix

ae65603

secustor requested changes Mar 31, 2024

View reviewed changes

viceice reviewed Mar 31, 2024

View reviewed changes

ViMaSter and others added 2 commits March 31, 2024 22:51

refactor: Replaces anonymous function

1bd8072

Co-authored-by: Sebastian Poxhofer <secustor@users.noreply.github.com>

Update lib/modules/manager/unity3d/extract.ts

2f7bc8a

Co-authored-by: Sebastian Poxhofer <secustor@users.noreply.github.com>

HonkingGoose suggested changes Apr 3, 2024

View reviewed changes

ViMaSter mentioned this pull request Apr 29, 2024

feat(manager/unity3d): Add Unity3D support ViMaSter/renovate#7

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(manager): Adds Unity3D #28189

feat(manager): Adds Unity3D #28189

ViMaSter commented Mar 30, 2024

secustor Mar 31, 2024

secustor Mar 31, 2024

secustor Mar 31, 2024

secustor Mar 31, 2024 •

edited

secustor Mar 31, 2024

secustor Mar 31, 2024 •

edited

viceice Mar 31, 2024

viceice left a comment

viceice Mar 31, 2024

ViMaSter Mar 31, 2024

secustor Apr 1, 2024

viceice Mar 31, 2024

viceice Mar 31, 2024

HonkingGoose Apr 3, 2024

HonkingGoose Apr 3, 2024

HonkingGoose Apr 3, 2024

HonkingGoose Apr 3, 2024

HonkingGoose Apr 3, 2024

rarkins commented May 7, 2024

		@@ -0,0 +1,10 @@
		Handles version numbers inside `ProjectSettings/ProjectVersion.txt` files used by the Unity game engine.

		@@ -0,0 +1,10 @@
		Handles version numbers inside `ProjectSettings/ProjectVersion.txt` files used by the Unity game engine.

		`ProjectSettings/ProjectVersion.txt` always contains two version references in a yml-like syntax; one with and one without a hash, both of which are updated:

feat(manager): Adds Unity3D #28189

Are you sure you want to change the base?

feat(manager): Adds Unity3D #28189

Conversation

ViMaSter commented Mar 30, 2024

Changes

Context

Documentation (please check one with an [x])

How I've tested my work (please select one)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

secustor Mar 31, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

secustor Mar 31, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viceice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rarkins commented May 7, 2024

secustor Mar 31, 2024 •

edited

secustor Mar 31, 2024 •

edited