refactor: extract mouse UI pointer target resolution

[hatayama]

Summary

• extract pointer data construction, hierarchy/path lookup, drop validation, and direct raycast construction into MouseUiPointerTargetResolver
• move the executor-consumed resolution result into the top-level ResolvedPointerTargets DTO
• add synchronous EditMode characterization coverage for path, drop, bypass hierarchy, pointer state, and no-hit behavior
• reduce SimulateMouseUiUseCase from 1,305 to 1,032 lines

Scope Notes

• The characterization commit advances five resolver methods and ResolvedPointerTargets from private to their required final internal visibility. The final implementation retains the existing signatures and nullability.
• The only non-move behavior-preserving adaptation removes the one-line RaycastUI wrapper. Its resolver and executor callers now invoke UiRaycastHelper.RaycastUI directly, matching the existing drag-finalization caller.
• The two existing out-parameter contracts remain byte-equivalent for move-only review. Their conversion to an explicit result DTO is recorded separately as R2-41.
• Event dispatch, drag execution/finalization, overlay and main-thread cleanup, logging, and action execution remain with later R2-33 stages.
• IPC request/response DTOs and wire shapes are unchanged, so no protocol version bump is required.

Verification

• dist/darwin-arm64/uloop compile (0 errors, 0 warnings)
• EditMode MouseUiPointerTargetResolverTests before and after the move (10 passed)
• PlayMode SimulateMouseUiTests (32 passed)
• EditMode MouseUiSimulationResponseFactoryTests (12 passed)
• EditMode SimulateMouseUiActionValidationTests (1 passed)
• EditMode PlayModeToolPreflightServiceTests (8 passed)
• EditMode StaticFacadeStateGuardTests (20 passed)
• EditMode OnionAssemblyDependencyTests (83 passed)
• normalized old/new resolver comparison (only the declared RaycastUI direct-call adaptation remains after visibility normalization)
• old owner definitions, reverse references, and RaycastUI wrapper references (0)
• git diff --check origin/v3-beta...HEAD

[coderabbitai]

📝 Walkthrough

Walkthrough

Extracts pointer/drop target resolution logic from SimulateMouseUiUseCase into a new MouseUiPointerTargetResolver helper and a ResolvedPointerTargets struct. SimulateMouseUiUseCase is refactored to delegate to the new resolver for click, long-press, and drag flows. New Editor unit tests validate the resolver's behavior.

Changes

Mouse UI Pointer Target Resolver Extraction

Layer / File(s)	Summary
Resolved targets data contract Packages/src/Editor/FirstPartyTools/SimulateMouseUi/ResolvedPointerTargets.cs	New internal readonly struct with Empty, Success, and Failure static factories carrying raw/press/click/hierarchy targets or a failure response.
Resolver core logic Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPointerTargetResolver.cs	New helper resolves pointer press targets (bypass or raycast), event-handler targets, drop targets with IDropHandler validation, hierarchy path lookup with not-found/ambiguous failures, and direct raycast result creation.
UseCase wiring Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs	ExecuteClick, ExecuteLongPress, ExecuteDragOneShot, ExecuteDragStart, ExecuteDragEnd, and FinalizeDrag now delegate to MouseUiPointerTargetResolver and UiRaycastHelper.RaycastUI; old local helper methods and private types are removed.
Resolver unit tests Assets/Tests/Editor/MouseUiPointerTargetResolverTests.cs	New test fixture covers pointer press mapping, hierarchy/drop path resolution outcomes, ResolvePressablePointerTargets, CreateDirectRaycastResult, plus test helpers and a handler MonoBehaviour.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant UseCase as SimulateMouseUiUseCase
  participant Resolver as MouseUiPointerTargetResolver
  participant Hierarchy as GameObject Hierarchy
  participant Raycast as UiRaycastHelper

  UseCase->>Resolver: CreatePointerPressData(button, position)
  UseCase->>Raycast: RaycastUI(position) [if not bypass]
  Raycast-->>UseCase: RaycastResult
  UseCase->>Resolver: ResolvePressablePointerTargets(command, raycastResult)
  alt BypassRaycast enabled
    Resolver->>Hierarchy: FindActiveGameObjectByPath(TargetPath)
    Hierarchy-->>Resolver: matched GameObject or ambiguous/not-found
    Resolver->>Resolver: CreateDirectRaycastResult(target)
  else Raycast hit
    Resolver->>Resolver: ResolveRaycastPressablePointerTargets(hitResult)
  end
  Resolver->>Resolver: CreateResolvedPressablePointerTargets(target)
  Resolver-->>UseCase: ResolvedPointerTargets
  UseCase->>Resolver: TryResolveDropTargetPath(dropPath) [drag flows]
  Resolver->>Hierarchy: FindActiveGameObjectByPath(dropPath)
  Resolver-->>UseCase: drop target or failure response

Loading

Possibly related PRs

• hatayama/unity-cli-loop#996: Implements the same bypass raycast via hierarchy targetPath/dropTargetPath targeting flow now centralized in MouseUiPointerTargetResolver.
• hatayama/unity-cli-loop#1366: Modifies UiRaycastHelper.RaycastUI, which SimulateMouseUiUseCase now calls directly via the refactored resolver flow.
• hatayama/unity-cli-loop#1516: Also refactors SimulateMouseUiUseCase.cs pointer-target resolution and payload handling in the same functional area.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: extracting mouse UI pointer target resolution into a dedicated helper.
Description check	✅ Passed	The description matches the refactor scope and testing coverage described in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/hatayama/extract-mouse-ui-target-resolver

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

No issues found across 7 files

_{You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

_{Re-trigger cubic}

[hatayama]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPointerTargetResolver.cs (1)

190-219: 🚀 Performance & Scalability | 🔵 Trivial

Consider caching or narrowing FindObjectsByType<GameObject> for path resolution.

FindObjectsByType<GameObject> allocates an array of every GameObject in the scene on each call. In ExecuteDragOneShot, this can be invoked twice per request (target path + drop target path). This is pre-existing behavior from the extraction, but as the resolver is now a centralized hot path, a future optimization could cache results per-frame or use a dictionary keyed by path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPointerTargetResolver.cs`
around lines 190 - 219, The hot-path lookup in FindActiveGameObjectByPath still
scans all GameObjects via FindObjectsByType<GameObject> on every call, which is
expensive when ExecuteDragOneShot resolves both target and drop target paths.
Narrow the search or cache path-to-GameObject results for the duration of a
frame/request, and reuse that cache from MouseUiPointerTargetResolver instead of
rebuilding the full array each time.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPointerTargetResolver.cs`:
- Around line 190-219: The hot-path lookup in FindActiveGameObjectByPath still
scans all GameObjects via FindObjectsByType<GameObject> on every call, which is
expensive when ExecuteDragOneShot resolves both target and drop target paths.
Narrow the search or cache path-to-GameObject results for the duration of a
frame/request, and reuse that cache from MouseUiPointerTargetResolver instead of
rebuilding the full array each time.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e4926826-513f-4a7f-926b-7245ed196a10

📥 Commits

Reviewing files that changed from the base of the PR and between ac53001 and 16de4da.

⛔ Files ignored due to path filters (3)

• Assets/Tests/Editor/MouseUiPointerTargetResolverTests.cs.meta is excluded by none and included by none
• Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPointerTargetResolver.cs.meta is excluded by none and included by none
• Packages/src/Editor/FirstPartyTools/SimulateMouseUi/ResolvedPointerTargets.cs.meta is excluded by none and included by none

📒 Files selected for processing (4)

• Assets/Tests/Editor/MouseUiPointerTargetResolverTests.cs
• Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPointerTargetResolver.cs
• Packages/src/Editor/FirstPartyTools/SimulateMouseUi/ResolvedPointerTargets.cs
• Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiUseCase.cs

[hatayama]

Reviewed the CodeRabbit caching nit. The full-scene scan is pre-existing and occurs only on explicit CLI bypass/drop path resolution; there is no measured performance problem. Adding a cache would introduce request lifetime, scene-mutation invalidation, and duplicate-path semantics beyond this move-only refactor. Under YAGNI, no code change or follow-up is warranted unless profiling demonstrates a real bottleneck. fable5 independently reviewed and agreed with this decision.