You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The canonical baked-lightmap pixel assertion intentionally runs only in clean batchmode because arbitrary interactive user scenes can alter Unity global lightmap state.
Baseline JSON and canonical validation scene assets remain unchanged.
- Add fixed-host and runtime contracts for independent Toon shadow visibility.
- Validate expected RED product behavior and GREEN non-Toon/light-kind controls.
- Render mode readbacks in a dedicated preview scene and culling mask.
- Verify Cutout and Transparent numeric contracts with active-scene contamination.
- Run the canonical pixel baseline only in a clean batch scene lifecycle and preserve interactive Editor state.
- Require explicit passed NUnit evidence for the strict baseline testcase and verify focused Unity and runner checks.
Co-authored-by: Copilot <copilot@github.com>
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR successfully separates direct-light color attenuation from Unity shadow visibility for the Toon shader model, fulfilling the intent of allowing independent control over lighting and shadowing. All required test scenarios for numerical correctness and contract preservation (PBR, Hybrid, and Unlit) have been found in the implementation.
However, the PR is currently 'not up to standards' according to Codacy, primarily due to the introduction of 30 new quality issues and a substantial increase in complexity within the test suites. Specifically, PureBaseToonLightingContractTests.Runtime.cs has seen a complexity delta of 70, with several methods exceeding length and parameter count thresholds. These maintainability issues should be addressed to ensure the new diagnostic and regression tests remain manageable.
About this PR
The test infrastructure introduced in this PR, while functional, has resulted in high complexity scores and file bloat. Systemic patterns of overly long methods and high parameter counts suggest that the test utility logic should be decoupled from the test definitions.
2 comments outside of the diffTests/Daily/Editor/PureBaseToonLightingContractTests.Runtime.cs
line 1 🔴 HIGH RISK
PureBaseToonLightingContractTests.Runtime.cs has exceeded 1000 lines. Since this is already a partial class, consider refactoring it by moving the nested 'ToonLightingCaptureRuntimeScope', 'LightVariantRequest', or 'ShadowReceiverObservation' into separate files to improve modularity and maintainability.
line 611 🔴 HIGH RISK
The Validate method is too long and complex. Decompose this method by extracting the validation logic for the root camera (LegacyBaselineCameraName), baked directional lights, and static renderers into dedicated private validation methods.
Test suggestions
Verify numerical separation logic for Toon shadow attenuation
Verify Point and Spot non-shadow attenuation affects radiance and aggregate direction weighting
Regression test for shadow visibility publishing in None, Hard, and Soft shadow modes
Verification of PBR/Hybrid retention of full attenuation visibility
Verification of Unlit shadow-invariance contract
Deterministic readback isolation test using preview scenes
Verification of directional and cubemap cookie attenuation support
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the BIRP host/light plumbing so Toon can expose Unity’s effective shadow visibility separately from non-shadow attenuation (distance/cone/cookie), while keeping PBR/Hybrid/Unlit contracts intact. It also adds a dedicated ToonShadow Shader-Core test host plus targeted regression coverage, and tightens Daily/batch validation determinism.
Changes:
Split main-light attenuation into non-shadow attenuation + effective visibility, publishing visibility via sd.shadow for Toon while preserving full attenuation for PBR/Hybrid and neutral behavior for Unlit.
Add a fixed ToonShadow Shader-Core host fixture and new Daily tests covering lighting/shadows/rendering-mode isolation and deterministic baseline evidence.
Strengthen regression runner validation to require strict canonical baseline testcase presence and pass status in NUnit evidence.
Reviewed changes
Copilot reviewed 27 out of 35 changed files in this pull request and generated 2 comments.
Show a summary per file
File
Description
Tests/Run-PureBaseRegression.ps1
Enforces presence/pass result for the strict canonical baseline testcase in NUnit evidence and extends smoke validation.
- Split Daily test rendering and assertion helpers into cohesive private operations.
- Preserve cleanup order and validate focused Unity tests plus local analyzer output.
- Initialize transient cookies through light-capture requests and formalize restore-row equality contracts.
- Verify focused cookie and ToonShadow restoration tests with zero Unity Console errors.
- Consume effective visibility once in Toon direct-light accumulation and clarify the test-host stencil state.
- Verify ToonShadow import plus focused fixed-host and non-Toon shadow controls.
- Require module-free Toon direct receiver shadow response while preserving indirect-light ownership.
- Guard the single visibility use and read-only ForwardAdd stencil through focused Daily contracts.
- Define the one-time Toon direct-light visibility rule and module double-attenuation migration constraint.
- Synchronize English and Japanese shader contract guidance with tested HLSL behavior.
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
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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
Summary
Validation
PureBase.Tests.Daily:78 total,77 passed,0 failed,1 intentional strict-baseline ignoreTests/Run-PureBaseRegression.ps1 -Mode Daily:78 passed,0 failed,0 skipped,0 inconclusive12623Notes