feat: add the migrate-birp-to-urp skill - #17
Open
ziyiunity wants to merge 4 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports
migrate-birp-to-urpfrom the AI Assistant skill set onto the plugin's CLI execution layer. Six reference files and roughly 1,650 lines of domain content are preserved verbatim; only the execution layer changed.What changed against the original
Unity.GetStaticProjectSettingsTool→eval. Replaced with two detection snippets: one readingGraphicsSettings.defaultRenderPipeline, and one walking every Quality level viaQualitySettings.GetRenderPipelineAssetAt(i). The per-level check matters — a project can be switched in Graphics settings while a Quality level still points somewhere else.Unity.Camera.Capture→ render-to-PNG. Points atreferences/capturing-the-editor.md, the same verified path the other capture-dependent skills use.ExecutionResultandresult.Logremoved. The exposure-validation block was astatic void …(ExecutionResult result)method; it is now a statement block that accumulates into a list and returns a string.eval, because they resolve throughusing. Two sites needed rewriting:camera.GetUniversalAdditionalCameraData()becameGetComponent<UniversalAdditionalCameraData>(), and.FirstOrDefault(…)becameSystem.Linq.Enumerable.FirstOrDefault(seq, pred). The skill now documents this so neither form gets reintroduced.sealed class MaterialSnapshot,[MenuItem]entry points). A class declaration cannot be flattened into a statement block, so those stay project files: save underAssets/Editor/, compile, then invoke through a one-lineeval. Theirusingdirectives are correct there and were left alone. For a migration this route is also the more robust one — it survives the domain reloads that URP installation and material conversion trigger, which a longevalpayload does not.unity commanddefaults to 30 seconds, which URP installation will outlast. The skill treats that as a phase boundary rather than telling the agent to raise the timeout.Verification
Every snippet was compile-checked against Unity 6000.5.7f1 with URP 17.5 by prefixing
if (true) return "OK";, which type-checks the whole block without executing it. A deliberately broken snippet was run as a control and correctly failed withCS0103.Confirmed to compile:
UniversalRenderPipelineAsset,UniversalRendererData,UniversalAdditionalCameraData,UnityEngine.Rendering.Volume,RenderSettings,SerializedObject,QualitySettings.GetRenderPipelineAssetAt, and the static-LINQ rewrites.Two things found by running a real migration, and now fixed here
I ran this skill end to end against a fresh Built-in project — five materials on
Standard,Standard (Specular setup),Legacy Shaders/DiffuseandParticles/Standard Unlit, plus a particle system and a directional light. It completed: URP assigned in Graphics settings and on all six Quality levels, 5/5 materials on URP shaders, and Built-in vs URP captures within 1–2/255 mean RGB. Both fixes below came out of that run, and both are the kind that pass review by eye, so they're worth a reviewer's attention specifically.1. The 2D upgrader silently hijacks
StandardmaterialsMaterialUpgrader.FetchAllUpgradersForPipelinereturns the 2D provider set alongside the 3D one, and two upgraders claimStandardat equal priority. On a 3D project that means a plainStandardmaterial can convert toUniversal Render Pipeline/2D/Mesh2D-Lit-Defaultinstead ofUniversal Render Pipeline/Lit.What makes it nasty: nothing throws, and the material does not render magenta. The only way to notice is to read
shader.nameback. In my run two of five materials landed on the 2D shader.The skill now says to verify every converted material's shader name against the intended target, and to restore-and-retry with a 3D-filtered upgrader list if any landed on a
2D/shader. The manual conversion pattern inreferences/implementation-patterns.mdnames its target shader explicitly, so it can't be hijacked — that's now stated as the reason to prefer it when this happens.Reviewer question: is this the expected behaviour of the converter, or a bug worth filing against URP? I've documented it as behaviour to defend against, but someone on the rendering side would know whether it should be fixed upstream instead.
2. The Pipeline package's declared Editor floor is wrong, and this skill is where it bites
com.unity.pipelineusesIPreprocessBuildWithContextandBuildCallbackContext. Those were introduced in Unity 6000.3 — verified against the API reference across versions, with a control to confirm the URL pattern resolves for older versions:Confirmed locally too: absent from
6000.0.81f1'sUnityEditor.dll, present in6000.5.8f1's.The package manifest declares
"unity": "6000.0", so on 6000.0–6000.2 it installs cleanly and then fails to compile. The symptom is actively misleading: the Pipeline server never starts,unity statusprints no row and no error, and the real cause is aCS0246pair buried in the Editor log. I lost time to exactly this before finding it.This skill is the one most likely to meet it — someone migrating off the Built-in pipeline is, by definition, often on an older Editor. So Step 0 now names the symptom and tells the agent to report "your Editor is too old for the Pipeline package" instead of debugging the CLI.
This is a workaround, not the fix. The real fix is raising
"unity"to6000.3in the Pipeline package manifest so the package manager refuses the install with a clear message. That belongs to the CLI/Pipeline team and is being raised separately.