Release 2.1.2 - #35
Merged
Merged
Conversation
GUID:ffab5256b265d45fa9fb86697af7ee2b resolves to no .asmdef anywhere in this repo or Library/PackageCache. The other two GUIDs in this reference list (Addressables, Addressables.Editor) are confirmed live; this third one was orphaned, likely from a removed dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tency Bumps to 2.1.2 (published at 2.1.1; this is a real release, not a fold). - README: converged repository links on the actual origin (github.com/CoderGamester/Services) -- was inconsistently mixed with com.gamelovers.services, which is not this repo's name. Also fixed the cross-package GameData link (actual origin is Unity-GameData). - package.json: ServicesPlayground sample description no longer claims the UI is built programmatically -- it ships as a hand-authored prefab with a [SerializeField]-wired driver script. - Renamed Tests/EditMode/GameLovers.Services.Tests.asmdef to GameLovers.Services.Editor.Tests.asmdef to match its own `name` field; GUID preserved via git mv on the paired .meta. - Tests/PlayMode/GameLovers.Services.Tests.Playmode.asmdef no longer sets autoReferenced: true (was the only test asmdef in the repo doing so). Verified safe: the Test Runner discovers test assemblies via the UNITY_INCLUDE_TESTS define constraint, independent of autoReferenced -- full PlayMode suite still finds and runs this assembly's tests (305/305 passing, same count as before the change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The sample uses #if ENABLE_INPUT_SYSTEM and UnityEngine.InputSystem.UI types while relying on Assembly-CSharp, which AGENTS.md 19 disallows. No editor asmdef is needed here -- the sample ships no editor scripts. References GameLovers.GameData directly. That one is not obvious: the sample reaches floatP through GameLovers.Services, but asmdef references are not transitive, so it fails with CS0012 without the direct reference. Caught by compiling the sample rather than by eyeballing the reference list -- Samples~ is invisible to Unity, so it was copied into Assets/ to build. Final state: 0 errors, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Under Active Input Handling = "Input System Package (New)" the swap destroyed the scene's StandaloneInputModule and added an InputSystemUIInputModule via AddComponent, which leaves its actions unassigned. An unassigned module processes no input at all -- silently, with nothing logged -- so every button in the sample was dead. AssignDefaultActions() wires the default UI action map. Found while driving the sibling uiservice UrpRendering sample through the Unity MCP: buttons rendered, reported interactable, and did nothing on click. This sample shares the same helper, copied from it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-pin 14 tests
Three consumer-facing bugs found by the test-suite audit, each now pinned:
- GameObjectPool.Dispose(bool) and GameObjectPool<T>.Dispose(bool) destroyed
SampleEntity unconditionally, ignoring disposeSampleEntity.
- ObjectPoolBase<T>.SpawnEntity could hand out a destroyed pooled object. Its
retry loop tested `entity == null`, which inside a generic constrained only to
`class` compiles to reference equality and never reaches UnityEngine.Object's
overloaded == that detects a fake-null native object. Now dispatched via a
runtime type check, falling back to reference equality for POCO pooled types.
- AddressableIdsGeneratorUtils emitted duplicate enum members when two addresses
sanitized to the same identifier, producing generated code that will not
compile. The append path now uses the disambiguated name it already computed.
AssetResolverService.Convert<T>'s type switch is extracted to an internal
SelectAsset<TAsset> so the not-yet-loaded placeholder path is directly testable
without an Addressables catalog; likewise BuildEnumSource in the generator. No
behaviour change in either extraction.
Also declares com.unity.test-framework.performance 3.5.0 (both test asmdefs
already referenced Unity.PerformanceTesting unconditionally), and adds
Tests/CLAUDE.md alongside the existing Tests/AGENTS.md.
EditMode 830/830, PlayMode 299/299.
RCR: GenerateEnumSource_TwoAddressesCollideAfterSanitization_ProducesDistinctMemberNames <- AddressableIdsGeneratorUtils.cs AppendAddressEnumMembers — revert to `stringBuilder.Append(GetCleanName(addresses[i], true));`
RCR: GenerateEnumSource_AddressAppendedToInput_DoesNotRenumberExistingMembers <- AddressableIdsGeneratorUtils.cs AppendAddressEnumMembers — reverse the loop to `for (var i = addresses.Count - 1; i >= 0; i--)`
RCR: GenerateEnumSource_EmptyAddressList_ProducesCompilableEmptyEnum <- AddressableIdsGeneratorUtils.cs BuildEnumSource — delete `stringBuilder.AppendLine("\t}");`
RCR: SelectAsset_WhenReferenceNotDone_ReturnsErrorPlaceholderNotNull <- AssetResolverService.cs SelectAsset — invert the Sprite branch to `isDone ? errorSprite as TAsset : asset as TAsset`
RCR: Spawn_OnPoolEntityObject_CallsInitWithOwningPoolEveryTime <- ObjectPool.cs CallInstantiator — delete `poolEntity?.Init(this);`
RCR: Range_FloatMinEqualsMaxWithMaxInclusive_ReturnsMin <- RngService.cs Range — change `if (min > max || ...)` to `if (min >= max || ...)`
RCR: Range_FloatMinEqualsMaxWithMaxExclusive_ThrowsIndexOutOfRange <- RngService.cs Range — drop the whole `|| (!maxInclusive && ...)` disjunct, leaving `if (min > max)`
RCR: LoadVersionDataAsync_Successfully <- VersionServices.cs LoadVersionDataAsync — comment out `ApplyTextAsset(textAsset, asyncContext: true);`
RCR: StopCoroutine_NullCoroutine_DoesNotThrow <- CoroutineService.cs StopCoroutine — remove the `coroutine == null ||` term from the guard
RCR: StopCoroutine_AfterServiceObjectDestroyed_DoesNotThrowMissingReference <- CoroutineService.cs StopCoroutine — remove the `_serviceObject == null ||` term from the guard
RCR: Dispose_WithDisposeSampleEntityFalse_DoesNotDestroySampleEntity <- GameObjectPool.cs Dispose — revert to unconditional `Object.Destroy(SampleEntity);`
RCR: Spawn_WhenPooledEntityWasDestroyedExternally_ReturnsFreshInstance <- ObjectPool.cs SpawnEntity — collapse the do-while retry to a single unconditional pop
RCR: Dispose_WithDisposeSampleEntityFalse_DoesNotDestroySampleEntity <- GameObjectPool.cs GameObjectPool<T>.Dispose — revert to unconditional `Object.Destroy(SampleEntity.gameObject);`
RCR: SubscribeOnUpdate_ZeroDeltaTimeWithOverflowToNextTick_TicksEveryFrame <- TickService.cs Update — drop the zero-guard, leaving `var overFlow = deltaTime % tickData.DeltaTime;`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ed §2) §2 declared any test without an RCR line "suspect by default", but some correct tests provably have no one-line mutation - double-guarded validation, where an unconfigured object trips two independent guards so disabling either leaves the other throwing. The rule was mislabelling tests that are right and unbreakable. Adds an UNFALSIFIABLE exemption on §13's terms: the reason must be falsifiable, must name both guards, and must record that a mutation was tried and observed green. "Couldn't find one" is explicitly not a reason - that is an unfinished RCR, not an exemption. Also adds a verdict table for tests that resist mutation, because they are not one problem: A5 duplicates get deleted (naming the surviving sibling), D2 overclaims get a strengthened assertion or an honest rename, and UNFALSIFIABLE tests are kept with the exemption comment. The class must be proven before acting - an A5 duplicate by observing the sibling's mutation redden both, a D2 overclaim by observing the implied mutation leave the test green. §1 and §2 remain byte-identical across all six packages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Section 2's verdicts table gains a fourth class. UNFALSIFIABLE was absorbing tests that section 1's A3 rule would never have admitted - the tell being reasons like "no line in Runtime/ participates" or "these are C#'s zero-init values", which describe a test that pins nothing rather than one that is hard to break. New rule: if no line in Runtime/ or Editor/ participates in the assertion, it is an A3 reject and the verdict is delete. UNFALSIFIABLE stays reserved for behaviour this package genuinely owns but cannot be broken one line at a time. Sections 1-2 are shared verbatim across all six Tests/AGENTS.md; no test or production changes in this package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Section 13 now carries a dated baseline for this package's runtime assembly, plus the reason to steer by that number rather than the combined one. Every earlier coverage figure in this repo was an artifact and must not be compared against: - reports before today ran without -debugCodeOptimization, so Unity compiled Release and emitted ~40% fewer sequence points (MathfloatP showed 637 coverable lines instead of 1002) - a silently shrunken denominator - some runs leaked test and sample assemblies into scope, and some covered only 3 of the 6 packages The current run covers all 11 production assemblies with none leaking, verified via the MathfloatP denominator check now documented in Tools/coverage.sh. Repo-wide: runtime 73.9%, Editor 5.5%, combined 41.0%. Editor is 48.1% of all coverable lines and is accepted-untestable per the ACCEPTED (iii) rows in section 13, which is the whole reason the combined figure is not the one to track. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
§9 permits Assert.That ONLY for tolerance/range constraints classic asserts cannot express, authorising exactly TimeServiceTest.cs:67,83 and calling any other use a review reject. These six were Assert.That(x.Contains(...), Is.True) / Is.False - plain boolean checks that Assert.IsTrue/IsFalse express directly, so they were outside the exemption on its own terms. No behavioural change; EditMode green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sections 1-2 are shared verbatim across all six packages; this adds a sixth admission question and the worked instance behind it. A6 asks whether an assertion's outcome would change if project configuration changed - a renderer feature installed or removed, an Addressables catalog built, a sample imported. If so the test must READ that state rather than assume one value of it. A6 is not A3. A3 asks whether the package computed the value; A6 asks whether the test assumed which value it would be. A test can satisfy A3 and still fail A6, which is exactly how the gap went unnoticed: UiBackdropBlurPresenterFeatureTests read a package-computed flag (UiBackdropBlurRendererFeature.IsInstalled) but hard-coded the expectation that it was false. Batchmode never instantiates the URP renderer, so the flag was false there and all five tests passed; in the Editor the feature registers from the project's renderer asset and all five failed. The fixture was asserting a fact about the repo, not about the code under test. Validated against the existing corpus before being written, per root AGENTS.md 2.2: the blur fixture was the only violation and is already fixed. AddressablesUiAssetLoaderTests asserts on a key that is unresolvable either way, and UiCameraStackFeatureTests builds its own cameras rather than reading project renderer state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
118 of 122 mutations went RED on the first pass; the remaining four were investigated rather than dropped, and three of them turned out to be defects in the TESTS, exposed by the mutation staying green. - UnityTime_Convertions_Successfully and UnixTime_Convertions_Successfully assert Assert.GreaterOrEqual(ErrorValue, converted - now), which bounds the difference only from ABOVE. Shrinking mutations are invisible: negating _initialUnityTime, and swapping TotalMilliseconds for TotalSeconds (a 1000x shrink), were both observed GREEN. Growing mutations redden immediately. Both want a two-sided bound on the absolute difference; recorded on the tests as a negative result until that is decided. - Peekfloat_DoesNotAdvanceState is precision-blind. Peekfloat draws over (0, floatP.MaxValue), where consecutive draws saturate to the same floatP - so making PeekRange consume the LIVE state stayed green, and the test cannot see the advance its name claims to catch. Narrowing the range reddens it. - TryResolve_NotBound_ReturnsFalse's first mutation (`return true;`) left the `out` parameter unassigned, so it failed to COMPILE and the harness reported no results rather than a red. `instance = default; return true;` is the valid form. 41 tests were left without a mutation, classified with evidence rather than given an invented one: A3 rejects where the assertion compares two runs of the same code (RngService determinism pairs) or reads Unity/BCL guarantees; A5 duplicates whose only falsifying edit belongs to a named sibling; and D2 overclaims that assert only DoesNotThrow - notably five AssetResolverServiceTest cases where the registration-failure signal is a Debug.LogWarning, which Unity's runner does not fail on. Two OPEN gaps found in passing: ObjectPool.Despawn's `onlyFirst` break is uncovered repo-wide (the EditMode test spawns one entity, the PlayMode sibling's predicate matches one), and DataServiceTest.ReplaceData_Successfully never enters the replace branch at all because its two payloads have different T. EditMode suite green at 826/826. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
59 mutations; 58 observed RED and reverted, production files byte-identical afterwards. EditMode 826/826, PlayMode 299/299. The single holdback confirms a prediction from the drafting pass rather than contradicting it: Despawn_WithCondition_FirstOnly_Successfully stays green when `if (onlyFirst)` is inverted. The EditMode fixture spawns one entity, and the PlayMode typed sibling's predicate matches only one, so ObjectPoolBase.Despawn's onlyFirst break is uncovered REPO-WIDE. That is now observed, not inferred, and is owed a section 13 OPEN row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deleted (A3 - no production symbol in the causal chain): - CommandServiceTest.ServerCommand_ExecuteLogic_InvokedWithGameLogic calls a private test-local class's ExecuteLogic directly; CommandService never runs and IGameServerCommand is a bare declaration. - RngServiceTest.Next_SameSeed_ReturnsDeterministicSequence and Nextfloat_ReturnsDeterministicSequence compare two runs of the SAME pure function, so any edit to GenerateRngState/NextNumber moves both sides identically and equality is preserved by construction. Determinism against a recorded expected sequence would be falsifiable; self-comparison is not. KEPT: RequestAsset_UnknownId_ThrowsMissingMember and IsValidNamespace_SingleSegment_ReturnsTrue - A5 candidates with narrow sibling evidence but no probe. Narrow evidence alone proved insufficient elsewhere in this pass: UnloadUiSet_WithOpenPresenters_UnloadsAll had radius-2 evidence and its probe still came back RED-OK. EditMode 806/806, PlayMode 295/295. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…it proved First trustworthy coverage figure for this repo. Regenerated with -debugCodeOptimization, all 11 GameLovers assemblies in scope, test and sample assemblies excluded. Repo-wide runtime coverage is 74.1% (6609/8922). Do not compare against any earlier number. 41.8% was stale, wrongly scoped to 6 assemblies, and diluted by Editor code; 38.3% was compiled in Release, which silently shrank the denominator ~40%. The register now names the sanity check that catches a repeat: MathfloatP must report ~1002 coverable lines, not 637. The OPEN rows added here are findings the mutation pass PROVED rather than suspected - each one is a mutation that was applied and observed leaving its test green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ones Three assertion pairs used Assert.GreaterOrEqual(ErrorValue, converted - now), which bounds the difference only from ABOVE. Any regression that made a conversion SMALLER was invisible: negating _initialUnityTime, and swapping TotalMilliseconds for TotalSeconds - a 1000x shrink in a time conversion - were both observed staying GREEN. Now Assert.LessOrEqual(Math.Abs(diff), tolerance). Re-verified: both shrink mutations redden, isolated. The third pair (DateTime_Convertions_Successfully) had the same defect and was NOT in the audit's findings - it surfaced while fixing the other two. Same fix applied. Adds UnixErrorMillis = 50d. The unix/DateTime comparisons are in MILLISECONDS while ErrorValue = 0.01f is seconds; reusing ErrorValue there would have made the bound absurdly tight and the test flaky. The two scales now have separate tolerances. RCR: UnityTime_Convertions_Successfully <- TimeService.cs UnityTimeFromDateTimeUtc negate the + _initialUnityTime term (RED, isolated) RCR: UnixTime_Convertions_Successfully <- TimeService.cs UnixTimeFromDateTimeUtc return TotalSeconds instead of TotalMilliseconds (RED, isolated) EditMode 806/806. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five AssetResolverServiceTest cases asserted only Assert.DoesNotThrow. They now read the internal AssetMap - already exposed via InternalsVisibleTo, so no production change was needed - and assert the entries that were actually registered, merged or released. Subscribe_SameSubscriberSameType_LastActionWins_ThenUnsubscribeRemovesOnlyIt (renamed) gains a bystander subscriber on the same message type that must survive the Unsubscribe. Without it the "strengthened" test was still an A5 duplicate: Subscribe_MultipleSubscriptionSameType_ ReplacePreviousSubscription already covered every assertion, and its committed RCR names the same mutation verbatim. The bystander is what makes the pin unique. Worth recording, because it nearly produced a wrong result: the intended anchor `subscriptionObjects.Remove(subscriber);` matches TWICE - not because the sites are identical but because the UnsubscribeAll occurrence is indented one level deeper, making the 3-tab string a substring of the 4-tab line. A uniqueness check on the short string passes by eye and still mutates the wrong site. The spec now uses a multi-line anchor. RCR: AddAsset_NewType_RegistersEntry <- AssetResolverService.cs AddAssets store null instead of assets[i].Value RCR: AddAssets_DuplicateType_MergesEntries <- AssetResolverService.cs AddAssets delete the merge-loop Add RCR: UnloadAssets_WithAssetConfigsContainer_ReleasesAssetsInContainer <- AssetResolverService.cs delete dictionary.Remove(pair.Key) RCR: UnloadAssets_WithIdsArray_ReleasesOnlyMatching <- AssetResolverService.cs dictionary.Remove(id) to Clear() RCR: AddConfigs_DelegatesToAddAssets <- IAssetAdderService default method forwards an empty list RCR: Subscribe_SameSubscriberSameType_LastActionWins_ThenUnsubscribeRemovesOnlyIt <- MessageBrokerService.cs Unsubscribe<T> Remove(subscriber) to Clear() RCR: ReplaceData_Successfully <- DataService.cs AddOrReplaceData neuter the replace branch RCR: VersionExternal_AlwaysAccessible_WithoutLoad <- VersionServices.cs VersionExternal return string.Empty RCR: Despawn_WithCondition_FirstOnly_Successfully <- ObjectPool.cs Despawn invert if (onlyFirst) The last one closes a gap this audit proved: the onlyFirst branch was uncovered repo-wide because both fixtures' predicates matched a single entity. The predicate now matches both. EditMode 806/806, PlayMode 295/295. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
07abdf1 replaced the one-sided GreaterOrEqual(ErrorValue, converted - now) bounds with two-sided bounds on the absolute difference. The 1000x TotalMilliseconds -> TotalSeconds shrink and the _initialUnityTime negation, both previously observed GREEN, are now RED. The unix comparisons carry a separate UnixErrorMillis because they are in milliseconds while ErrorValue is in seconds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
§13 asserted every named symbol was 'either ACCEPTED or OPEN' while four packages had already grown CLOSED rows — the spec forbade rows it contained. CLOSED is now first-class, and it carries a contract: name the commit AND the observation, including the environment the observation came from. A row closed on 'the fix landed' is still OPEN, because the fix is the edit and the closure is the evidence. Second rule: closing a row means re-deriving its claim against current source, never reading the commit that claimed to fix it. A partial fix and a complete one produce the same green suite and the same confident commit message, so the commit cannot be evidence for its own completeness. §1 and §2 are shared verbatim across all six packages; §13's preamble is too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… never applied Same recovery as the gamedata commit: probed and observed RED during the backfill, but the annotate step never reached the source. Gated on a recorded RED-OK verdict per test. Note Despawn_WithCondition_FirstOnly_Successfully carries both a RED-OK and a STAYED-GREEN observation, from two different mutations. The annotation records the one that reddens it (the break body returning false); the section 13 OPEN row about the onlyFirst inversion staying green is a separate, still-owed gap and is untouched. Comment-only diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… unowned-edit measurement Tests/AGENTS.md section 2 said a test with no // RCR: line is a suspect by default, without carving out Smoke/. Section 1 already exempts that directory (its defect class is "the assembly no longer loads", which has no one-line mutation), so the omission flagged those fixtures forever. Exemption is now explicit, on the same directory basis. Also records that "unannotated" is three states, not one: observed RED with the write-back lost, seen reddening only as collateral, or never probed. Only the last needs a probe, and prepared annotation text must never be written without a matching RED-OK - it exists for tests that were never probed, and writing it fabricates a verified claim. Adds a section 13 row for the measured count of production edits that redden only collaterally (223 repo-wide, from .test-all/rcr/unowned-edits.json). Recorded with the caveat that it is NOT that many missing tests: for foundational primitives and the UiService integration hub, having no isolated owner follows from centrality, not neglect.
Mechanical pass, no behaviour change. - Removed the AssetReferenceScene constructor's doc block (§6.6 never XML-documents constructors). - Converted the doc comments on 12 private members to `//` comments rather than deleting them, so the rationale survives where §6.6 forbids `///` on private code. Two carry real knowledge worth keeping: ObjectPoolBase.IsDestroyedOrNull's explanation of why `entity == null` cannot see a destroyed UnityEngine.Object through a `class`-only generic constraint, and VersionServices.Bootstrap's note that ordering between SubsystemRegistration callbacks across assemblies is undefined. - Added `/// <inheritdoc />` to 41 overrides whose base declaration is already documented: the Explorer tab BuildUi/Refresh/OnExitingPlayMode family, and Equals/GetHashCode on TickData. - Moved the internal AddressableIdsGeneratorUtils.BuildEnumSource and AssetResolverService.SelectAsset test seams above their types' private blocks (§6.6: `internal` is never interleaved with `private`), and removed the change-narration sentences from both summaries — "Extracted from …; no behaviour change versus the logic it replaces" is diff context, which §6.6's Code comments rule forbids. Verified: Tools/style-audit.py reports 0 remaining mechanical-class violations here; brace and paren counts are unchanged against HEAD on every touched file (including the two whose members moved); and every file's UTF-8 BOM still matches HEAD, after an intermediate pass stripped two of them. The 5 findings left are overrides whose own base declarations are undocumented (GameObjectPool.SpawnEntity/PostDespawnEntity, AsyncCoroutine.OnCompleteTrigger); `<inheritdoc />` there would yield empty IntelliSense, so they are handled with their bases in the prose pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ot exist
MultipleInstances_CreateMultipleGameObjects carried "// RCR: none exists -- only a structural
change (making the guard static) flips it". That was wrong: TickService.cs declares
`private readonly TickServiceMonoBehaviour _tickObject;` and changing that one line to
`private static TickServiceMonoBehaviour _tickObject;` is a single-line edit. Run: the test
went RED with production's own InvalidOperationException, "The tick service is being
initialized for the second time and that is not valid". TickService.cs restored byte-identical.
Also removes 15 lines from the test body: a narration block ("Wait, I saw a check in the
constructor:") wrapping a commented-out copy of the production constructor. The rationale it
carried is real and now lives in the one-line ADMIT above; the commented-out code was dead.
Records a new section 13 row for what the mutation exposed: that ctor guard is unreachable.
It reads a readonly INSTANCE field assigned later in the same constructor, so it is always
null when checked, and its message promises singleton protection the package deliberately does
not provide (see the package AGENTS.md section 4). Deleting the dead guard is owed but is a
production edit outside this pass.
RCR: MultipleInstances_CreateMultipleGameObjects <- TickService.cs _tickObject readonly -> static
Editor PlayMode 295/295.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…st's RCR
The TickService constructor opened with a guard that could never fire:
if (_tickObject != null) throw new InvalidOperationException("...second time...");
_tickObject is a readonly INSTANCE field assigned later in that same constructor, so it is
always null at the check. The guard advertised singleton protection the service deliberately
does not provide - AGENTS.md section 4 documents multi-instance as the contract, and
MultipleInstances_CreateMultipleGameObjects pins it. CoroutineService, which spawns its host
the same way, never carried such a guard; deleting this one makes the pair consistent.
Deleting it voided the test's RCR, so the mutation was re-derived rather than assumed:
- Old mutation re-run WITH the guard gone: _tickObject readonly -> static left the test GREEN
(observed, not inferred). Recorded on the test as a negative result so the next reader does
not repeat it.
- New mutation: the ctor's own AddComponent, routed through
Object.FindAnyObjectByType<TickServiceMonoBehaviour>() ?? gameObject.AddComponent<...>() -
a simulated singleton - went RED with "Expected: greater than or equal to 2". Green -> RED ->
green, TickService.cs restored to the guard-deleted state (md5 verified).
RCR: MultipleInstances_CreateMultipleGameObjects <- TickService.cs ctor AddComponent -> reuse an
existing host
Batchmode EditMode 805/805, PlayMode 295/295.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The earlier §6.6 pass converted every private member's doc block into a `//`
comment to avoid losing rationale. That was too generous: a comment on a private
member is a last resort, and needing one is usually evidence the member is wrong.
§6.6 now says so explicitly, and this applies it.
Deleted outright — the name and signature already say it:
- GitEditorProcess.ExecuteCommand ("Execute a command eg. status --verbose")
- VersionEditorUtils.OnEditorLoad (an [InitializeOnLoadMethod] that sets the
version — the attribute and body carry it)
- VersionEditorUtils.SaveVersionData
- RngService.NextNumber ("Generates the next random number ... based on rndState")
- MobileSimulatorRuntimeOverlay.OverlayController, whose second sentence was also
change narration about a window that no longer exists
Reduced to only what the code cannot state:
- SortedSetDiff -> its pre-sorted precondition, which no type enforces
- ResolveSanitizedEnumName -> the name_filetype disambiguation strategy, dropping
the "used by X and Y" call-graph narration §6.6 forbids in <remarks>
- GetRelativePath -> the silent unchanged-string fallback
- VersioningMenu.Refresh -> what the `false` argument means
- VersionServices.Bootstrap -> the cross-assembly SubsystemRegistration race and
why EnsureLoaded is what closes it, dropping the consumer-facing sentence that
belongs on the public API and is already in AGENTS.md §4
- AssetResolverTab / MessageBrokerTab digests -> a pointer at AGENTS.md §4, which
documents the rapid-click rationale at length, instead of restating it
Kept in full: ObjectPoolBase.IsDestroyedOrNull. A `class`-only generic constraint
cannot dispatch to UnityEngine.Object's `==`, so the fake-null hazard is invisible
in the code and nothing but prose can convey it.
Net: 31 comment lines removed, 9 added.
Verified after this pass: Tools/style-audit.py still reports 0 for every
mechanical rule class, and batchmode is green — EditMode 805/805, PlayMode 295/295.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes this package — Tools/style-audit.py reports 0 items for
com.gamelovers.services.
- Documented the internal editor-introspection accessors as one consistent set
(AssetMap, ActiveAsyncCoroutines, DataEntries, Bindings, InstallerInstance,
Subscriptions, IsPublishing, Pools, the three tick lists, ExtraTime,
InitialTime). Each points at AGENTS.md §4 rather than restating the pattern.
- Documented ObjectPoolBase's subclassing surface, keeping the two things the
signatures cannot say: SpawnEntity retries through IsDestroyedOrNull because an
external owner can destroy a pooled entity while it sits in the stack, and the
CallOn* hooks are virtual because this base casts the entity directly whereas
the GameObject pools resolve it with GetComponent.
- Documented the AssetsConfigsImporter extension points and the generator
importer's three abstract members; IndexOfId records that the trailing dot in
"/{id}." is what stops a prefix matching.
- Documented the AddressableIdsEditorSettings snapshot readouts, including that a
filename or label-filter change is what makes a snapshot stale.
- The 19 Explorer-tab DisplayName / RefreshIntervalMs overrides now carry
`/// <inheritdoc />` against ServiceTab's existing summaries. These were missed
by the mechanical pass because it only considered overridden METHODS, not
properties.
Removed the `/// <inheritdoc />` from ScriptNameEditAction.Action / Cancelled in
both #if branches: they are `public override`s of a Unity base but live inside a
`private` nested class, so §6.6 treats them as private. Effective accessibility,
not the declared modifier, is what the rule turns on.
Verified: batchmode green — EditMode 805/805, PlayMode 295/295.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A docs pass rewrote the file with LF, changing every historical byte and breaking the release-notes validator's baseline comparison. Restores the committed convention per AGENTS.md; no content change.
Unity's packer uses .gitignore as its pack-ignore list, so listing .github/ drops the CI workflow from the tarball while git keeps tracking it (gitignore does not untrack existing files). Verified on a real clone: 434 -> 433 entries, .github 1 -> 0, Runtime unchanged.
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
1 participant
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.
Changed:
com.unity.test-framework.performance(3.5.0) dependency so the package's test assemblies compile when tests are enabled.Fixed:
GameObjectPool.Dispose(false)and its generic equivalent so they preserve the sample entity when requested.GameObjectandBehaviourinstances are skipped instead of being returned to callers.