Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
6 changes: 3 additions & 3 deletions .github/workflows/pr-comment-artifact-url.yml
Comment thread
popuz marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ jobs:

Build skipped — no changes detected under `Explorer/`.

[badge]: https://img.shields.io/badge/Build-Skipped-yellow?logo=github&style=for-the-badge
[badge]: https://img.shields.io/badge/Build-Skipped-yellow?logo=unity&logoColor=white&style=for-the-badge

comment-success:
needs: [pre-validation, check-build-ran]
Expand Down Expand Up @@ -231,7 +231,7 @@ jobs:

${{ env.SIZE_REPORT }}

[badge]: https://img.shields.io/badge/Build-Success!-3fb950?logo=github&style=for-the-badge
[badge]: https://img.shields.io/badge/Build-Success!-3fb950?logo=unity&logoColor=white&style=for-the-badge

- name: Find latest release
env:
Expand Down Expand Up @@ -278,4 +278,4 @@ jobs:
Build failed! Check the logs to see what went wrong.
If the error repeats please consider the `clean-build` tag.

[badge]: https://img.shields.io/badge/Build-Failed!-ff0000?logo=github&style=for-the-badge
[badge]: https://img.shields.io/badge/Build-Failed!-ff0000?logo=unity&logoColor=white&style=for-the-badge
99 changes: 72 additions & 27 deletions .github/workflows/pr-comment-test-failures.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ on:
permissions:
contents: read
pull-requests: write
actions: read

jobs:
comment:
Expand Down Expand Up @@ -60,43 +61,87 @@ jobs:
- name: Compose comment body
if: steps.pr.outputs.pr-number != '' && (steps.download-editmode.outcome == 'success' || steps.download-playmode.outcome == 'success')
id: body
env:
GITHUB_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
WORKFLOW_RUN_ID: ${{ github.event.workflow_run.id }}
WORKFLOW_RUN_URL: ${{ github.event.workflow_run.html_url }}
run: |
set -euo pipefail

# Job list of the originating "Unity Test" run, used to deep-link a
# crashed/timed-out suite straight to its job page in the warning line.
JOBS_JSON=$(gh api "/repos/$REPO/actions/runs/$WORKFLOW_RUN_ID/jobs" 2>/dev/null || echo '{"jobs":[]}')

declare -A DISPLAY=( [editmode]=EditMode [playmode]=PlayMode )

status=passed # passed | failed | incomplete
rows=""
warnings=""
failed_list=""
total_failed=0

for mode in editmode playmode; do
file="failed-tests/$mode/failed-tests-$mode.json"
[ -f "$file" ] || continue
disp=${DISPLAY[$mode]}

# A suite that produced no result XML crashed or timed out before finishing.
# Surface it as its own state instead of silently contributing 0 to a green total.
if [ "$(jq -r '.hasResults' "$file")" != "true" ]; then
status=incomplete
job_url=$(jq -r --arg n "Test ($mode)" '.jobs[]? | select(.name==$n) | .html_url' <<< "$JOBS_JSON" | head -1)
[ -n "$job_url" ] || job_url="$WORKFLOW_RUN_URL"
rows="$rows| $disp | ⚠️ No results | — | — | — |"$'\n'
warnings="$warnings⚠️ **$disp** produced no results — the run likely crashed or timed out before finishing. Check the [\`Unity Test / Test ($mode)\`]($job_url) job."$'\n\n'
continue
fi

# The artifact comes from the untrusted pull_request job - never let a
# non-numeric value reach the arithmetic context.
p=$(jq -r '.passed' "$file"); [[ "$p" =~ ^[0-9]+$ ]] || p=0
t=$(jq -r '.total' "$file"); [[ "$t" =~ ^[0-9]+$ ]] || t=0
f=$(jq -r '.failed | length' "$file")
s=$((t - p - f)); if [ "$s" -lt 0 ]; then s=0; fi

if [ "$f" -gt 0 ]; then
if [ "$status" = "passed" ]; then status=failed; fi
total_failed=$((total_failed + f))
rows="$rows| $disp | ❌ $f failed | $p | $f | $s |"$'\n'
names=$(jq -r --arg mode "$mode" '.failed[] | "- [\($mode)] \(. | gsub("[\r\n]"; " "))"' "$file")
failed_list="$failed_list$names"$'\n'
else
rows="$rows| $disp | ✅ Passed | $p | 0 | $s |"$'\n'
fi
done

case "$status" in
incomplete) badge="https://img.shields.io/badge/Tests-Incomplete-d29922?logo=codecov&logoColor=white&style=for-the-badge"; headline="$warnings" ;;
failed) badge="https://img.shields.io/badge/Tests-Failed!-ff0000?logo=codecov&logoColor=white&style=for-the-badge"; headline="Some Unity tests failed ❌" ;;
*) badge="https://img.shields.io/badge/Tests-Passed!-3fb950?logo=codecov&logoColor=white&style=for-the-badge"; headline="All Unity tests passed ✅" ;;
esac

DELIM="EOF_$(uuidgen)"
{
echo "body<<$DELIM"
echo "<!-- test-failures -->"

passed=0
failed=0
any_results=false
for mode in editmode playmode; do
file="failed-tests/$mode/failed-tests-$mode.json"
[ -f "$file" ] || continue
[ "$(jq -r '.hasResults' "$file")" = "true" ] && any_results=true
# The artifact comes from the untrusted pull_request job - never let a
# non-numeric value reach the arithmetic context.
p=$(jq -r '.passed' "$file"); [[ "$p" =~ ^[0-9]+$ ]] || p=0
passed=$((passed + p))
failed=$((failed + $(jq -r '.failed | length' "$file")))
done

if [ "$any_results" = "false" ]; then
echo "**Test results not found** — the test run likely crashed or timed out before producing results; check the \`Unity Tests\` checks."
elif [ "$failed" -eq 0 ]; then
echo "**Tests: $passed passed, 0 failed** ✅"
else
echo "**Tests: $passed passed, $failed failed**"
echo "![badge]"
echo ""
printf '%s\n' "$headline"
echo ""
echo "| TESTS SUITE | Result | Passed | Failed | Skipped |"
echo "| ----------- | ------ | -----: | -----: | ------: |"
printf '%s' "$rows"
if [ "$total_failed" -gt 0 ]; then
echo ""
echo "<details><summary>Failed tests ($failed)</summary>"
echo "<details><summary>Failed tests ($total_failed)</summary>"
echo ""
for mode in editmode playmode; do
file="failed-tests/$mode/failed-tests-$mode.json"
[ -f "$file" ] || continue
jq -r --arg mode "$mode" '.failed[] | "- [\($mode)] \(. | gsub("[\r\n]"; " "))"' "$file"
done
printf '%s' "$failed_list"
echo ""
echo "</details>"
fi
echo ""
echo "[badge]: $badge"
echo "$DELIM"
} >> "$GITHUB_OUTPUT"

Expand Down
9 changes: 4 additions & 5 deletions .github/workflows/pr-comment-warnings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ jobs:
edit-mode: replace
body: |-
<!-- warning-ratchet -->
![badge] <img src="https://ui.decentraland.org/decentraland_256x256.png" width="30">
![badge]

Lint in progress, come back later!

[badge]: https://img.shields.io/badge/Lint-Pending!-ffff00?logo=github&style=for-the-badge
[badge]: https://img.shields.io/badge/Lint-Pending!-ffff00?logo=jetbrains&logoColor=white&style=for-the-badge

comment:
if: github.event.action == 'completed'
Expand Down Expand Up @@ -126,8 +126,7 @@ jobs:
RUN_URL: ${{ github.event.workflow_run.html_url }}
run: |
DELIM="EOF_$(uuidgen)"
LOGO='<img src="https://ui.decentraland.org/decentraland_256x256.png" width="30">'
BADGE_STYLE="?logo=github&style=for-the-badge"
BADGE_STYLE="?logo=jetbrains&logoColor=white&style=for-the-badge"
DETAILS=""

if [ "$FOUND" != "true" ]; then
Expand Down Expand Up @@ -181,7 +180,7 @@ jobs:
{
echo "body<<$DELIM"
echo "<!-- warning-ratchet -->"
echo "![badge] $LOGO"
echo "![badge]"
echo ""
echo "$MSG"
[ -n "$DETAILS" ] && cat "$DETAILS"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,14 @@ public struct JumpState

public readonly bool IsCoyoteTimeActive(int currentTick, int coyoteTimeTickCount) =>
currentTick - LastGroundedTick < coyoteTimeTickCount;

/// <summary>
/// Restores the jump budget: clears the jump counter and cancels any pending air jump.
/// </summary>
public void ResetJumps()
{
JumpCount = 0;
AirJumpDelay = float.MinValue;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private void ResolveAvatarVelocity([Data] float dt,
ApplyGliding.Execute(settings, in rigidTransform, jumpState, in jumpInput, ref glideState, physicsTick, dt);

// External impulses must run after gravity so it nullify gravity velocity.y
ApplyExternalImpulse.Execute(settings, ref rigidTransform);
ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

// Drag
ApplyHorizontalAirDrag.Execute(settings, ref rigidTransform, dt);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
using DCL.CharacterMotion.Components;
using DCL.CharacterMotion.Settings;
using NSubstitute;
using NUnit.Framework;
using UnityEngine;

namespace DCL.Character.CharacterMotion.Tests
{
public class ApplyExternalImpulseShould
{
private ICharacterControllerSettings settings = null!;

[SetUp]
public void SetUp()
{
settings = Substitute.For<ICharacterControllerSettings>();
settings.CharacterMass.Returns(1f);
}

[Test]
public void ResetJumpCountOnUpwardImpulseWhileGrounded()
{
var rigidTransform = new CharacterRigidTransform
{
IsGrounded = true,
ExternalImpulse = Vector3.up * 10f,
};

var jumpState = new JumpState { JumpCount = 2, AirJumpDelay = 0.1f };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(0, jumpState.JumpCount, "An upward impulse launching a grounded character restores jumps");
Assert.AreEqual(float.MinValue, jumpState.AirJumpDelay, "A pending air jump is cancelled by the launch");
Assert.IsFalse(rigidTransform.IsGrounded, "The launch ungrounds the character");
}

[Test]
public void ResetJumpCountOnUpwardImpulseNearGround()
{
// The scene may fire the pad before physics registers a grounded tick, while the character is still
// within centimeters of the ground.
var rigidTransform = new CharacterRigidTransform
{
IsGrounded = false,
GroundDistance = 0.3f,
ExternalImpulse = Vector3.up * 10f,
};

var jumpState = new JumpState { JumpCount = 2 };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(0, jumpState.JumpCount, "An upward impulse close to the ground counts as a landing");
}

[Test]
public void KeepJumpCountOnUpwardImpulseAwayFromGround()
{
var rigidTransform = new CharacterRigidTransform
{
IsGrounded = false,
GroundDistance = 10f,
ExternalImpulse = Vector3.up * 10f,
};

var jumpState = new JumpState { JumpCount = 2 };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(2, jumpState.JumpCount, "An upward impulse clear of the ground is not a landing and does not restore jumps");
}

[Test]
public void ZeroFallingVelocityOnUpwardImpulse()
{
var rigidTransform = new CharacterRigidTransform
{
IsGrounded = false,
GroundDistance = 10f,
ExternalImpulse = Vector3.up * 10f,
GravityVelocity = new Vector3(0f, -20f, 0f),
};

var jumpState = new JumpState { JumpCount = 2 };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(0f, rigidTransform.GravityVelocity.y, "A downward fall must not fight the launch impulse");
}

[Test]
public void ApplyImpulseToExternalVelocityScaledByMass()
{
settings.CharacterMass.Returns(2f);

var rigidTransform = new CharacterRigidTransform
{
IsGrounded = true,
ExternalImpulse = new Vector3(0f, 10f, 0f),
};

var jumpState = new JumpState { JumpCount = 2 };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(5f, rigidTransform.ExternalVelocity.y, "Δv = J / m, so a mass of 2 halves the impulse velocity");
Assert.AreEqual(Vector3.zero, rigidTransform.ExternalImpulse, "The impulse is consumed after being applied");
}

[Test]
public void KeepStateOnDownwardImpulse()
{
var rigidTransform = new CharacterRigidTransform
{
IsGrounded = true,
ExternalImpulse = new Vector3(0f, -10f, 0f),
};

var jumpState = new JumpState { JumpCount = 2 };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(-10f, rigidTransform.ExternalVelocity.y, "A downward impulse is still applied to the velocity");
Assert.AreEqual(2, jumpState.JumpCount, "A downward impulse is not a launch and does not restore jumps");
Assert.IsTrue(rigidTransform.IsGrounded, "A downward impulse does not unground the character");
}

[Test]
public void KeepJumpCountOnHorizontalImpulse()
{
var rigidTransform = new CharacterRigidTransform
{
IsGrounded = true,
ExternalImpulse = Vector3.right * 10f,
};

var jumpState = new JumpState { JumpCount = 2 };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(2, jumpState.JumpCount, "Only an upward impulse counts as a launch");
Assert.IsTrue(rigidTransform.IsGrounded, "A horizontal impulse does not unground the character");
}

[Test]
public void DoNothingOnNegligibleImpulse()
{
var rigidTransform = new CharacterRigidTransform
{
IsGrounded = false,
ExternalImpulse = Vector3.zero,
};

var jumpState = new JumpState { JumpCount = 2 };

ApplyExternalImpulse.Execute(settings, ref rigidTransform, ref jumpState);

Assert.AreEqual(2, jumpState.JumpCount, "A negligible impulse leaves the jump state untouched");
Assert.AreEqual(Vector3.zero, rigidTransform.ExternalVelocity, "A negligible impulse adds no velocity");
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@ namespace DCL.Character.CharacterMotion
{
public static class ApplyExternalImpulse
{
// Ground proximity within which an upward impulse still counts as a landing:
// the scene can launch the character before our physics registers a grounded tick
private const float JUMP_RESET_GROUND_DISTANCE = 1f;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void Execute(ICharacterControllerSettings settings, ref CharacterRigidTransform characterPhysics)
public static void Execute(ICharacterControllerSettings settings, ref CharacterRigidTransform characterPhysics, ref JumpState jumpState)
{
if (characterPhysics.ExternalImpulse.sqrMagnitude < float.Epsilon)
{
Expand All @@ -21,6 +25,9 @@ public static void Execute(ICharacterControllerSettings settings, ref CharacterR

if (characterPhysics.ExternalImpulse.y > 0f)
{
if (characterPhysics.IsGrounded || characterPhysics.GroundDistance <= JUMP_RESET_GROUND_DISTANCE)
jumpState.ResetJumps();

characterPhysics.IsGrounded = false;

// fix for jump pads - so that impulse can win (note: gravity velocity can be positive by jump)
Expand Down
Loading
Loading