Skip to content

RuntimeSingleton safety, WProto shapes, IntMap, JSON allocs - #581

Merged
Eli Pinkerton (wallstop) merged 17 commits into
mainfrom
dev/wallstop/session-229-issue-sweep
Aug 27, 2026
Merged

RuntimeSingleton safety, WProto shapes, IntMap, JSON allocs#581
Eli Pinkerton (wallstop) merged 17 commits into
mainfrom
dev/wallstop/session-229-issue-sweep

Conversation

@wallstop

@wallstop Eli Pinkerton (wallstop) commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Why: A pass over the open-issue backlog: preserve scene-authored runtime singleton state, finish the measured WallstopProto standard-library sweep, add the int-keyed map the benchmarks asked for, and fix the named JSON allocation class.

What:

Fixes #504, fixes #577, fixes #582


Note

Medium Risk
Play-mode singleton behavior and JSON/protobuf deserialization paths change in production-facing code; CI cache invalidation logic affects build reliability but not game runtime.

Overview
This PR bundles backlog fixes across singleton lifecycle, protobuf BCL types, JSON deserialization, a new int map, and Unity CI warm-cache rules.

Runtime singletons no longer destroy scene-authored RuntimeSingleton components when entering Play Mode: BeforeSceneLoad only resets static caches via ResetAllRegisteredCaches, while ClearInstance / ClearAllRegisteredInstances still tear down live objects for tests.

WallstopProto adds char (varint code unit, repeated elements never packed) and Uri (UTF-8 of OriginalString, strict decode). Generator Shape gains NeverPacked; runtime mirrors that with IWProtoNeverPacked. IntPtr, UIntPtr, and Type stay WPROTO003 refusals with documented reasons. Differential tests and docs are updated.

JSON array reads use JsonArrayAccumulator<T> with pooled growth so steady state avoids list backing-array churn plus ToArray; dictionary/set converters match property names with ValueTextEquals instead of allocating strings.

IntMap<TValue> is a new open-addressing int-key map (raw int keys, power-of-two table, tombstones) aimed at hit-heavy lookups vs Dictionary<int,int>.

Unity CI invalidates Bee/ScriptAssemblies caches when the repo root or package source path inventory changes (content-only edits keep the warm cache); the source inventory marker is written only after passing NUnit results.

Reviewed by Cursor Bugbot for commit 40379b4. Bugbot is set up for automated code reviews on this repo. Configure here.

Emit SHA-256 return-log digests from both evidence producers in the
return-unity-license composite and pass them to both classifier call
sites, then move every build-lock pin to v1.13.0 together. Closes #498.
Both oracle majors write a char as a plain varint and a Uri as the
OriginalString bytes, so both now serialize without a surrogate. The
remaining standard-library shapes stay refused on recorded evidence.
Closes part of #399.
Array growth rents pooled buffers instead of leaving a discarded List
backing array behind at each doubling, and collection property names
compare against the raw token without allocating. Closes #504.
Linear probing with raw-int comparison and a power-of-two table beats
Dictionary on hit-heavy lookups, measured under the settled-heap
protocol. Closes #578.
The folk advice costs eleven percent on Unity's Mono, where a
specialized enum comparer already ships. Closes #577.
Copilot AI lite review requested due to automatic review settings August 26, 2026 21:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@wallstop

Copy link
Copy Markdown
Collaborator Author

Heads-up for triage: the Copilot Code Review run fails with SessionModelError: You have exceeded your monthly quota — organization billing state, unrelated to the diff (same shape as #460's history). All repository gates are proceeding normally.

Comment thread Runtime/Core/DataStructure/IntMap.cs
Comment thread Runtime/Core/Serialization/JsonConverters/WJsonArray.cs
Comment thread Runtime/Core/Serialization/WallstopProto/WProtoBclFormatters.cs
Comment thread Runtime/Core/DataStructure/IntMap.cs
Comment thread Runtime/Core/DataStructure/IntMap.cs Outdated
Comment thread Runtime/Core/DataStructure/IntMap.cs
Clear now restores empty markers instead of zeroing keys into live
slots; the array accumulator copies before the pooled buffer goes
back; char closes through the runtime scalar registry with oracle-
pinned goldens; a huge capacity hint throws instead of spinning.
IntMap is an IReadOnlyDictionary and Reset honors the version guard.
The cleanup classifier pins return to the tolerant commit until the
truncation strictness in #356 has an upstream answer.
Copilot AI review requested due to automatic review settings August 27, 2026 00:54
@wallstop

Copy link
Copy Markdown
Collaborator Author

All four Bugbot findings are fixed, and the red Unity legs were a separate root cause, now addressed:

Bugbot fixes (commit 99c1a3c):

  1. Clear corrupting empty slots — real, and it is what actually reddened every Unity leg (IntMapTests.ClearEmptiesTheMapButKeepsItUsable: Expected 0, was 1024). Clear now restores EmptySlot per key slot before clearing values; the remarks on the method record why an array-wide zero fill is wrong for this table.
  2. Finish() copy-after-pool-return — real. The exact-size copy now happens before Dispose(), with the ordering documented; this is what turned "a","b" into [null,null] across the SerializableDictionary/JSON fuzz failures.
  3. char missing from the runtime scalar registry — real gap in generated-contract-visible terms only via raw-closure paths (WProtoCollectionMarshals.CanEncode<char>), since contract members inline primitives. Registered CharFormatter, pinned by a golden test in WProtoContractFormatterTests and Box<char> rows in both-oracle differential tests.
  4. Unbounded power loop — real on a 1 << 31 hint. The loop is bounded at MaximumTablePower and the constructor throws ArgumentOutOfRangeException immediately (verified: throws in <1 ms).

Owner feedback:

  • Should IntMap implement IDictionary/IReadOnlyDictionary? It now implements IReadOnlyDictionary<int, TValue> (Keys/Values/ContainsKey/TryGetValue view over live slots). Full IDictionary<T> was deliberately left off: its mutable surface would offer a second write path that bypasses tombstone bookkeeping semantics without adding capability over TrySet/indexer.
  • Do we care about resetting version in Reset()? Yes — Reset() now honors the same version guard as MoveNext(), so a stale enumerator fails loudly everywhere.

Why every Unity leg failed while tests mostly passed: the pin bump onto the v1.13.0 cleanup classifier activated #356's known strictness — Unity 2021–2022 legs crash after writing valid results, truncating the return log, and the newer classifier refuses that evidence (cleanup-reason=return-log-truncated). The digest plumbing stays; the classify/gate pins return to the tolerant commit until upstream tolerates that condition or we own a graceful-return path. scripts/tests/test-portable-cleanup-classifier.js passes against the reverted pin.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread Runtime/Core/Serialization/WallstopProto/WProtoScalarFormatters.cs
Comment thread Runtime/Core/DataStructure/IntMap.cs Outdated
Copilot AI review requested due to automatic review settings August 27, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

char opts out of packed runs through a package-internal marker the
runtime packability check honors, and the Keys and Values views
return struct enumerators that fail fast on version changes --
typed foreach over either view allocates nothing, verified to zero
bytes.
Copilot AI review requested due to automatic review settings August 27, 2026 02:07
@wallstop

Copy link
Copy Markdown
Collaborator Author

Second Bugbot round addressed (commit d29b9ce):

  • Char scalar path may pack repeats — real. Added the package-internal IWProtoNeverPacked marker (the runtime mirror of the generator's Shape.NeverPacked) and made WProtoGeneric<T>.Packable honor it, so WProtoGeneric<char>.Packable is now false, pinned by an assertion in CharClosesThroughTheRuntimeScalarPath. The runtime write path always emits per-element keys either way; this closes the packed-read/packed-build hole for raw char collections.
  • Keys and Values skip version checks — real. Both views now return struct enumerators (KeyEnumerator/ValueEnumerator via public KeyView/ValueView) that fail fast on every MoveNext() and Reset() against a version change, matching pair enumeration. Typed foreach over either view allocates zero bytes — verified locally with GC.GetAllocatedBytesForCurrentThread() deltas of exactly 0 across repeated sweeps.

Both fixes are regression-tested in IntMapTests (KeysAndValuesViewsEnumerateLiveEntriesWithoutAllocating, KeyAndViewEnumeratorsFailFastWhenTheMapChanges).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The licensed matrix is being refused at the license-return layer by
runner capacity quarantine, not by anything this branch encodes, so
the pins go back to main exactly. Issue #584 carries the evidence.
Copilot AI review requested due to automatic review settings August 27, 2026 05:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Reset RuntimeSingleton caches before scene load without invoking the destructive cleanup sweep. Keep explicit registry teardown intact for test isolation and cover authored NeverCreate state recovery in Play Mode.
Copilot AI review requested due to automatic review settings August 27, 2026 05:19
@wallstop Eli Pinkerton (wallstop) changed the title License digest, WProto shapes, IntMap, JSON allocs RuntimeSingleton safety, WProto shapes, IntMap, JSON allocs Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 27, 2026 06:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c0239f7. Configure here.

Comment thread scripts/tests/test-unity-workflow-matrix-contract.ps1 Outdated
Copilot AI review requested due to automatic review settings August 27, 2026 07:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 27, 2026 16:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copy link
Copy Markdown
Collaborator Author

Correction to the earlier Unity-failure attribution: failed-job rerun 33050443415 disproved the claim that every remaining red leg was cleanup-only. The pre-Unity-6 standalone players deterministically crashed in IntMapTests_MeasureAllocated because this new test directly called GC.GetAllocatedBytesForCurrentThread() without the repository's required IL2CPP guard. Jobs 98590263098 (2022) and 98590263120 (2021) reproduced it; all Edit/Play Mode and Unity 6 legs passed. Commit 40379b4 adds GCAssert.IgnoreIfAllocationMeasurementUnavailable() before warmup/counter access while retaining unconditional live-entry correctness assertions. The separate latest-build-lock cleanup/adoption work remains open in #498/#411.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants