Skip to content

Commit d7f287e

Browse files
eordanoclaude
andauthored
fix: resolve null-forgiving suppression on events-tab fetch, trim test comments
nickkhalow's review blocked on the diff-ratchet lint BLOCK for `place!.Positions` at PlaceInfoPanelController.cs:439: the diff retyped the fetch's return value but re-touched a pre-existing null-forgiving dereference instead of guarding it. Root cause is that FetchEventsAndShowThemAsync never establishes that `place` is non-null before using it, unlike sibling methods in this file (SetOriginParcel, StartNavigation) which already guard with `if (place == null) return;`. Add the same guard and capture the narrowed value into a local before the SetAsLoadingState() call runs (a field's narrowed null-state does not survive a method call), so the fetch call site no longer needs `!`. The file's other six pre-existing `place!` occurrences are left untouched as out of scope for this PR. Also trims PlaceInfoPanelControllerEventOrderingShould.cs's three comment blocks (class doc + two SetUp asides) down to this repo's established 1-3 line regression-comment convention (JumpButtonShould.cs, LiveKitChatMessagesBusShould.cs, DebugWebRequestInfoShould.cs); the narrative content they trimmed was independently verified accurate but belongs in the PR description, not the diff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018c638dR1vPysCMbYt2qQg5
1 parent 54c969a commit d7f287e

2 files changed

Lines changed: 10 additions & 39 deletions

File tree

Explorer/Assets/DCL/Navmap/PlaceInfoPanelController.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,9 +434,12 @@ async UniTaskVoid FetchEventsAndShowThemAsync(CancellationToken ct)
434434
{
435435
view.EmptyEventsContainer.SetActive(false);
436436

437+
if (place == null) return;
438+
PlacesData.PlaceInfo placeInfo = place;
439+
437440
SetAsLoadingState();
438441

439-
EventDTO[] events = await eventsApiService.GetEventsByParcelAsync(place!.Positions, ct);
442+
EventDTO[] events = await eventsApiService.GetEventsByParcelAsync(placeInfo.Positions, ct);
440443

441444
ClearEventElements();
442445

Explorer/Assets/DCL/Tests/Editor/PlaceInfoPanelControllerEventOrderingShould.cs

Lines changed: 6 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,35 +15,9 @@
1515

1616
namespace DCL.Tests.Editor
1717
{
18-
// Regression coverage for https://github.com/decentraland/unity-explorer/issues/9529:
19-
// "Map -> Genesis Plaza -> Events tab: events out of order, order changes on every reopen".
20-
//
21-
// PlaceInfoPanelController's Events-tab loading skeleton (SetAsLoadingState(), a local
22-
// function inside the private FetchAndShowEventsOfThePlace()) pulls rows from a
23-
// stack-based UnityEngine.Pool.ObjectPool<EventElementView> without ever repositioning
24-
// them under their parent, so a reused row keeps whatever transform-sibling slot it last
25-
// occupied. Because the pool is LIFO, releasing N rows in list order and then
26-
// re-acquiring N rows hands them back in the exact reverse order - so after any "reopen"
27-
// the row the controller *thinks* is first (eventElements[0]) is not the GameObject that
28-
// is actually first on screen (sibling index 0). The fix adds
29-
// `element.transform.SetAsLastSibling()` right after every `eventElementPool.Get()` (both
30-
// in this skeleton loop and in the main fetch-result loop) so on-screen order always
31-
// tracks population order.
32-
//
33-
// This test drives the real, unmodified SetAsLoadingState()/FetchAndShowEventsOfThePlace()
34-
// through reflection: FetchAndShowEventsOfThePlace is private but its name and signature
35-
// are untouched by the patch (only its body changes), so reflecting into it is stable
36-
// across both the unpatched and patched source - unlike its compiler-generated local
37-
// functions, whose generated names could shift with the patch. The heavy constructor
38-
// (~15 unrelated view/service dependencies) is bypassed via FormatterServices
39-
// .GetUninitializedObject - already precedented in this repo, see
40-
// NearbyAudioPerformanceManualTest.cs - and only the handful of private fields this one
41-
// code path actually touches are injected.
42-
//
43-
// Scope: this covers the loading-skeleton half of the patch (SetAsLastSibling in
44-
// SetAsLoadingState). The sort's order contract is covered separately by
45-
// EventDisplayOrderComparerShould; driving the main fetch-result loop with real event
46-
// data would additionally require a fully wired EventElementView hierarchy.
18+
// Regression coverage for https://github.com/decentraland/unity-explorer/issues/9529: pooled
19+
// event rows kept their stale transform-sibling slot on reopen, so on-screen order silently
20+
// diverged from population order. Reflects into the private FetchAndShowEventsOfThePlace().
4721
[TestFixture]
4822
public class PlaceInfoPanelControllerEventOrderingShould
4923
{
@@ -77,12 +51,8 @@ public void SetUp()
7751
actionOnRelease: result => result.gameObject.SetActive(false),
7852
defaultCapacity: SKELETON_ROW_COUNT);
7953

80-
// Hermetic double for the one HTTP fetch this code path performs, following the
81-
// same IWebRequestController.SendAsync<...> generic-method stubbing idiom already
82-
// used in DCL/Profiles/Tests/RealmProfileRepositoryShould.cs. The stubbed task is
83-
// deliberately left forever-pending: this test only needs the synchronous
84-
// pre-await portion of FetchAndShowEventsOfThePlace (the loading skeleton), so the
85-
// event payload never has to arrive.
54+
// Stub double for the one HTTP fetch this path performs; left forever-pending since this
55+
// test only needs the synchronous loading-skeleton portion, before the awaited response.
8656
IWebRequestController webRequestController = Substitute.For<IWebRequestController>();
8757
webRequestController.RequestHub.Returns(Substitute.For<IRequestHub>());
8858

@@ -101,9 +71,7 @@ public void SetUp()
10171
PlaceInfoPanelView view = new GameObject("PlaceInfoPanelView").AddComponent<PlaceInfoPanelView>();
10272
SetPrivate(view, "EmptyEventsContainer", new GameObject("EmptyEventsContainer"));
10373

104-
// Bypasses PlaceInfoPanelController's heavy constructor (it wires ~15 view
105-
// buttons/services unrelated to this code path) and instead injects only the
106-
// fields FetchAndShowEventsOfThePlace()/SetAsLoadingState() actually touch.
74+
// Bypasses the heavy constructor; injects only the fields this code path touches.
10775
controller = (PlaceInfoPanelController) FormatterServices.GetUninitializedObject(typeof(PlaceInfoPanelController));
10876
SetPrivate(controller, "view", view);
10977
SetPrivate(controller, "eventElementPool", pool);

0 commit comments

Comments
 (0)