Skip to content

Commit 95f0043

Browse files
fix: trigger area was reporting the wrong entity (#7835)
* fix: release CRDT payloads after trigger area handlings and make the pool local to avoid usage outside of the system * fix: replace the pool approach for a new struct that is not reused.
1 parent 99a855c commit 95f0043

File tree

4 files changed

+81
-66
lines changed

4 files changed

+81
-66
lines changed

Explorer/Assets/DCL/Infrastructure/Global/StaticContainer.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,16 @@ public UniTask InitializeAsync(StaticSettings settings, CancellationToken ct)
295295
new AnimatorPlugin(),
296296
new TweenPlugin(),
297297
container.MediaContainer.CreatePlugin(exposedGlobalDataContainer.ExposedCameraData),
298-
new SDKEntityTriggerAreaPlugin(globalWorld, container.MainPlayerAvatarBaseProxy, exposedGlobalDataContainer.ExposedCameraData.CameraEntityProxy, container.CharacterContainer.CharacterObject, componentsContainer.ComponentPoolsRegistry, container.assetsProvisioner, container.CacheCleaner, exposedGlobalDataContainer.ExposedCameraData, container.SceneRestrictionBusController, web3IdentityProvider, componentsContainer.ComponentPoolsRegistry.AddComponentPool<PBTriggerAreaResult.Types.Trigger>()),
298+
new SDKEntityTriggerAreaPlugin(
299+
globalWorld,
300+
container.MainPlayerAvatarBaseProxy,
301+
exposedGlobalDataContainer.ExposedCameraData.CameraEntityProxy,
302+
container.CharacterContainer.CharacterObject,
303+
componentsContainer.ComponentPoolsRegistry,
304+
container.assetsProvisioner,
305+
container.CacheCleaner,
306+
exposedGlobalDataContainer.ExposedCameraData,
307+
container.SceneRestrictionBusController, web3IdentityProvider),
299308
new PointerInputAudioPlugin(container.assetsProvisioner),
300309
new MapPinPlugin(globalWorld, container.MapPinsEventBus),
301310
new MultiplayerPlugin(),

Explorer/Assets/DCL/PluginSystem/World/SDKEntityTriggerAreaPlugin.cs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ public class SDKEntityTriggerAreaPlugin : IDCLWorldPlugin<SDKEntityTriggerAreaSe
4141
private readonly IWeb3IdentityCache web3IdentityCache;
4242

4343
private IComponentPool<SDKEntityTriggerArea.SDKEntityTriggerArea>? sdkEntityTriggerAreaPoolRegistry;
44-
private IComponentPool<PBTriggerAreaResult.Types.Trigger> triggerAreaResultTriggerPool;
4544

4645
public SDKEntityTriggerAreaPlugin(
4746
Arch.Core.World globalWorld,
@@ -53,8 +52,7 @@ public SDKEntityTriggerAreaPlugin(
5352
CacheCleaner cacheCleaner,
5453
IExposedCameraData cameraData,
5554
ISceneRestrictionBusController sceneRestrictionBusController,
56-
IWeb3IdentityCache web3IdentityCache,
57-
IComponentPool<PBTriggerAreaResult.Types.Trigger> triggerAreaResultTriggerPool)
55+
IWeb3IdentityCache web3IdentityCache)
5856
{
5957
this.globalWorld = globalWorld;
6058
this.assetsProvisioner = assetsProvisioner;
@@ -66,7 +64,6 @@ public SDKEntityTriggerAreaPlugin(
6664
this.cameraData = cameraData;
6765
this.sceneRestrictionBusController = sceneRestrictionBusController;
6866
this.web3IdentityCache = web3IdentityCache;
69-
this.triggerAreaResultTriggerPool = triggerAreaResultTriggerPool;
7067
}
7168

7269
public void Dispose()
@@ -91,9 +88,6 @@ public void InjectToWorld(ref ArchSystemsWorldBuilder<Arch.Core.World> builder,
9188
ref builder,
9289
globalWorld,
9390
sharedDependencies.EcsToCRDTWriter,
94-
componentPoolsRegistry.GetReferenceTypePool<PBTriggerAreaResult>(),
95-
triggerAreaResultTriggerPool,
96-
sharedDependencies.SceneStateProvider,
9791
sharedDependencies.EntityCollidersSceneCache,
9892
sharedDependencies.SceneData));
9993
finalizeWorldSystems.Add(SDKEntityTriggerAreaCleanupSystem.InjectToWorld(ref builder, sdkEntityTriggerAreaPoolRegistry!));

Explorer/Assets/DCL/SDKComponents/TriggerArea/Systems/TriggerAreaHandlerSystem.cs

Lines changed: 63 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
using DCL.Diagnostics;
1111
using DCL.ECSComponents;
1212
using DCL.Interaction.Utility;
13-
using DCL.Optimization.Pools;
1413
using DCL.SDKComponents.TriggerArea.Components;
1514
using ECS.Abstract;
1615
using ECS.Groups;
@@ -29,29 +28,57 @@ namespace DCL.SDKComponents.TriggerArea.Systems
2928
[LogCategory(ReportCategory.CHARACTER_TRIGGER_AREA)]
3029
public partial class TriggerAreaHandlerSystem : BaseUnityLoopSystem, IFinalizeWorldSystem
3130
{
31+
/// <summary>
32+
/// Value-type snapshot of all result data passed into the CRDT writer closure,
33+
/// avoiding shared pooled references that could be overwritten before deferred serialization.
34+
/// </summary>
35+
internal readonly struct ResultData
36+
{
37+
public readonly TriggerAreaEventType EventType;
38+
public readonly uint TriggeredEntity;
39+
public readonly uint Timestamp;
40+
public readonly Decentraland.Common.Vector3 TriggeredEntityPosition;
41+
public readonly Decentraland.Common.Quaternion TriggeredEntityRotation;
42+
public readonly uint TriggerEntity;
43+
public readonly uint TriggerLayers;
44+
public readonly Decentraland.Common.Vector3 TriggerEntityPosition;
45+
public readonly Decentraland.Common.Quaternion TriggerEntityRotation;
46+
public readonly Decentraland.Common.Vector3 TriggerEntityScale;
47+
48+
public ResultData(
49+
TriggerAreaEventType eventType, uint triggeredEntity, uint timestamp,
50+
Decentraland.Common.Vector3 triggeredEntityPosition, Decentraland.Common.Quaternion triggeredEntityRotation,
51+
uint triggerEntity, uint triggerLayers,
52+
Decentraland.Common.Vector3 triggerEntityPosition, Decentraland.Common.Quaternion triggerEntityRotation,
53+
Decentraland.Common.Vector3 triggerEntityScale)
54+
{
55+
EventType = eventType;
56+
TriggeredEntity = triggeredEntity;
57+
Timestamp = timestamp;
58+
TriggeredEntityPosition = triggeredEntityPosition;
59+
TriggeredEntityRotation = triggeredEntityRotation;
60+
TriggerEntity = triggerEntity;
61+
TriggerLayers = triggerLayers;
62+
TriggerEntityPosition = triggerEntityPosition;
63+
TriggerEntityRotation = triggerEntityRotation;
64+
TriggerEntityScale = triggerEntityScale;
65+
}
66+
}
67+
3268
private readonly World globalWorld;
3369
private readonly IECSToCRDTWriter ecsToCRDTWriter;
34-
private readonly IComponentPool<PBTriggerAreaResult> triggerAreaResultPool;
35-
private readonly IComponentPool<PBTriggerAreaResult.Types.Trigger> triggerAreaResultTriggerPool;
36-
private readonly ISceneStateProvider sceneStateProvider;
3770
private readonly IEntityCollidersSceneCache collidersSceneCache;
3871
private readonly ISceneData sceneData;
3972

4073
public TriggerAreaHandlerSystem(
4174
World world,
4275
World globalWorld,
4376
IECSToCRDTWriter ecsToCRDTWriter,
44-
IComponentPool<PBTriggerAreaResult> triggerAreaResultPool,
45-
IComponentPool<PBTriggerAreaResult.Types.Trigger> triggerAreaResultTriggerPool,
46-
ISceneStateProvider sceneStateProvider,
4777
IEntityCollidersSceneCache collidersSceneCache,
4878
ISceneData sceneData) : base(world)
4979
{
5080
this.globalWorld = globalWorld;
5181
this.ecsToCRDTWriter = ecsToCRDTWriter;
52-
this.triggerAreaResultPool = triggerAreaResultPool;
53-
this.triggerAreaResultTriggerPool = triggerAreaResultTriggerPool;
54-
this.sceneStateProvider = sceneStateProvider;
5582
this.collidersSceneCache = collidersSceneCache;
5683
this.sceneData = sceneData;
5784
}
@@ -155,41 +182,38 @@ private void PropagateResultComponent(in Entity triggerAreaEntity, in CRDTEntity
155182
triggerEntityScale = characterTransform.Transform.localScale.ToProtoVector();
156183
}
157184

158-
var resultComponent = triggerAreaResultPool.Get();
159-
resultComponent.EventType = eventType;
160-
resultComponent.TriggeredEntity = (uint)triggerAreaCRDTEntity.Id;
161-
resultComponent.Timestamp = incrementalTick;
162-
resultComponent.TriggeredEntityPosition = triggerAreaTransform.localPosition.ToProtoVector();
163-
resultComponent.TriggeredEntityRotation = triggerAreaTransform.localRotation.ToProtoQuaternion();
164-
165-
// 'Trigger' Entity (the entity that provokes the trigger event)
166-
resultComponent.Trigger = triggerAreaResultTriggerPool.Get();
167-
resultComponent.Trigger.Layers = avatarEntity == Entity.Null ? (uint)entityInfo.SDKLayer : (uint)ColliderLayer.ClPlayer;
168-
resultComponent.Trigger.Position = triggerEntityPos;
169-
resultComponent.Trigger.Rotation = triggerEntityRot;
170-
resultComponent.Trigger.Scale = triggerEntityScale;
185+
uint triggerLayers = avatarEntity == Entity.Null ? (uint)entityInfo.SDKLayer : (uint)ColliderLayer.ClPlayer;
171186

187+
uint triggerEntity;
172188
if (avatarEntity != Entity.Null)
173-
resultComponent.Trigger.Entity = globalWorld.TryGet(avatarEntity, out PlayerCRDTEntity playerCrdtEntityComp) ? (uint)playerCrdtEntityComp.CRDTEntity.Id : 999999;
189+
triggerEntity = globalWorld.TryGet(avatarEntity, out PlayerCRDTEntity playerCrdtEntityComp) ? (uint)playerCrdtEntityComp.CRDTEntity.Id : 999999;
174190
else
175-
resultComponent.Trigger.Entity = (uint)entityInfo.SDKEntity.Id;
191+
triggerEntity = (uint)entityInfo.SDKEntity.Id;
176192

177-
ecsToCRDTWriter.AppendMessage<PBTriggerAreaResult, (PBTriggerAreaResult result, uint timestamp)>
193+
var data = new ResultData(
194+
eventType, (uint)triggerAreaCRDTEntity.Id, incrementalTick,
195+
triggerAreaTransform.localPosition.ToProtoVector(), triggerAreaTransform.localRotation.ToProtoQuaternion(),
196+
triggerEntity, triggerLayers, triggerEntityPos, triggerEntityRot, triggerEntityScale);
197+
198+
ecsToCRDTWriter.AppendMessage<PBTriggerAreaResult, ResultData>
178199
(
179200
prepareMessage: static (pbTriggerAreaResult, data) =>
180201
{
181-
pbTriggerAreaResult.EventType = data.result.EventType;
182-
pbTriggerAreaResult.TriggeredEntity = data.result.TriggeredEntity;
183-
pbTriggerAreaResult.Timestamp = data.timestamp;
184-
pbTriggerAreaResult.TriggeredEntityRotation = data.result.TriggeredEntityRotation;
185-
pbTriggerAreaResult.TriggeredEntityPosition = data.result.TriggeredEntityPosition;
186-
pbTriggerAreaResult.Trigger = data.result.Trigger;
202+
pbTriggerAreaResult.EventType = data.EventType;
203+
pbTriggerAreaResult.TriggeredEntity = data.TriggeredEntity;
204+
pbTriggerAreaResult.Timestamp = data.Timestamp;
205+
pbTriggerAreaResult.TriggeredEntityPosition = data.TriggeredEntityPosition;
206+
pbTriggerAreaResult.TriggeredEntityRotation = data.TriggeredEntityRotation;
207+
208+
pbTriggerAreaResult.Trigger ??= new PBTriggerAreaResult.Types.Trigger();
209+
pbTriggerAreaResult.Trigger.Entity = data.TriggerEntity;
210+
pbTriggerAreaResult.Trigger.Layers = data.TriggerLayers;
211+
pbTriggerAreaResult.Trigger.Position = data.TriggerEntityPosition;
212+
pbTriggerAreaResult.Trigger.Rotation = data.TriggerEntityRotation;
213+
pbTriggerAreaResult.Trigger.Scale = data.TriggerEntityScale;
187214
},
188-
triggerAreaCRDTEntity, (int)incrementalTick, (resultComponent, incrementalTick)
215+
triggerAreaCRDTEntity, (int)incrementalTick, data
189216
);
190-
191-
triggerAreaResultTriggerPool.Release(resultComponent.Trigger);
192-
triggerAreaResultPool.Release(resultComponent);
193217
}
194218

195219
[Query]
@@ -220,6 +244,8 @@ public void FinalizeComponents(in Query query)
220244
FinalizeComponentsQuery(World);
221245
}
222246

247+
protected override void OnDispose() { }
248+
223249
private bool TryGetAvatarEntity(Transform transform, out Entity entity)
224250
{
225251
entity = Entity.Null;
@@ -229,7 +255,4 @@ private bool TryGetAvatarEntity(Transform transform, out Entity entity)
229255
return true;
230256
}
231257
}
232-
}
233-
234-
235-
258+
}

Explorer/Assets/DCL/SDKComponents/TriggerArea/Tests/TriggerAreaHandlerSystemShould.cs

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using DCL.SDKComponents.TriggerArea.Systems;
77
using DCL.SDKEntityTriggerArea.Components;
88
using DCL.Interaction.Utility;
9-
using DCL.Optimization.Pools;
109
using DCL.SDKComponents.TriggerArea.Components;
1110
using ECS.Prioritization.Components;
1211
using ECS.TestSuite;
@@ -22,9 +21,6 @@ public class TriggerAreaHandlerSystemShould : UnitySystemTestBase<TriggerAreaHan
2221
private World globalWorld;
2322
private IECSToCRDTWriter ecsToCRDTWriter;
2423
private IEntityCollidersSceneCache collidersSceneCache;
25-
private ISceneStateProvider sceneStateProvider;
26-
private IComponentPool<PBTriggerAreaResult> triggerAreaResultPool;
27-
private IComponentPool<PBTriggerAreaResult.Types.Trigger> triggerAreaResultTriggerPool;
2824
private ISceneData sceneData;
2925
private PBTriggerAreaResult capturedResult;
3026
private Entity entity;
@@ -37,14 +33,6 @@ public void Setup()
3733

3834
ecsToCRDTWriter = Substitute.For<IECSToCRDTWriter>();
3935
collidersSceneCache = Substitute.For<IEntityCollidersSceneCache>();
40-
sceneStateProvider = Substitute.For<ISceneStateProvider>();
41-
sceneStateProvider.TickNumber.Returns(123u);
42-
43-
triggerAreaResultPool = Substitute.For<IComponentPool<PBTriggerAreaResult>>();
44-
triggerAreaResultPool.Get().Returns(_ => new PBTriggerAreaResult());
45-
46-
triggerAreaResultTriggerPool = Substitute.For<IComponentPool<PBTriggerAreaResult.Types.Trigger>>();
47-
triggerAreaResultTriggerPool.Get().Returns(_ => new PBTriggerAreaResult.Types.Trigger());
4836

4937
sceneData = Substitute.For<ISceneData>();
5038
sceneData.SceneLoadingConcluded.Returns(true);
@@ -53,9 +41,6 @@ public void Setup()
5341
world,
5442
globalWorld,
5543
ecsToCRDTWriter,
56-
triggerAreaResultPool,
57-
triggerAreaResultTriggerPool,
58-
sceneStateProvider,
5944
collidersSceneCache,
6045
sceneData);
6146

@@ -244,18 +229,22 @@ private SDKEntityTriggerArea.SDKEntityTriggerArea CreateAndAttachAreaMonoBehavio
244229
}
245230

246231
private void ClearCapturedResult() => capturedResult = null;
232+
247233
private void SetupCRDTWriterCapture(bool firstCallOnly = true)
248234
{
249235
ecsToCRDTWriter
250-
.AppendMessage<PBTriggerAreaResult, (PBTriggerAreaResult result, uint timestamp)>(Arg.Any<System.Action<PBTriggerAreaResult, (PBTriggerAreaResult result, uint timestamp)>>(), Arg.Any<CRDTEntity>(), Arg.Any<int>(), Arg.Any<(PBTriggerAreaResult, uint)>())
236+
.AppendMessage<PBTriggerAreaResult, TriggerAreaHandlerSystem.ResultData>(
237+
Arg.Any<System.Action<PBTriggerAreaResult, TriggerAreaHandlerSystem.ResultData>>(),
238+
Arg.Any<CRDTEntity>(), Arg.Any<int>(),
239+
Arg.Any<TriggerAreaHandlerSystem.ResultData>())
251240
.Returns(ci =>
252241
{
253-
var prepare = ci.Arg<System.Action<PBTriggerAreaResult, (PBTriggerAreaResult, uint)>>();
242+
var prepare = ci.Arg<System.Action<PBTriggerAreaResult, TriggerAreaHandlerSystem.ResultData>>();
254243
var res = new PBTriggerAreaResult();
255244

256245
if (firstCallOnly && capturedResult != null) return res;
257246

258-
var data = ci.ArgAt<(PBTriggerAreaResult, uint)>(3);
247+
var data = ci.ArgAt<TriggerAreaHandlerSystem.ResultData>(3);
259248
prepare(res, data);
260249
capturedResult = res;
261250
return res;

0 commit comments

Comments
 (0)