feat: avatar movement using double click by olavra · Pull Request #7978 · decentraland/unity-explorer

olavra · 2026-03-31T23:56:49Z

Pull Request Description

Now you can click anywhere in the world and the avatar will try to reach the destination in a straigh line. A marker determines the target location. You can jump, double jump, glide and shift to run while automatically reaching the destination.
The avatar will stop after a few milliseconds if there is no sustancial increment in movement in the desired direction (cant reach it).

This does not use Navmesh Agents so its pretty simple straight movement, nothing fancy.

Test Instructions

Double click anywhere, a marker will appear
The avatar will try to move in that direction
The marker should dissapear after reaching the destination or failing to reach.
Only one marker visibile at a time
You can shift to run , space to jump and glide while automatically walking.
Open Controls menu to see new input double click on mouse left

- Added Rainbow Effect on marker - Single instance of the marker - Enabled jump and glide while auto walking - Removed Animator Contoller from the Global Plugin System

github-actions · 2026-03-31T23:57:03Z

Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below.

Name	Link
Commit	`215bc03`
Logs	https://github.com/decentraland/unity-explorer/actions/runs/24083790031
Download Windows	https://github.com/decentraland/unity-explorer/suites/63616671377/artifacts/6308620521
Download Windows S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/feat/mouse-move/pr-20766-215bc03/Decentraland_windows64.zip
Download Mac	https://github.com/decentraland/unity-explorer/suites/63616671377/artifacts/6308736686
Download Mac S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/feat/mouse-move/pr-20766-215bc03/Decentraland_macos.zip
Built on	2026-04-07T15:08:24Z

…ty-explorer into feat/mouse-move

AnsisMalins · 2026-04-02T22:06:37Z

feat: avatar movement using double click

PR number: 7978
Windows change build
Windows baseline build
macOS change build
macOS baseline build

Intel Core i3

Change samples: 2671
Baseline samples: 2675

Metric	Change	Baseline	Delta	Improvement
CPU average	33.6 ms	33.6 ms	0.0 ms	-0.1%	⚪
CPU 1% worst	55.4 ms	54.3 ms	1.0 ms	-1.9%	⚪
CPU 0.1% worst	118.3 ms	120.4 ms	-2.1 ms	1.8%	⚪
GPU average	15.4 ms	14.7 ms	0.7 ms	-4.5%	🔴
GPU 1% worst	21.5 ms	21.1 ms	0.4 ms	-1.7%	⚪
GPU 0.1% worst	22.6 ms	23.0 ms	-0.4 ms	1.7%	⚪

daniele-dcl

Besides the 1 change to the grass material that seems unintentional, everything else is minor 👍

daniele-dcl · 2026-04-03T09:09:00Z

Explorer/Assets/DCL/AvatarRendering/AvatarShape/Assets/DestinationMarkerView.cs

+        private static readonly int IDLE_STATE = Animator.StringToHash("Idle");
+        private static readonly int EMISSION_COLOR = Shader.PropertyToID("_EmissionColor");
+
+        [SerializeField] public float rainbowSpeed = 1f;


Usually we go with a property with private setter and use [field: SerializeField] on it

Done in the last comit

daniele-dcl · 2026-04-03T09:10:59Z

Explorer/Assets/DCL/AvatarRendering/AvatarShape/Assets/DestinationMarkerView.cs

+        {
+            if (transitionedToIdle || animator == null) return;
+
+            AnimatorStateInfo info = animator.GetCurrentAnimatorStateInfo(0);


I am not 100% sure if I understood this correctly. It seems the goal is to transition the animator to a given state after a while?

I believe we would want to do that with the exit time of the default state of the animator instead?

Is there a good reason to do it programmatically?

I was having problems with the animator controller transition and end up doing it programatically. It was a problem with the animation, so reverted this and made the transition directly using the exit time.

daniele-dcl · 2026-04-03T09:17:05Z

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

+        private readonly GameObject markerPrefab;
+
+        // Stores the time of the previous left-click to detect double-clicks.
+        private float lastClickTime = float.MinValue;


It would be nice to check if it's possible to do this in the input actions asset by using a multi-tap interaction.

Yeah! I chanted it use the multitap option of the input system.

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

daniele-dcl · 2026-04-03T09:22:09Z

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

+            };
+
+            if (World.Has<PointAndClickMovementComponent>(entity))
+                World.Set(entity, component);


IIRC we have AddOrSet.

You were right, we have that. Changed.

daniele-dcl · 2026-04-03T09:24:08Z

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

+                    Vector2 screenPos = mouse.position.ReadValue();
+                    Ray ray = cameraComponent.Camera.ScreenPointToRay(new Vector3(screenPos.x, screenPos.y, 0f));
+
+                    if (Physics.Raycast(ray, out RaycastHit hit))


Maybe we can consider filtering this out by layer? Maybe also max distance to avoid super far away clicks?

I've added the max distance configurable in the CharacterControllerSettings. Regarding the layers I would like th player to be able to click anywhere in the scene, the floor and even the avatars, I dont know the right combination of layers here... :S

daniele-dcl · 2026-04-03T09:24:43Z

Explorer/Assets/DCL/Landscape/Assets/GPUI/Grass/StylizedGrass_Indirect.mat

  m_ValidKeywords:
  - _ADVANCED_LIGHTING
  - _FADING
+  - _GPU_GRASS_BATCHING


Are changes to this material intentional?

Yeah i tried my best to not tick these mat changes in the commits but I failed. Shall we include those in the gitignore?

daniele-dcl · 2026-04-03T09:26:34Z

@claude review

claude · 2026-04-03T09:27:18Z

Claude finished @daniele-dcl's task in 2m 57s —— View job

Code Review

Gather context
Read key changed files
Analyze for issues
Post inline review comments

Summary

The feature works conceptually but has several issues that must be fixed before merging.

Blocking Issues

#	Location	Problem
1	`UpdatePointAndClickInputSystem.cs:44,48`	System state violation — `lastClickTime` and `markerShouldBeHidden` are behavioral state stored in the system. CLAUDE.md explicitly forbids this: "Systems must not contain state — all state goes into ECS."
2	`UpdatePointAndClickInputSystem.cs:209,224,244`	Structural change after `ref` — `World.Remove<PointAndClickMovementComponent>` is called while `ref PointAndClickMovementComponent pcm` is still in scope. Per CLAUDE.md this is strictly forbidden as it can corrupt the Arch ECS chunk iterator. Entities needing removal must be collected in a temporary list and removed after the query finishes. Fix this →
3	`DestinationMarkerView.cs:65`	Per-frame `r.material` access — calling `r.material.SetColor()` every frame creates material instances that break GPU batching. Should use `MaterialPropertyBlock` instead. Fix this →
4	`CharacterMotionSettings.cs:26`	Bypasses Addressables — `DestinationMarkerPrefab` is a direct `GameObject` reference while `PointAtMarkerPrefab` uses `AssetReferenceGameObject` + async provisioner. This keeps the prefab in memory from startup regardless of use. Either align with the pattern or add a comment explaining why the direct reference is intentional.
5	`StylizedGrass_Indirect.mat`	Unintentional change — already flagged by @daniele-dcl, needs to be reverted.

REVIEW_RESULT: FAIL ❌

claude · 2026-04-03T09:29:49Z

...rer/Assets/DCL/AvatarRendering/AvatarShape/Assets/DestinationMarker/DestinationMarkerView.cs

+            Color emission = Color.HSVToRGB(hue, 1f, 1f) * emissionIntensity;
+
+            foreach (Renderer r in renderers)
+                r.material.SetColor(EMISSION_COLOR, emission);


GC/Batching Issue: r.material instantiates a per-renderer material the first time it's accessed, which prevents GPU instancing/batching for all renderers sharing that material. More critically, calling this inside foreach every frame is the wrong pattern entirely.

Use a MaterialPropertyBlock instead — it lets you set per-object shader properties without breaking batching or creating material instances:

Suggested change

r.material.SetColor(EMISSION_COLOR, emission);

if (_propertyBlock == null) _propertyBlock = new MaterialPropertyBlock();

_propertyBlock.SetColor(EMISSION_COLOR, emission);

foreach (Renderer r in renderers)

r.SetPropertyBlock(_propertyBlock);

Declare private MaterialPropertyBlock _propertyBlock; as a field.

claude · 2026-04-03T09:29:56Z

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

+        private readonly GameObject markerPrefab;
+
+        // Stores the time of the previous left-click to detect double-clicks.
+        private float lastClickTime = float.MinValue;


CLAUDE.md Violation — System State: lastClickTime is behavioral state stored in the system. The project standard explicitly states: "Systems must not contain state — all state goes into ECS."

The same applies to markerShouldBeHidden on line 48. Both should be moved into ECS components (e.g., a PointAndClickInputStateComponent singleton on the player or a dedicated singleton entity). Holding them here also makes the system untestable since UnitySystemTestBase<T> won't be able to inspect or seed this state.

claude · 2026-04-03T09:30:05Z

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

+            if (hasManualMovement)
+            {
+                markerShouldBeHidden = true;
+                World.Remove<PointAndClickMovementComponent>(entity);


ECS Safety Violation: World.Remove is a structural change that triggers an archetype move, relocating entity data in memory. The function has ref PointAndClickMovementComponent pcm in its signature, so pcm is an active ref at this call site. Per CLAUDE.md:

"NEVER perform structural changes (Add/Remove that trigger archetype moves) after obtaining a ref, in, or out reference to a component."

This applies even though you return immediately after — the structural change can invalidate the Arch ECS chunk iterator for the current query, potentially causing skipped or double-visited entities in the same frame.

The same issue exists at lines 224 and 244.

Fix: collect entities that need removal in a temporary list and remove them after the query completes.

claude · 2026-04-03T09:30:13Z

Explorer/Assets/DCL/PluginSystem/Global/CharacterMotionSettings.cs

        [field: SerializeField] internal PointAtMarkerVisibilitySettings PointAtMarkerVisibilitySettings { get; private set; }

+        [field: SerializeField]
+        public GameObject DestinationMarkerPrefab { get; private set; }


Inconsistency with codebase pattern: PointAtMarkerPrefab uses AssetReferenceGameObject and is loaded asynchronously via assetsProvisioner.ProvideMainAssetAsync in InitializeAsync. DestinationMarkerPrefab bypasses this entirely with a direct serialized GameObject reference.

This means the prefab is held in memory from startup regardless of whether the feature is used, and it doesn't participate in the asset lifecycle managed by the provisioner. It should follow the same pattern as PointAtMarkerPrefab — unless the intent is to always have it loaded (e.g. it's in the Essentials group), in which case please add a comment explaining why the direct reference is appropriate here.

AnsisMalins · 2026-04-03T16:04:17Z

PR #7978, run #23909705745

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Intel Core i3

Metric	Change	Baseline	Delta	Improvement
Samples	2652	2673
CPU average	33.9 ms	33.6 ms	0.3 ms	-0.8%	⚪
CPU 1% worst	69.5 ms	54.6 ms	14.9 ms	-27.2%	🔴
CPU 0.1% worst	254.0 ms	142.5 ms	111.4 ms	-78.2%	🔴
GPU average	15.7 ms	16.1 ms	-0.4 ms	2.3%	⚪
GPU 1% worst	23.2 ms	21.1 ms	2.2 ms	-10.3%	🔴
GPU 0.1% worst	29.0 ms	24.0 ms	4.9 ms	-20.5%	🔴

Changes public class attributes to private setters and serializable. Changed double click to use Input System double tap Removed programmatic transition of Start to Idle in favor of Exit Time in the animator controller. Moved destionation marker assets into its own folder Broke System update into smaller logic blocks for readability Added a warning log when Destination Marker is not set in the global configuration Used World.AddOrSet Max raycast distance limited to 250. Restored Grass shader

markerShouldBeHidden removed — replaced with entitiesToCancelNavigation (per-frame temp list, cleared at top of Update) World.Remove<PointAndClickMovementComponent> is now deferred — DriveMovement adds to the list, removals happen after the query when all refs are out of scope DestinationMarkerView uses a single shared MaterialPropertyBlock — one SetColor call, then SetPropertyBlock per renderer, no material instances created DestinationMarkerPrefab is now AssetReferenceGameObject, loaded in InitializeAsync via assetsProvisioner, matching the PointAtMarkerPrefab pattern

olavra · 2026-04-05T20:47:20Z

@claude review

claude · 2026-04-05T20:47:58Z

Claude finished @olavra's task in 2m 0s —— View job

Code Review

Gather context
Read changed files
Analyze for issues
Post review

All five blocking issues from the previous review have been resolved:

#	Issue	Status
1	System state violation (`lastClickTime`, `markerShouldBeHidden`)	✅ Fixed — state removed from system
2	Structural change after `ref` (`World.Remove` while `ref pcm` in scope)	✅ Fixed — deferred via `entitiesToCancelNavigation` list
3	Per-frame `r.material.SetColor()` breaking GPU batching	✅ Fixed — `MaterialPropertyBlock` with `r.SetPropertyBlock`
4	`DestinationMarkerPrefab` bypassing Addressables	✅ Fixed — now `AssetReferenceGameObject` loaded via `assetsProvisioner.ProvideMainAssetAsync`
5	`StylizedGrass_Indirect.mat` unintentional change	✅ Fixed — no longer in the diff

The code is clean and follows codebase conventions. No remaining blocking issues.

REVIEW_RESULT: PASS ✅

Ludmilafantaniella

QA Result: ✅ Approved

Tested on Windows and Mac.

What was tested:

Double clicking moves the avatar towards the target point correctly
Marker appears and disappears after reaching the destination or failing to reach it
Only one marker visible at a time
Movement is cancelled when using directional keys (WASD)
Movement is cancelled when opening menus
Movement is cancelled when the avatar can't reach the destination due to an obstacle
Shift to run, Space to jump and glide work correctly during automatic movement
Multiple rapid double clicks handled correctly
Tested in Genesis City, personal world and scenes with objects
Controls menu shows the new double click input option

All PR test steps and edge cases verified. No regressions found.

7978-evi2.mp4

7978-evi.mp4

✅Sanity check performed:

✔️ Backpack and wearables in world
✔️ Emotes in world and in backpack
✔️ Teleport with map/coordinates/Jump In
✔️ Chat and multiplayer
✔️ Profile card
✔️ Camera
✔️ Skybox
✔️ Settings

alejandro-jimenez-dcl

Approved with a few small requests

alejandro-jimenez-dcl · 2026-04-07T09:44:10Z

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

+        private GameObject targetMarker;
+        private DestinationMarkerView targetMarkerView;


Nitpicking here, I'd use either Destination or Target, but not both

alejandro-jimenez-dcl · 2026-04-07T09:46:22Z

Explorer/Assets/DCL/Character/CharacterMotion/Systems/UpdatePointAndClickInputSystem.cs

+            if (!hasDoubleClick)
+                return;


we could avoid passing this param and place the query call inside the hasDoubleClick check in the Update.

sandrade-dcl

Amazing feature, good work!! I only left a comment, please take a look to it.

sandrade-dcl · 2026-04-07T09:43:01Z

...rer/Assets/DCL/AvatarRendering/AvatarShape/Assets/DestinationMarker/DestinationMarkerView.cs

@@ -0,0 +1,55 @@
+using UnityEngine;
+
+namespace DCL.AvatarRendering.AvatarShape


In my opinion DestinationMarkerView.cs doesn't have nothing to do with avatar rendering, I think we should move this class to the CharacterMotion namespace (in fact you have the rest of logic already there).

olavra added 5 commits March 30, 2026 16:11

Added double click to move behaviour

dccda4d

Added marker and animations

11aa383

Merge branch 'dev' into feat/mouse-move

3203866

Marker improvements and extended locomotion

4b6b7dc

- Added Rainbow Effect on marker - Single instance of the marker - Enabled jump and glide while auto walking - Removed Animator Contoller from the Global Plugin System

Merge branch 'dev' into feat/mouse-move

0f6788d

olavra changed the title ~~Avatar Movement using double click~~ feat: Avatar Movement using double click Mar 31, 2026

olavra changed the title ~~feat: Avatar Movement using double click~~ feat: avatar movement using double click Apr 1, 2026

Merge branch 'dev' into feat/mouse-move

f85f4e6

olavra marked this pull request as ready for review April 1, 2026 13:12

olavra requested review from a team as code owners April 1, 2026 13:12

github-actions bot requested review from DafGreco, daniele-dcl and mikhail-dcl April 1, 2026 13:12

github-actions bot assigned olavra Apr 1, 2026

RominaMarchetti and others added 5 commits April 1, 2026 15:01

Update controls texture

2bbce7a

Merge branch 'dev' into feat/mouse-move

c6c6682

Update Controls.png

4cefa2f

Merge branch 'feat/mouse-move' of https://github.com/decentraland/uni…

252b2d3

…ty-explorer into feat/mouse-move

Merge branch 'dev' into feat/mouse-move

7bf9ae7

dalkia added the clean-build Used to trigger clean build on PR label Apr 1, 2026

Merge branch 'dev' into feat/mouse-move

5c98770

daniele-dcl suggested changes Apr 3, 2026

View reviewed changes

claude bot reviewed Apr 3, 2026

View reviewed changes

olavra added 3 commits April 5, 2026 22:28

Merge branch 'dev' into feat/mouse-move

64e0e5a

olavra requested a review from daniele-dcl April 5, 2026 20:46

olavra and others added 2 commits April 5, 2026 23:42

Merge branch 'dev' into feat/mouse-move

e65d884

Reverted point at marker movement

eb53e5b

Ludmilafantaniella approved these changes Apr 6, 2026

View reviewed changes

Merge branch 'dev' into feat/mouse-move

c16282b

olavra requested review from dalkia and lorenzo-ranciaffi and removed request for daniele-dcl April 6, 2026 20:04

alejandro-jimenez-dcl approved these changes Apr 7, 2026

View reviewed changes

sandrade-dcl requested changes Apr 7, 2026

View reviewed changes

Merge branch 'dev' into feat/mouse-move

215bc03

-                r.material.SetColor(EMISSION_COLOR, emission);
+            if (_propertyBlock == null) _propertyBlock = new MaterialPropertyBlock();
+            _propertyBlock.SetColor(EMISSION_COLOR, emission);
+            foreach (Renderer r in renderers)
+                r.SetPropertyBlock(_propertyBlock);

		private GameObject targetMarker;
		private DestinationMarkerView targetMarkerView;

		@@ -0,0 +1,55 @@
		using UnityEngine;

		namespace DCL.AvatarRendering.AvatarShape

Conversation

olavra commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Description

Test Instructions

Uh oh!

github-actions bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AnsisMalins commented Apr 2, 2026

Intel Core i3

Uh oh!

daniele-dcl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniele-dcl commented Apr 3, 2026

Uh oh!

claude bot commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Summary

Blocking Issues

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

AnsisMalins commented Apr 3, 2026

Intel Core i3

Uh oh!

olavra commented Apr 5, 2026

Uh oh!

claude bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Uh oh!

Ludmilafantaniella left a comment

Choose a reason for hiding this comment

Uh oh!

alejandro-jimenez-dcl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olavra commented Mar 31, 2026 •

edited

Loading

github-actions bot commented Mar 31, 2026 •

edited

Loading

claude bot commented Apr 3, 2026 •

edited

Loading

claude bot commented Apr 5, 2026 •

edited

Loading