Skip to content

Commit 60336e8

Browse files
authored
Fix the Github Action CI (#835)
## Summary <!-- What does this PR change and why? Link related issues. --> Currently all the linux builds on Github were no longer building, decided to go and fix it. ## Required checks All boxes below must be ticked before this PR can merge. If a check is genuinely N/A, tick it anyway and explain under **Notes**. <!-- required-checks-start --> <!-- Tick the boxes in place — do not edit the line text. The pr-checklist workflow parses this block; per-PR context goes under Notes. --> - [x] **Tested** — I built and ran this locally. The change works in the editor and (where relevant) in a built player. - [x] **Transform access is combined and limited** — In hot paths, transform reads/writes go through `TransformAccessArray` or are otherwise batched. I have not added per-frame `transform.position` / `transform.rotation` / `transform.localPosition` calls inside loops. Whenever I need both position and rotation, I use the combined APIs — `SetPositionAndRotation` / `SetLocalPositionAndRotation` for writes, `GetPositionAndRotation` / `GetLocalPositionAndRotation` for reads — instead of two separate property accesses; the combined call does one local-to-world matrix traversal instead of two. - [x] **Addressables used for asset/memory loading** — Any new asset loads go through Addressables. No new `Resources.Load`, no direct asset references that pull large content into memory on scene load. - [x] **No new `GetComponent` / `AddComponent` where avoidable** — Where unavoidable, the result is cached on a field, and any `GetComponent<T>` is replaced with `TryGetComponent<T>(out var x)` — bare `GetComponent` will be denied. `TryGetComponent` is the modern API (Unity 2019.2+) and skips the Editor-only GC allocation `GetComponent` causes when a component is missing: Unity wraps the `null` return in a managed "fake null" object so its overloaded `==` operator can still detect destroyed C++ objects, and constructing that wrapper allocates; `TryGetComponent` returns a `bool` plus `out` parameter and never builds the wrapper. None of these calls run inside `Update`, `LateUpdate`, `FixedUpdate`, jobs, or other per-frame code paths. - [x] **Per-frame work is scheduled through `BasisEventDriver`** — Any new per-frame work hooks into `BasisEventDriver` rather than adding standalone `Update` / `LateUpdate` / `FixedUpdate` callbacks on a MonoBehaviour. - [x] **Anything added to `BasisEventDriver` is bulletproof, or guarded by `try`/`catch`** — `BasisEventDriver` runs the single per-frame tick that drives the whole framework (network apply, local player sim, blendshapes, JigglePhysics, nameplates, and more) as one sequential chain. An unhandled exception anywhere in that chain aborts the rest of the tick, so every step after the throwing one is silently skipped for that frame. New work added to the driver must either be guaranteed not to throw, or be wrapped in a `try`/`catch` that contains the failure and surfaces it through `BasisDebug` — logged once / rate-limited, never every frame (see the existing `HVRBasisBuiltInAddresses.Simulate()` guard for the pattern). Expect this to be scrutinized closely in review. - [x] **Considered jobification** — I asked whether this work can be moved to a Unity Job (Burst-compiled where possible). If it can, it is. If it cannot, the reason is in **Notes**. - [x] **No needless `{ get; set; }` properties or access lockdowns** — Public fields are fine; Basis is meant to be read and modified freely, so don't wall things off `private`/`internal` without a real reason. Don't wrap a field in `{ get; set; }` when the accessors do nothing — property accessors have a real performance cost vs direct field access, and the lead maintainer prefers plain fields (or a method / setter-only property when only the setter needs logic) over a noop-getter pair. For `.Instance` singletons, callers reassigning `Type.Instance` is allowed; if that would break your code, log a warning or throw — don't block the assignment. Locking down access is not your call. - [x] **Camera access goes through `BasisLocalCameraDriver`** — Code that needs the local camera (transform, projection, rig data, etc.) pulls it from `BasisLocalCameraDriver` rather than looking one up itself. Don't roll a separate camera discovery path. - [x] **Logging uses `BasisDebug`** — All new logging calls go through `BasisDebug.Log` / `BasisDebug.LogWarning` / `BasisDebug.LogError` (with an appropriate `LogTag`) instead of `UnityEngine.Debug.Log` / `Debug.LogWarning` / `Debug.LogError`. `BasisDebug` routes through Basis's tagged, color-coded logger and respects the project-wide `LoggingDisabled` toggle so logging can be killed at runtime; bare `Debug.Log` calls bypass that and will be denied. - [x] **No scene-wide discovery for dependencies** — New code is architected so it does not need `FindObjectOfType` / `FindObjectsOfType` / `GameObject.Find` / `FindGameObjectsWithTag` to locate what it depends on. References are wired in — registered through an existing manager/driver, injected at init, or passed in by the caller — rather than discovered by scanning the scene at runtime. If a scene scan is genuinely unavoidable, justify it under **Notes**. - [x] **No allocations in hot paths** — Per-frame code (Update / LateUpdate / FixedUpdate, simulation loops, jobs, anything called once per frame or more) does not allocate. No `new` on reference types, no LINQ, no `string` concatenation/interpolation, no boxing, no `foreach` over interface-typed collections. Allocate once at init and reuse the buffer. - [x] **No debugging in hot paths** — No log calls of any kind on per-frame paths, including `BasisDebug`. Hot-path logging floods the console and incurs cost on every frame regardless of whether the message is filtered out downstream. If a hot-path log is needed while iterating, gate it behind `#if UNITY_EDITOR` and remove (or leave gated) before merge. - [x] **Hot-path collection access is optimized** — Cache `.Count` (lists) / `.Length` (arrays) into a local `int` before the loop instead of re-reading the property each iteration. Prefer `T[]` (with a separate length int when the array is over-sized) over `List<T>` where the data is hot — Unity's mono BCL doesn't expose `CollectionsMarshal.AsSpan(List<T>)`, so a list can't be fed into `Span<T>` / unsafe paths cleanly. Where the perf justifies it, drop into `Span<T>` / `ref` locals / `Unsafe.As` / `unsafe` pointer code to skip bounds checks and copies, and call out the invariants you're relying on under **Notes** so reviewers can sanity-check them. <!-- required-checks-end --> ## Testing details Tick the platforms you actually tested on. Leave the rest unticked — these are informational and do not block merge. - [x] Windows - [x] Linux - [ ] Android - [ ] iOS - [ ] macOS Input / control mode coverage: - [ ] Tested in VR (note headset under **Notes**) - [ ] Tested in desktop / non-VR mode - [ ] Tested with phone controls (mobile touch input) - [x] N/A — change does not touch player/XR/input code Where applicable, confirm these flows still work after your changes: - [ ] Hot-switching (desktop ↔ VR mode swap at runtime) - [ ] Avatar swapping - [ ] Server swapping (joining / leaving / changing servers) - [x] N/A — change does not touch any of the above ## Notes <!-- Optional context for reviewers. Headset model, why a required box is N/A, anything else worth knowing. -->
2 parents a7405cd + 07a1450 commit 60336e8

3 files changed

Lines changed: 151 additions & 6 deletions

File tree

.github/scripts/sanitize_headless_ci.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
PACKAGE_DEPENDENCIES_TO_REMOVE = (
1919
"com.llealloo.audiolink",
2020
"com.unity.xr.openxr",
21+
"com.valvesoftware.openxr.utils",
2122
"com.valvesoftware.unity.openvr",
2223
)
2324

@@ -41,6 +42,11 @@
4142
"AudioLink",
4243
)
4344

45+
PACKAGE_LOCK_ENTRIES_TO_REMOVE = PACKAGE_DEPENDENCIES_TO_REMOVE + (
46+
"com.basis.openvr",
47+
"com.basis.openxr",
48+
)
49+
4450

4551
def normalize_project_path(path: Path, project_root: Path) -> str:
4652
return path.relative_to(project_root).as_posix()
@@ -100,6 +106,35 @@ def remove_manifest_dependencies(project_root: Path) -> list[str]:
100106
return removed_dependencies
101107

102108

109+
def remove_package_lock_entries(project_root: Path) -> list[str]:
110+
lock_path = project_root / "Packages/packages-lock.json"
111+
if not lock_path.exists():
112+
return []
113+
114+
import json
115+
116+
lock_data = json.loads(lock_path.read_text(encoding="utf-8"))
117+
dependencies = lock_data.get("dependencies", {})
118+
removed_entries: list[str] = []
119+
120+
for dependency in PACKAGE_LOCK_ENTRIES_TO_REMOVE:
121+
if dependencies.pop(dependency, None) is not None:
122+
removed_entries.append(dependency)
123+
124+
for package_info in dependencies.values():
125+
package_dependencies = package_info.get("dependencies")
126+
if not isinstance(package_dependencies, dict):
127+
continue
128+
129+
for dependency in PACKAGE_LOCK_ENTRIES_TO_REMOVE:
130+
package_dependencies.pop(dependency, None)
131+
132+
if removed_entries:
133+
lock_path.write_text(json.dumps(lock_data, indent=2) + "\n", encoding="utf-8")
134+
135+
return removed_entries
136+
137+
103138
def strip_editor_build_settings(project_root: Path) -> list[str]:
104139
settings_path = project_root / "ProjectSettings/EditorBuildSettings.asset"
105140
lines = settings_path.read_text(encoding="utf-8").splitlines(keepends=True)
@@ -238,6 +273,12 @@ def main() -> int:
238273
for dependency in removed_dependencies:
239274
print(f" - {dependency}")
240275

276+
removed_lock_entries = remove_package_lock_entries(project_root)
277+
if removed_lock_entries:
278+
print(f"Removed {len(removed_lock_entries)} package lock entries from {project_root / 'Packages/packages-lock.json'}")
279+
for dependency in removed_lock_entries:
280+
print(f" - {dependency}")
281+
241282
removed_config_keys = strip_editor_build_settings(project_root)
242283
if removed_config_keys:
243284
print(f"Removed {len(removed_config_keys)} XR config objects from {project_root / 'ProjectSettings/EditorBuildSettings.asset'}")
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
#!/usr/bin/env python3
2+
from __future__ import annotations
3+
4+
import json
5+
import shutil
6+
import sys
7+
from pathlib import Path
8+
9+
10+
PACKAGE_DIRS_TO_REMOVE = (
11+
"Packages/com.basis.openxr",
12+
"Packages/com.meta.xr.sdk.core",
13+
)
14+
15+
PACKAGE_DEPENDENCIES_TO_REMOVE = (
16+
"com.unity.xr.openxr",
17+
"com.valvesoftware.openxr.utils",
18+
)
19+
20+
PACKAGE_LOCK_ENTRIES_TO_REMOVE = PACKAGE_DEPENDENCIES_TO_REMOVE + (
21+
"com.basis.openxr",
22+
)
23+
24+
25+
def remove_manifest_dependencies(project_root: Path) -> list[str]:
26+
manifest_path = project_root / "Packages/manifest.json"
27+
manifest = json.loads(manifest_path.read_text(encoding="utf-8"))
28+
dependencies = manifest.get("dependencies", {})
29+
removed: list[str] = []
30+
31+
for dependency in PACKAGE_DEPENDENCIES_TO_REMOVE:
32+
if dependencies.pop(dependency, None) is not None:
33+
removed.append(dependency)
34+
35+
if removed:
36+
manifest_path.write_text(json.dumps(manifest, indent=2) + "\n", encoding="utf-8")
37+
38+
return removed
39+
40+
41+
def remove_package_lock_entries(project_root: Path) -> list[str]:
42+
lock_path = project_root / "Packages/packages-lock.json"
43+
if not lock_path.exists():
44+
return []
45+
46+
lock_data = json.loads(lock_path.read_text(encoding="utf-8"))
47+
dependencies = lock_data.get("dependencies", {})
48+
removed: list[str] = []
49+
50+
for dependency in PACKAGE_LOCK_ENTRIES_TO_REMOVE:
51+
if dependencies.pop(dependency, None) is not None:
52+
removed.append(dependency)
53+
54+
for package_info in dependencies.values():
55+
package_dependencies = package_info.get("dependencies")
56+
if not isinstance(package_dependencies, dict):
57+
continue
58+
59+
for dependency in PACKAGE_LOCK_ENTRIES_TO_REMOVE:
60+
package_dependencies.pop(dependency, None)
61+
62+
if removed:
63+
lock_path.write_text(json.dumps(lock_data, indent=2) + "\n", encoding="utf-8")
64+
65+
return removed
66+
67+
68+
def remove_package_dirs(project_root: Path) -> list[Path]:
69+
removed: list[Path] = []
70+
71+
for relative_dir in PACKAGE_DIRS_TO_REMOVE:
72+
package_dir = project_root / relative_dir
73+
if package_dir.exists():
74+
shutil.rmtree(package_dir)
75+
removed.append(package_dir)
76+
else:
77+
print(f"Package directory already absent: {package_dir}")
78+
79+
return removed
80+
81+
82+
def main() -> int:
83+
project_root = Path(sys.argv[1]) if len(sys.argv) > 1 else Path("Basis")
84+
project_root = project_root.resolve()
85+
86+
if not project_root.exists():
87+
print(f"Project root not found: {project_root}", file=sys.stderr)
88+
return 1
89+
90+
removed_manifest_dependencies = remove_manifest_dependencies(project_root)
91+
for dependency in removed_manifest_dependencies:
92+
print(f"Removed manifest dependency: {dependency}")
93+
94+
removed_lock_entries = remove_package_lock_entries(project_root)
95+
for dependency in removed_lock_entries:
96+
print(f"Removed package lock entry: {dependency}")
97+
98+
removed_package_dirs = remove_package_dirs(project_root)
99+
for package_dir in removed_package_dirs:
100+
print(f"Removed package directory: {package_dir}")
101+
102+
return 0
103+
104+
105+
if __name__ == "__main__":
106+
raise SystemExit(main())

.github/workflows/compilation.yml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,11 @@ jobs:
8484
- name: "Create version string"
8585
id: version
8686
run: echo "gitversion=$(git describe --tags --always)" >> "$GITHUB_OUTPUT"
87-
- name: "Remove OpenXR when building for Linux"
87+
- name: "Sanitize Linux client project"
8888
if: matrix.targetPlatform == 'StandaloneLinux64'
89-
run: rm -rf ${projectPath}/Packages/com.basis.openxr
90-
# run: jq 'del(.dependencies ["com.unity.xr.openxr"])' < ${projectPath}/Packages/manifest.json > manifest.json.tmp && mv manifest.json.tmp ${projectPath}/Packages/manifest.json
91-
- name: "Remove Meta XR Core when building for Linux"
92-
if: matrix.targetPlatform == 'StandaloneLinux64'
93-
run: rm -rf ${projectPath}/Packages/com.meta.xr.sdk.core
89+
shell: bash
90+
run: |
91+
python3 .github/scripts/sanitize_linux_client_ci.py "${projectPath}"
9492
# Fix docker not ready on windows
9593
- name: Ensure Docker service is running
9694
if: matrix.buildPlatform == 'windows-2022'

0 commit comments

Comments
 (0)