Skip to content
Merged

m #139

Show file tree
Hide file tree
Changes from 3 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
34 changes: 31 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
exit $exit_code

- name: InspectCode
uses: JetBrains/ReSharper-InspectCode@v0.11
uses: JetBrains/ReSharper-InspectCode@v0.12
Comment on lines 52 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the v0.12 tag exists for JetBrains/ReSharper-InspectCode
gh api repos/JetBrains/ReSharper-InspectCode/git/refs/tags/v0.12 --jq '.ref' 2>/dev/null || echo "Tag v0.12 not found"

Repository: winnerspiros/osu

Length of output: 191


🌐 Web query:

JetBrains ReSharper InspectCode GitHub Action v0.12 release

💡 Result:

No evidence found of a JetBrains ReSharper InspectCode GitHub Action v0.12 release. The official GitHub repository https://github.com/JetBrains/ReSharper-InspectCode shows the latest release as v0.11 on 2025-10-09. Usage examples and Marketplace listings also reference v0.11 as the current version. As of 2026-04-02, v0.12 has not been released.

Citations:


The action version v0.12 does not exist; use v0.11 instead.

JetBrains/ReSharper-InspectCode@v0.12 is not available. The latest version is v0.11 (released 2025-10-09). Update to uses: JetBrains/ReSharper-InspectCode@v0.11 to fix the workflow failure.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 52 - 53, The workflow step using
JetBrains/ReSharper-InspectCode references a non-existent tag v0.12; update the
step that currently shows uses: JetBrains/ReSharper-InspectCode@v0.12 to use the
valid tag JetBrains/ReSharper-InspectCode@v0.11 so the CI action resolves
correctly.

with:
# this is WTF tier but if you don't specify *both* of these the defaults assume `build: true`
build: false
Expand Down Expand Up @@ -101,14 +101,42 @@ jobs:
NUnit.ConsoleOut=0

# Attempt to upload results even if test fails.
# https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#always
# https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#cancelled
- name: Upload Test Results
uses: actions/upload-artifact@v7
if: ${{ always() }}
if: ${{ !cancelled() }}
with:
name: osu-test-results-${{matrix.os.prettyname}}-${{matrix.threadingMode}}
path: ${{github.workspace}}/TestResults/TestResults-${{matrix.os.prettyname}}-${{matrix.threadingMode}}.trx

test-results:
name: Test results
runs-on: ubuntu-latest
# we want to wait for the `test` job to complete, but run regardless of whether it succeeds or fails
# https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#example-not-requiring-successful-dependent-jobs
if: ${{ !cancelled() }}
needs: test
timeout-minutes: 5
steps:
- name: Checkout
uses: actions/checkout@v6

- name: Download results
uses: actions/download-artifact@v8
with:
pattern: osu-test-results-*
merge-multiple: true

- name: Add test results summary to workflow run
uses: dorny/test-reporter@v3.0.0
with:
name: Results
path: "*.trx"
reporter: dotnet-trx
list-suites: 'failed'
list-tests: 'failed'
use-actions-summary: 'true'
Comment on lines +112 to +138

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing checks: write permission for dorny/test-reporter.

The dorny/test-reporter action requires checks: write permission to create check runs and publish test results. The current workflow-level permissions (lines 7-9) only include contents: read and security-events: write. Without this permission, the action will fail or be unable to post results.

🔧 Proposed fix to add the required permission
 permissions:
   contents: read # to fetch code (actions/checkout)
   security-events: write # for reporting InspectCode issues
+  checks: write # for reporting test results (dorny/test-reporter)

Alternatively, add job-level permissions if you prefer to scope it to only the test-results job:

   test-results:
     name: Test results
     runs-on: ubuntu-latest
+    permissions:
+      checks: write
     # we want to wait for the `test` job to complete, but run regardless of whether it succeeds or fails
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test-results:
name: Test results
runs-on: ubuntu-latest
# we want to wait for the `test` job to complete, but run regardless of whether it succeeds or fails
# https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#example-not-requiring-successful-dependent-jobs
if: ${{ !cancelled() }}
needs: test
timeout-minutes: 5
steps:
- name: Checkout
uses: actions/checkout@v6
- name: Download results
uses: actions/download-artifact@v8
with:
pattern: osu-test-results-*
merge-multiple: true
- name: Add test results summary to workflow run
uses: dorny/test-reporter@v3.0.0
with:
name: Results
path: "*.trx"
reporter: dotnet-trx
list-suites: 'failed'
list-tests: 'failed'
use-actions-summary: 'true'
test-results:
name: Test results
runs-on: ubuntu-latest
permissions:
checks: write
# we want to wait for the `test` job to complete, but run regardless of whether it succeeds or fails
# https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#example-not-requiring-successful-dependent-jobs
if: ${{ !cancelled() }}
needs: test
timeout-minutes: 5
steps:
- name: Checkout
uses: actions/checkout@v6
- name: Download results
uses: actions/download-artifact@v8
with:
pattern: osu-test-results-*
merge-multiple: true
- name: Add test results summary to workflow run
uses: dorny/test-reporter@v3.0.0
with:
name: Results
path: "*.trx"
reporter: dotnet-trx
list-suites: 'failed'
list-tests: 'failed'
use-actions-summary: 'true'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 112 - 138, The test-reporter action in
the "test-results" job (uses: dorny/test-reporter@v3.0.0) needs the GitHub
Checks API write permission; update the workflow permissions to include checks:
write either at the top-level workflow permissions block or as job-level
permissions for the "test-results" job so the action can create check runs and
publish results. Ensure the permissions block includes "checks: write" alongside
existing permissions and remove any conflicting scopes.


build-only-android:
name: Build only (Android)
runs-on: windows-latest
Expand Down
45 changes: 0 additions & 45 deletions .github/workflows/report-nunit.yml

This file was deleted.

1 change: 1 addition & 0 deletions osu.Game.Rulesets.Catch/CatchRuleset.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ public override IEnumerable<Mod> GetModsFor(ModType type)
new CatchModMuted(),
new CatchModNoScope(),
new CatchModMovingFast(),
new CatchModSynesthesia(),
};
Comment on lines 155 to 159

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new fun mod is being added to the catch mod list, but there doesn't appear to be a corresponding mod test scene (there are existing ones under osu.Game.Rulesets.Catch.Tests/Mods/ for other catch mods). Adding a basic ModTestScene for CatchModSynesthesia would help prevent regressions (at minimum verifying fruits get recoloured and bananas are excluded).

Copilot uses AI. Check for mistakes.

case ModType.System:
Expand Down
60 changes: 60 additions & 0 deletions osu.Game.Rulesets.Catch/Mods/CatchModSynesthesia.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Game.Beatmaps;
using osu.Game.Graphics;
using osu.Game.Rulesets.Catch.Objects;
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Screens.Edit;
using osuTK.Graphics;

namespace osu.Game.Rulesets.Catch.Mods
{
/// <summary>
/// Mod that colours <see cref="HitObject"/>s based on the musical division they are on
/// </summary>
public class CatchModSynesthesia : ModSynesthesia, IApplicableToBeatmap, IApplicableToDrawableHitObject
{
private readonly OsuColour colours = new OsuColour();

private IBeatmap? currentBeatmap { get; set; }

public void ApplyToBeatmap(IBeatmap beatmap)
{
//Store a reference to the current beatmap to look up the beat divisor when notes are drawn
if (currentBeatmap != beatmap)
currentBeatmap = beatmap;
}

public void ApplyToDrawableHitObject(DrawableHitObject d)
{
if (currentBeatmap == null) return;

Color4? timingBasedColour = null;

d.HitObjectApplied += _ =>
{
// Block bananas from getting coloured.
if (d.HitObject is not Banana)
{
timingBasedColour = BindableBeatDivisor.GetColourFor(currentBeatmap.ControlPointInfo.GetClosestBeatDivisor(d.HitObject.StartTime), colours);
}

// Colour droplets into a solid colour, as droplets aren't generated snapped to timeline ticks.
if (d.HitObject is Droplet)
{
timingBasedColour = Color4.LightGreen;
}
};

// Need to set this every update to ensure it doesn't get overwritten by DrawableHitObject.OnApply() -> UpdateComboColour().
d.OnUpdate += _ =>
{
if (timingBasedColour != null)
d.AccentColour.Value = timingBasedColour.Value;
};
Comment on lines +52 to +57

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d.OnUpdate is used to reapply AccentColour every frame for every drawable hit object. This can add per-frame overhead on maps with many objects and will also spam bindable change notifications. Consider reapplying only when needed (e.g. schedule a one-time apply after DrawableHitObject.Apply() finishes, and/or listen to d.AccentColour.BindValueChanged and restore the synesthesia colour only when it gets changed by UpdateComboColour()/skinning).

Copilot uses AI. Check for mistakes.
}
}
}
Loading