Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2026 The Terasology Foundation
// SPDX-License-Identifier: Apache-2.0
package org.terasology.engine.logic.characters;

import org.joml.Quaternionf;
import org.joml.Vector3f;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.terasology.engine.entitySystem.entity.EntityRef;
import org.terasology.engine.logic.location.LocationComponent;
import org.terasology.engine.physics.engine.CharacterCollider;
import org.terasology.engine.physics.engine.PhysicsEngine;

import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;

/**
* Regression test for #4969: an entity mid-death can lose {@link CharacterMovementComponent} (or {@link
* LocationComponent}) between being queued for a state update and that update actually running - see
* {@link ServerCharacterPredictionSystem#onDestroy}. {@link CharacterMovementSystemUtility#setToExtrapolateState}
* used to assume both components were always present and NPE'd the moment they weren't.
*/
public class CharacterMovementSystemUtilityTest {
private PhysicsEngine physics;
private CharacterMovementSystemUtility utility;
private CharacterStateEvent state;

@BeforeEach
public void setup() {
physics = Mockito.mock(PhysicsEngine.class);
utility = new CharacterMovementSystemUtility(physics);
state = new CharacterStateEvent(0, 1, new Vector3f(), new Quaternionf(), new Vector3f(),
0, 0, MovementMode.WALKING, true);
}

@Test
public void setToExtrapolateStateSkipsEntityMissingCharacterMovementComponent() {
EntityRef entity = Mockito.mock(EntityRef.class);
Mockito.when(entity.getComponent(LocationComponent.class)).thenReturn(new LocationComponent());
Mockito.when(entity.getComponent(CharacterMovementComponent.class)).thenReturn(null);

// Must not throw - this is the exact NPE from #4969's traceback.
utility.setToExtrapolateState(entity, state, 100);

verify(entity, never()).saveComponent(Mockito.any());
verifyNoInteractions(physics);
}

@Test
public void setToExtrapolateStateSkipsEntityMissingLocationComponent() {
EntityRef entity = Mockito.mock(EntityRef.class);
Mockito.when(entity.getComponent(LocationComponent.class)).thenReturn(null);
Mockito.when(entity.getComponent(CharacterMovementComponent.class)).thenReturn(new CharacterMovementComponent());

utility.setToExtrapolateState(entity, state, 100);

verify(entity, never()).saveComponent(Mockito.any());
verifyNoInteractions(physics);
}

@Test
public void setToExtrapolateStateUpdatesEntityWithBothComponents() {
EntityRef entity = Mockito.mock(EntityRef.class);
Mockito.when(entity.getComponent(LocationComponent.class)).thenReturn(new LocationComponent());
Mockito.when(entity.getComponent(CharacterMovementComponent.class)).thenReturn(new CharacterMovementComponent());
CharacterCollider collider = Mockito.mock(CharacterCollider.class);
Mockito.when(physics.getCharacterCollider(entity)).thenReturn(collider);

utility.setToExtrapolateState(entity, state, 100);

verify(entity).saveComponent(Mockito.isA(LocationComponent.class));
verify(entity).saveComponent(Mockito.isA(CharacterMovementComponent.class));
verify(collider).setLocation(Mockito.any());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@ public void setToInterpolateState(EntityRef entity, CharacterStateEvent a, Chara
}

public void setToExtrapolateState(EntityRef entity, CharacterStateEvent state, long time) {
// Mirrors setToState's own null-checks above: an entity mid-death can lose
// CharacterMovementComponent (or LocationComponent) between being queued for a state update
// and this actually running - see ServerCharacterPredictionSystem#onDestroy. Extrapolating a
// now-nonexistent state is meaningless, not an error; skip it like setToState does.
if (entity.getComponent(LocationComponent.class) == null
|| entity.getComponent(CharacterMovementComponent.class) == null) {
return;
}

float t = (time - state.getTime()) * 0.0001f;
Vector3f newPos = new Vector3f(state.getVelocity());
newPos.mul(t);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ public void onCreate(final OnActivatedComponent event, final EntityRef entity) {
AliveCharacterComponent.class})
public void onDestroy(final BeforeDeactivateComponent event, final EntityRef entity) {
physics.removeCharacterCollider(entity);
// Queued rather than removed immediately: this fires whenever any of the three required
// components is dropped (e.g. a dying entity losing AliveCharacterComponent mid-animation,
// before the rest of it is torn down), so characterStates can still hold this entity - with
// some of its components already gone - until update() batches the actual removal below.
// lagCompensate()/restoreToPresent() skip anything in this queue for the same reason.
characterStatesToRemove.add(entity);
lastInputEvent.remove(entity);
}
Expand Down Expand Up @@ -260,6 +265,14 @@ private void setToTime(long renderTime, EntityRef entity, CircularBuffer<Charact
@Override
public void lagCompensate(EntityRef client, long timeMs) {
for (Map.Entry<EntityRef, CircularBuffer<CharacterStateEvent>> entry : characterStates.entrySet()) {
// onDestroy queues the entity here rather than removing it immediately (see its own
// comment); until the next update() drains that queue, characterStates can still hold an
// entity whose CharacterMovementComponent (or LocationComponent) is already gone - e.g.
// a mob that died mid-tick but is still finishing its death animation. Skipping it here
// matches what update()'s own replication loop already does for the same reason.
if (characterStatesToRemove.contains(entry.getKey())) {
continue;
}
if (networkSystem.getOwnerEntity(entry.getKey()).equals(client)) {
characterMovementSystemUtility.setToState(entry.getKey(), entry.getValue().getLast());
} else {
Expand All @@ -272,6 +285,11 @@ public void lagCompensate(EntityRef client, long timeMs) {
public void restoreToPresent() {
long renderTime = time.getGameTimeInMs() - RENDER_DELAY;
for (Map.Entry<EntityRef, CircularBuffer<CharacterStateEvent>> entry : characterStates.entrySet()) {
// See the matching comment in lagCompensate(): entries pending removal can already be
// missing the components setToTime -> setToExtrapolateState needs.
if (characterStatesToRemove.contains(entry.getKey())) {
continue;
}
setToTime(renderTime, entry.getKey(), entry.getValue());
}
}
Expand Down
Loading