From 0bcb99f2c46cf08e0c05e1cd27a0662a4a1e375b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 30 Sep 2023 01:16:56 +0300 Subject: [PATCH 01/14] Crop oversized gameplay textures instead of downscaling them --- osu.Game/Skinning/LegacySkinExtensions.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game/Skinning/LegacySkinExtensions.cs b/osu.Game/Skinning/LegacySkinExtensions.cs index dde6c1fa29..868f36fb34 100644 --- a/osu.Game/Skinning/LegacySkinExtensions.cs +++ b/osu.Game/Skinning/LegacySkinExtensions.cs @@ -9,6 +9,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Animations; +using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; using osuTK; @@ -112,9 +113,11 @@ namespace osu.Game.Skinning if (texture.DisplayWidth <= maxSize.X && texture.DisplayHeight <= maxSize.Y) return texture; - // use scale adjust property for downscaling the texture in order to meet the specified maximum dimensions. - texture.ScaleAdjust *= Math.Max(texture.DisplayWidth / maxSize.X, texture.DisplayHeight / maxSize.Y); - return texture; + maxSize *= texture.ScaleAdjust; + + var croppedTexture = texture.Crop(new RectangleF(texture.Width / 2f - maxSize.X / 2f, texture.Height / 2f - maxSize.Y / 2f, maxSize.X, maxSize.Y)); + croppedTexture.ScaleAdjust = texture.ScaleAdjust; + return croppedTexture; } public static bool HasFont(this ISkin source, LegacyFont font) From 314ecec65b34c73a293e7f8fbf14183c5dcfa6b4 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 30 Sep 2023 01:17:59 +0300 Subject: [PATCH 02/14] Refactor player max dimensions test scene to actually upscale textures --- .../Gameplay/TestScenePlayerMaxDimensions.cs | 77 ++++++++++++++++--- osu.Game/Skinning/Skin.cs | 5 +- 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerMaxDimensions.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerMaxDimensions.cs index a8ed44c7f8..68443b234b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerMaxDimensions.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerMaxDimensions.cs @@ -3,14 +3,20 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Graphics.Textures; +using osu.Framework.IO.Stores; using osu.Framework.Testing; using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Screens.Play; using osu.Game.Skinning; +using SixLabors.ImageSharp; +using SixLabors.ImageSharp.Processing; namespace osu.Game.Tests.Visual.Gameplay { @@ -23,6 +29,9 @@ namespace osu.Game.Tests.Visual.Gameplay /// public partial class TestScenePlayerMaxDimensions : TestSceneAllRulesetPlayers { + // scale textures to 4 times their size. + private const int scale_factor = 4; + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) { var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); @@ -63,18 +72,66 @@ namespace osu.Game.Tests.Visual.Gameplay remove { } } - public override Texture? GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) - { - var texture = base.GetTexture(componentName, wrapModeS, wrapModeT); - - if (texture != null) - texture.ScaleAdjust /= 8f; - - return texture; - } - public ISkin FindProvider(Func lookupFunction) => this; public IEnumerable AllSources => new[] { this }; + + protected override IResourceStore CreateTextureLoaderStore(IStorageResourceProvider resources, IResourceStore storage) + => new UpscaledTextureLoaderStore(base.CreateTextureLoaderStore(resources, storage)); + + private class UpscaledTextureLoaderStore : IResourceStore + { + private readonly IResourceStore? textureStore; + + public UpscaledTextureLoaderStore(IResourceStore? textureStore) + { + this.textureStore = textureStore; + } + + public void Dispose() + { + textureStore?.Dispose(); + } + + public TextureUpload Get(string name) + { + var textureUpload = textureStore?.Get(name); + + // NRT not enabled on framework side classes (IResourceStore / TextureLoaderStore), welp. + if (textureUpload == null) + return null!; + + return upscale(textureUpload); + } + + public async Task GetAsync(string name, CancellationToken cancellationToken = new CancellationToken()) + { + // NRT not enabled on framework side classes (IResourceStore / TextureLoaderStore), welp. + if (textureStore == null) + return null!; + + var textureUpload = await textureStore.GetAsync(name, cancellationToken).ConfigureAwait(false); + + if (textureUpload == null) + return null!; + + return await Task.Run(() => upscale(textureUpload), cancellationToken).ConfigureAwait(false); + } + + private TextureUpload upscale(TextureUpload textureUpload) + { + var image = Image.LoadPixelData(textureUpload.Data.ToArray(), textureUpload.Width, textureUpload.Height); + + // The original texture upload will no longer be returned or used. + textureUpload.Dispose(); + + image.Mutate(i => i.Resize(new Size(textureUpload.Width, textureUpload.Height) * scale_factor)); + return new TextureUpload(image); + } + + public Stream? GetStream(string name) => textureStore?.GetStream(name); + + public IEnumerable GetAvailableResources() => textureStore?.GetAvailableResources() ?? Array.Empty(); + } } } } diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index ccf49d722f..1e312142d7 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -88,7 +88,7 @@ namespace osu.Game.Skinning } Samples = samples; - Textures = new TextureStore(resources.Renderer, new MaxDimensionLimitedTextureLoaderStore(resources.CreateTextureLoaderStore(storage))); + Textures = new TextureStore(resources.Renderer, CreateTextureLoaderStore(resources, storage)); } else { @@ -171,6 +171,9 @@ namespace osu.Game.Skinning } } + protected virtual IResourceStore CreateTextureLoaderStore(IStorageResourceProvider resources, IResourceStore storage) + => new MaxDimensionLimitedTextureLoaderStore(resources.CreateTextureLoaderStore(storage)); + protected virtual void ParseConfigurationStream(Stream stream) { using (LineBufferedReader reader = new LineBufferedReader(stream, true)) From 48209872bfd73d347a7fa6f35958d77a659a6fbc Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 4 Oct 2023 13:53:33 +0900 Subject: [PATCH 03/14] Fix spinner requirements being susceptible to FP precision --- osu.Game.Rulesets.Osu/Objects/Spinner.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Spinner.cs b/osu.Game.Rulesets.Osu/Objects/Spinner.cs index aca55c6bd9..e3dfe8e69a 100644 --- a/osu.Game.Rulesets.Osu/Objects/Spinner.cs +++ b/osu.Game.Rulesets.Osu/Objects/Spinner.cs @@ -70,8 +70,11 @@ namespace osu.Game.Rulesets.Osu.Objects double secondsDuration = Duration / 1000; - SpinsRequired = (int)(minRps * secondsDuration); - MaximumBonusSpins = Math.Max(0, (int)(maxRps * secondsDuration) - SpinsRequired - bonus_spins_gap); + // Allow a 0.1ms floating point precision error in the calculation of the duration. + const double duration_error = 0.0001; + + SpinsRequired = (int)(minRps * secondsDuration + duration_error); + MaximumBonusSpins = Math.Max(0, (int)(maxRps * secondsDuration + duration_error) - SpinsRequired - bonus_spins_gap); } protected override void CreateNestedHitObjects(CancellationToken cancellationToken) From 2ae1a2435504f8c49391f6870286074af1a4781b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 4 Oct 2023 18:55:20 +0200 Subject: [PATCH 04/14] Add failing test covering counting multiple swell hits per frame --- .../Judgements/TestSceneSwellJudgements.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/osu.Game.Rulesets.Taiko.Tests/Judgements/TestSceneSwellJudgements.cs b/osu.Game.Rulesets.Taiko.Tests/Judgements/TestSceneSwellJudgements.cs index 4abad98eab..6e42ae7eb5 100644 --- a/osu.Game.Rulesets.Taiko.Tests/Judgements/TestSceneSwellJudgements.cs +++ b/osu.Game.Rulesets.Taiko.Tests/Judgements/TestSceneSwellJudgements.cs @@ -115,6 +115,48 @@ namespace osu.Game.Rulesets.Taiko.Tests.Judgements AddAssert("all tick offsets are 0", () => JudgementResults.Where(r => r.HitObject is SwellTick).All(r => r.TimeOffset == 0)); } + [Test] + public void TestAtMostOneSwellTickJudgedPerFrame() + { + const double swell_time = 1000; + + Swell swell = new Swell + { + StartTime = swell_time, + Duration = 1000, + RequiredHits = 10 + }; + + List frames = new List + { + new TaikoReplayFrame(1000), + new TaikoReplayFrame(1250, TaikoAction.LeftCentre, TaikoAction.LeftRim), + new TaikoReplayFrame(1251), + new TaikoReplayFrame(1500, TaikoAction.LeftCentre, TaikoAction.LeftRim, TaikoAction.RightCentre, TaikoAction.RightRim), + new TaikoReplayFrame(1501), + new TaikoReplayFrame(2000), + }; + + PerformTest(frames, CreateBeatmap(swell)); + + AssertJudgementCount(11); + + // this is a charitable interpretation of the inputs. + // + // for the frame at time 1250, we only count either one of the input actions - simple. + // + // for the frame at time 1500, we give the user the benefit of the doubt, + // and we ignore actions that wouldn't otherwise cause a hit due to not alternating, + // but we still count one (just one) of the actions that _would_ normally cause a hit. + // this is done as a courtesy to avoid stuff like key chattering after press blocking legitimate inputs. + for (int i = 0; i < 2; i++) + AssertResult(i, HitResult.IgnoreHit); + for (int i = 2; i < swell.RequiredHits; i++) + AssertResult(i, HitResult.IgnoreMiss); + + AssertResult(0, HitResult.IgnoreMiss); + } + /// /// Ensure input is correctly sent to subsequent hits if a swell is fully completed. /// From a1368df62f8bdcc72078c3672fd9c9f7630dfd0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 4 Oct 2023 19:03:12 +0200 Subject: [PATCH 05/14] Allow judging at most one swell tick per frame --- .../Objects/Drawables/DrawableSwell.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs index 3fa6f4b756..297ea93637 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs @@ -38,6 +38,8 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables private readonly CircularContainer targetRing; private readonly CircularContainer expandingRing; + private double? lastPressHandleTime; + public override bool DisplayResult => false; public DrawableSwell() @@ -140,6 +142,7 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables UnproxyContent(); lastWasCentre = null; + lastPressHandleTime = null; } protected override void AddNestedHitObject(DrawableHitObject hitObject) @@ -285,7 +288,14 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables if (lastWasCentre == isCentre) return false; + // If we've already successfully judged a tick this frame, do not judge more. + // Note that the ordering is important here - this is intentionally placed after the alternating check. + // That is done to prevent accidental double inputs blocking simultaneous but legitimate hits from registering. + if (lastPressHandleTime == Time.Current) + return true; + lastWasCentre = isCentre; + lastPressHandleTime = Time.Current; UpdateResult(true); From 9f350ce13c58a16c47a0e5bd770c4495cc892455 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 5 Oct 2023 13:20:07 +0900 Subject: [PATCH 06/14] Add new difficulty calculation github action --- .github/workflows/diffcalc.yml | 481 +++++++++++++++++++++------------ 1 file changed, 302 insertions(+), 179 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index 2c6ec17e18..ef1909dfae 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -1,206 +1,329 @@ -# Listens for new PR comments containing !pp check [id], and runs a diffcalc comparison against master. -# Usage: -# !pp check 0 | Runs only the osu! ruleset. -# !pp check 0 2 | Runs only the osu! and catch rulesets. -# - name: Difficulty Calculation + +run-name: "${{ github.event_name == 'workflow_dispatch' && format('Manual run: {0}', inputs.osu-b) || 'Automatic comment trigger' }}" + on: issue_comment: types: [ created ] + workflow_dispatch: + inputs: + osu-b: + description: "The target build of ppy/osu" + type: string + required: true + ruleset: + description: "The ruleset to process" + type: choice + required: true + options: + - osu + - taiko + - catch + - mania + converts: + description: "Include converted beatmaps" + type: boolean + required: false + default: true + ranked-only: + description: "Only ranked beatmaps" + type: boolean + required: false + default: true + generators: + description: "Comma-separated list of generators (available: [sr, pp, score])" + type: string + required: false + default: 'pp,sr' + osu-a: + description: "The source build of ppy/osu" + type: string + required: false + default: 'latest' + difficulty-calculator-a: + description: "The source build of ppy/osu-difficulty-calculator" + type: string + required: false + default: 'latest' + difficulty-calculator-b: + description: "The target build of ppy/osu-difficulty-calculator" + type: string + required: false + default: 'latest' + score-processor-a: + description: "The source build of ppy/osu-queue-score-statistics" + type: string + required: false + default: 'latest' + score-processor-b: + description: "The target build of ppy/osu-queue-score-statistics" + type: string + required: false + default: 'latest' + +permissions: + pull-requests: write env: - CONCURRENCY: 4 - ALLOW_DOWNLOAD: 1 - SAVE_DOWNLOADED: 1 - SKIP_INSERT_ATTRIBUTES: 1 + COMMENT_TAG: execution-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }} jobs: - metadata: - name: Check for requests + wait-for-queue: + name: "Wait for previous workflows" + runs-on: ubuntu-latest + if: ${{ !cancelled() && (github.event_name == 'workflow_dispatch' || contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER') }} + timeout-minutes: 50400 # 35 days, the maximum for jobs. + steps: + - uses: ahmadnassri/action-workflow-queue@v1 + with: + timeout: 2147483647 # Around 24 days, maximum supported. + delay: 120000 # Poll every 2 minutes. API seems fairly low on this one. + + create-comment: + name: Create PR comment + runs-on: ubuntu-latest + if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER' }} + steps: + - name: Create comment + uses: thollander/actions-comment-pull-request@v2 + with: + comment_tag: ${{ env.COMMENT_TAG }} + message: | + Difficulty calculation queued -- please wait! (${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) + + *This comment will update on completion* + + directory: + name: Prepare directory + needs: wait-for-queue runs-on: self-hosted - if: github.event.issue.pull_request && contains(github.event.comment.body, '!pp check') && (github.event.comment.author_association == 'MEMBER' || github.event.comment.author_association == 'OWNER') + if: ${{ !cancelled() && (github.event_name == 'workflow_dispatch' || contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER') }} outputs: - matrix: ${{ steps.generate-matrix.outputs.matrix }} - continue: ${{ steps.generate-matrix.outputs.continue }} + GENERATOR_DIR: ${{ steps.set-outputs.outputs.GENERATOR_DIR }} + GENERATOR_ENV: ${{ steps.set-outputs.outputs.GENERATOR_ENV }} + GOOGLE_CREDS_FILE: ${{ steps.set-outputs.outputs.GOOGLE_CREDS_FILE }} steps: - - name: Construct build matrix - id: generate-matrix + - name: Checkout + uses: actions/checkout@v3 + + - name: Checkout diffcalc-sheet-generator + uses: actions/checkout@v3 + with: + path: 'diffcalc-sheet-generator' + repository: 'smoogipoo/diffcalc-sheet-generator' + + - name: Set outputs + id: set-outputs run: | - if [[ "${{ github.event.comment.body }}" =~ "osu" ]] ; then - MATRIX_PROJECTS_JSON+='{ "name": "osu", "id": 0 },' - fi - if [[ "${{ github.event.comment.body }}" =~ "taiko" ]] ; then - MATRIX_PROJECTS_JSON+='{ "name": "taiko", "id": 1 },' - fi - if [[ "${{ github.event.comment.body }}" =~ "catch" ]] ; then - MATRIX_PROJECTS_JSON+='{ "name": "catch", "id": 2 },' - fi - if [[ "${{ github.event.comment.body }}" =~ "mania" ]] ; then - MATRIX_PROJECTS_JSON+='{ "name": "mania", "id": 3 },' - fi + echo "GENERATOR_DIR=${{ github.workspace }}/diffcalc-sheet-generator" >> "${GITHUB_OUTPUT}" + echo "GENERATOR_ENV=${{ github.workspace }}/diffcalc-sheet-generator/.env" >> "${GITHUB_OUTPUT}" + echo "GOOGLE_CREDS_FILE=${{ github.workspace }}/diffcalc-sheet-generator/google-credentials.json" >> "${GITHUB_OUTPUT}" - if [[ "${MATRIX_PROJECTS_JSON}" != "" ]]; then - MATRIX_JSON="{ \"ruleset\": [ ${MATRIX_PROJECTS_JSON} ] }" - echo "${MATRIX_JSON}" - CONTINUE="yes" - else - CONTINUE="no" - fi - - echo "continue=${CONTINUE}" >> $GITHUB_OUTPUT - echo "matrix=${MATRIX_JSON}" >> $GITHUB_OUTPUT - diffcalc: - name: Run + environment: + name: Setup environment + needs: directory runs-on: self-hosted - timeout-minutes: 1440 - if: needs.metadata.outputs.continue == 'yes' - needs: metadata - strategy: - matrix: ${{ fromJson(needs.metadata.outputs.matrix) }} + if: ${{ !cancelled() && needs.directory.result == 'success' }} + env: + VARS_JSON: ${{ toJSON(vars) }} steps: - - name: Verify MySQL connection from host + - name: Add base environment run: | - mysql -e "SHOW DATABASES" + # Required by diffcalc-sheet-generator + cp '${{ github.workspace }}/diffcalc-sheet-generator/.env.sample' "${{ needs.directory.outputs.GENERATOR_ENV }}" - - name: Drop previous databases - run: | - for db in osu_master osu_pr - do - mysql -e "DROP DATABASE IF EXISTS $db" + # Add Google credentials + echo '${{ secrets.DIFFCALC_GOOGLE_CREDENTIALS }}' | base64 -d > "${{ needs.directory.outputs.GOOGLE_CREDS_FILE }}" + + # Add repository variables + echo "${VARS_JSON}" | jq -c '. | to_entries | .[]' | while read -r line; do + opt=$(jq -r '.key' <<< ${line}) + val=$(jq -r '.value' <<< ${line}) + + if [[ "${opt}" =~ ^DIFFCALC_ ]]; then + optNoPrefix=$(echo "${opt}" | cut -d '_' -f2-) + sed -i "s;^${optNoPrefix}=.*$;${optNoPrefix}=${val};" "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi done - - name: Create directory structure + - name: Add pull-request environment + if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request }} run: | - mkdir -p $GITHUB_WORKSPACE/master/ - mkdir -p $GITHUB_WORKSPACE/pr/ + sed -i "s;^OSU_B=.*$;OSU_B=${{ github.event.issue.pull_request.url }};" "${{ needs.directory.outputs.GENERATOR_ENV }}" - - name: Get upstream branch # https://akaimo.hatenablog.jp/entry/2020/05/16/101251 - id: upstreambranch - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Add comment environment + if: ${{ github.event_name == 'issue_comment' }} run: | - echo "branchname=$(curl -H "Authorization: token ${GITHUB_TOKEN}" ${{ github.event.issue.pull_request.url }} | jq '.head.ref' | sed 's/\"//g')" >> $GITHUB_OUTPUT - echo "repo=$(curl -H "Authorization: token ${GITHUB_TOKEN}" ${{ github.event.issue.pull_request.url }} | jq '.head.repo.full_name' | sed 's/\"//g')" >> $GITHUB_OUTPUT - - # Checkout osu - - name: Checkout osu (master) - uses: actions/checkout@v3 - with: - path: 'master/osu' - - name: Checkout osu (pr) - uses: actions/checkout@v3 - with: - path: 'pr/osu' - repository: ${{ steps.upstreambranch.outputs.repo }} - ref: ${{ steps.upstreambranch.outputs.branchname }} - - - name: Checkout osu-difficulty-calculator (master) - uses: actions/checkout@v3 - with: - repository: ppy/osu-difficulty-calculator - path: 'master/osu-difficulty-calculator' - - name: Checkout osu-difficulty-calculator (pr) - uses: actions/checkout@v3 - with: - repository: ppy/osu-difficulty-calculator - path: 'pr/osu-difficulty-calculator' - - - name: Install .NET 5.0.x - uses: actions/setup-dotnet@v3 - with: - dotnet-version: "5.0.x" - - # Sanity checks to make sure diffcalc is not run when incompatible. - - name: Build diffcalc (master) - run: | - cd $GITHUB_WORKSPACE/master/osu-difficulty-calculator - ./UseLocalOsu.sh - dotnet build - - name: Build diffcalc (pr) - run: | - cd $GITHUB_WORKSPACE/pr/osu-difficulty-calculator - ./UseLocalOsu.sh - dotnet build - - - name: Download + import data - run: | - PERFORMANCE_DATA_NAME=$(curl https://data.ppy.sh/ | grep performance_${{ matrix.ruleset.name }}_top_1000 | tail -1 | awk -F "\"" '{print $2}' | sed 's/\.tar\.bz2//g') - BEATMAPS_DATA_NAME=$(curl https://data.ppy.sh/ | grep osu_files | tail -1 | awk -F "\"" '{print $2}' | sed 's/\.tar\.bz2//g') - - # Set env variable for further steps. - echo "BEATMAPS_PATH=$GITHUB_WORKSPACE/$BEATMAPS_DATA_NAME" >> $GITHUB_ENV - - cd $GITHUB_WORKSPACE - - echo "Downloading database dump $PERFORMANCE_DATA_NAME.." - wget -q -nc https://data.ppy.sh/$PERFORMANCE_DATA_NAME.tar.bz2 - echo "Extracting.." - tar -xf $PERFORMANCE_DATA_NAME.tar.bz2 - - echo "Downloading beatmap dump $BEATMAPS_DATA_NAME.." - wget -q -nc https://data.ppy.sh/$BEATMAPS_DATA_NAME.tar.bz2 - echo "Extracting.." - tar -xf $BEATMAPS_DATA_NAME.tar.bz2 - - cd $PERFORMANCE_DATA_NAME - - for db in osu_master osu_pr - do - echo "Setting up database $db.." - - mysql -e "CREATE DATABASE $db" - - echo "Importing beatmaps.." - cat osu_beatmaps.sql | mysql $db - echo "Importing beatmapsets.." - cat osu_beatmapsets.sql | mysql $db - - echo "Creating table structure.." - mysql $db -e 'CREATE TABLE `osu_beatmap_difficulty` ( - `beatmap_id` int unsigned NOT NULL, - `mode` tinyint NOT NULL DEFAULT 0, - `mods` int unsigned NOT NULL, - `diff_unified` float NOT NULL, - `last_update` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY (`beatmap_id`,`mode`,`mods`), - KEY `diff_sort` (`mode`,`mods`,`diff_unified`) - ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;' + # Add comment environment + echo '${{ github.event.comment.body }}' | sed -r 's/\r$//' | grep -E '^\w+=' | while read -r line; do + opt=$(echo ${line} | cut -d '=' -f1) + sed -i "s;^${opt}=.*$;${line};" "${{ needs.directory.outputs.GENERATOR_ENV }}" done - - name: Run diffcalc (master) - env: - DB_NAME: osu_master + - name: Add dispatch environment + if: ${{ github.event_name == 'workflow_dispatch' }} run: | - cd $GITHUB_WORKSPACE/master/osu-difficulty-calculator/osu.Server.DifficultyCalculator - dotnet run -c:Release -- all -m ${{ matrix.ruleset.id }} -ac -c ${{ env.CONCURRENCY }} - - name: Run diffcalc (pr) - env: - DB_NAME: osu_pr - run: | - cd $GITHUB_WORKSPACE/pr/osu-difficulty-calculator/osu.Server.DifficultyCalculator - dotnet run -c:Release -- all -m ${{ matrix.ruleset.id }} -ac -c ${{ env.CONCURRENCY }} + sed -i 's;^OSU_B=.*$;OSU_B=${{ inputs.osu-b }};' "${{ needs.directory.outputs.GENERATOR_ENV }}" + sed -i 's/^RULESET=.*$/RULESET=${{ inputs.ruleset }}/' "${{ needs.directory.outputs.GENERATOR_ENV }}" + sed -i 's/^GENERATORS=.*$/GENERATORS=${{ inputs.generators }}/' "${{ needs.directory.outputs.GENERATOR_ENV }}" - - name: Print diffs - run: | - mysql -e " - SELECT - m.beatmap_id, - m.mods, - b.filename, - m.diff_unified as 'sr_master', - p.diff_unified as 'sr_pr', - (p.diff_unified - m.diff_unified) as 'diff' - FROM osu_master.osu_beatmap_difficulty m - JOIN osu_pr.osu_beatmap_difficulty p - ON m.beatmap_id = p.beatmap_id - AND m.mode = p.mode - AND m.mods = p.mods - JOIN osu_pr.osu_beatmaps b - ON b.beatmap_id = p.beatmap_id - WHERE abs(m.diff_unified - p.diff_unified) > 0.1 - ORDER BY abs(m.diff_unified - p.diff_unified) - DESC - LIMIT 10000;" + if [[ '${{ inputs.osu-a }}' != 'latest' ]]; then + sed -i 's;^OSU_A=.*$;OSU_A=${{ inputs.osu-a }};' "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi - # Todo: Run ppcalc + if [[ '${{ inputs.difficulty-calculator-a }}' != 'latest' ]]; then + sed -i 's;^DIFFICULTY_CALCULATOR_A=.*$;DIFFICULTY_CALCULATOR_A=${{ inputs.difficulty-calculator-a }};' "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi + + if [[ '${{ inputs.difficulty-calculator-b }}' != 'latest' ]]; then + sed -i 's;^DIFFICULTY_CALCULATOR_B=.*$;DIFFICULTY_CALCULATOR_B=${{ inputs.difficulty-calculator-b }};' "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi + + if [[ '${{ inputs.score-processor-a }}' != 'latest' ]]; then + sed -i 's;^SCORE_PROCESSOR_A=.*$;SCORE_PROCESSOR_A=${{ inputs.score-processor-a }};' "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi + + if [[ '${{ inputs.score-processor-b }}' != 'latest' ]]; then + sed -i 's;^SCORE_PROCESSOR_B=.*$;SCORE_PROCESSOR_B=${{ inputs.score-processor-b }};' "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi + + if [[ '${{ inputs.converts }}' == 'true' ]]; then + sed -i 's/^NO_CONVERTS=.*$/NO_CONVERTS=0/' "${{ needs.directory.outputs.GENERATOR_ENV }}" + else + sed -i 's/^NO_CONVERTS=.*$/NO_CONVERTS=1/' "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi + + if [[ '${{ inputs.ranked-only }}' == 'true' ]]; then + sed -i 's/^RANKED_ONLY=.*$/RANKED_ONLY=1/' "${{ needs.directory.outputs.GENERATOR_ENV }}" + else + sed -i 's/^RANKED_ONLY=.*$/RANKED_ONLY=0/' "${{ needs.directory.outputs.GENERATOR_ENV }}" + fi + + scores: + name: Setup scores + needs: [ directory, environment ] + runs-on: self-hosted + if: ${{ !cancelled() && needs.environment.result == 'success' }} + steps: + - name: Query latest data + id: query + run: | + ruleset=$(cat ${{ needs.directory.outputs.GENERATOR_ENV }} | grep -E '^RULESET=' | cut -d '=' -f2-) + performance_data_name=$(curl -s "https://data.ppy.sh/" | grep "performance_${ruleset}_top_1000\b" | tail -1 | awk -F "'" '{print $2}' | sed 's/\.tar\.bz2//g') + + echo "TARGET_DIR=${{ needs.directory.outputs.GENERATOR_DIR }}/sql/${ruleset}" >> "${GITHUB_OUTPUT}" + echo "DATA_NAME=${performance_data_name}" >> "${GITHUB_OUTPUT}" + + - name: Restore cache + id: restore-cache + uses: maxnowack/local-cache@v1 + with: + path: ${{ steps.query.outputs.DATA_NAME }}.tar.bz2 + key: ${{ steps.query.outputs.DATA_NAME }} + + - name: Download + if: steps.restore-cache.outputs.cache-hit != 'true' + run: | + wget -q -nc "https://data.ppy.sh/${{ steps.query.outputs.DATA_NAME }}.tar.bz2" + + - name: Extract + run: | + tar -I lbzip2 -xf "${{ steps.query.outputs.DATA_NAME }}.tar.bz2" + rm -r "${{ steps.query.outputs.TARGET_DIR }}" + mv "${{ steps.query.outputs.DATA_NAME }}" "${{ steps.query.outputs.TARGET_DIR }}" + + beatmaps: + name: Setup beatmaps + needs: directory + runs-on: self-hosted + if: ${{ !cancelled() && needs.directory.result == 'success' }} + steps: + - name: Query latest data + id: query + run: | + beatmaps_data_name=$(curl -s "https://data.ppy.sh/" | grep "osu_files" | tail -1 | awk -F "'" '{print $2}' | sed 's/\.tar\.bz2//g') + + echo "TARGET_DIR=${{ needs.directory.outputs.GENERATOR_DIR }}/beatmaps" >> "${GITHUB_OUTPUT}" + echo "DATA_NAME=${beatmaps_data_name}" >> "${GITHUB_OUTPUT}" + + - name: Restore cache + id: restore-cache + uses: maxnowack/local-cache@v1 + with: + path: ${{ steps.query.outputs.DATA_NAME }}.tar.bz2 + key: ${{ steps.query.outputs.DATA_NAME }} + + - name: Download + if: steps.restore-cache.outputs.cache-hit != 'true' + run: | + wget -q -nc "https://data.ppy.sh/${{ steps.query.outputs.DATA_NAME }}.tar.bz2" + + - name: Extract + run: | + tar -I lbzip2 -xf "${{ steps.query.outputs.DATA_NAME }}.tar.bz2" + rm -r "${{ steps.query.outputs.TARGET_DIR }}" + mv "${{ steps.query.outputs.DATA_NAME }}" "${{ steps.query.outputs.TARGET_DIR }}" + + generator: + name: Run generator + needs: [ directory, environment, scores, beatmaps ] + runs-on: self-hosted + timeout-minutes: 720 + if: ${{ !cancelled() && needs.scores.result == 'success' && needs.beatmaps.result == 'success' }} + outputs: + TARGET: ${{ steps.run.outputs.TARGET }} + SPREADSHEET_LINK: ${{ steps.run.outputs.SPREADSHEET_LINK }} + steps: + - name: Run + id: run + run: | + # Add the GitHub token. This needs to be done here because it's unique per-job. + sed -i 's/^GH_TOKEN=.*$/GH_TOKEN=${{ github.token }}/' "${{ needs.directory.outputs.GENERATOR_ENV }}" + + cd "${{ needs.directory.outputs.GENERATOR_DIR }}" + docker-compose up --build generator + + link=$(docker-compose logs generator -n 10 | grep 'http' | sed -E 's/^.*(http.*)$/\1/') + target=$(cat "${{ needs.directory.outputs.GENERATOR_ENV }}" | grep -E '^OSU_B=' | cut -d '=' -f2-) + + echo "TARGET=${target}" >> "${GITHUB_OUTPUT}" + echo "SPREADSHEET_LINK=${link}" >> "${GITHUB_OUTPUT}" + + - name: Shutdown + if: ${{ always() }} + run: | + cd "${{ needs.directory.outputs.GENERATOR_DIR }}" + docker-compose down + + - name: Output info + if: ${{ success() }} + run: | + echo "Target: ${{ steps.run.outputs.TARGET }}" + echo "Spreadsheet: ${{ steps.run.outputs.SPREADSHEET_LINK }}" + + update-comment: + name: Update PR comment + needs: [ create-comment, generator ] + runs-on: ubuntu-latest + if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER' }} + steps: + - name: Update comment on success + if: ${{ needs.generator.result == 'success' }} + uses: thollander/actions-comment-pull-request@v2 + with: + comment_tag: ${{ env.COMMENT_TAG }} + mode: upsert + create_if_not_exists: false + message: | + Target: ${{ needs.generator.outputs.TARGET }} + Spreadsheet: ${{ needs.generator.outputs.SPREADSHEET_LINK }} + + - name: Update comment on failure + if: ${{ needs.generator.result == 'failure' }} + uses: thollander/actions-comment-pull-request@v2 + with: + comment_tag: ${{ env.COMMENT_TAG }} + mode: upsert + create_if_not_exists: false + message: | + Difficulty calculation failed: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} \ No newline at end of file From bca257eeda9d3aac95011c5d1631a93af547de86 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 5 Oct 2023 13:32:20 +0900 Subject: [PATCH 07/14] Add description --- .github/workflows/diffcalc.yml | 42 ++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index ef1909dfae..168f411264 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -1,4 +1,42 @@ -name: Difficulty Calculation +# ## Description +# +# Uses [diffcalc-sheet-generator](https://github.com/smoogipoo/diffcalc-sheet-generator) to run two builds of osu and generate an SR/PP/Score comparison spreadsheet. +# +# Can be dispatched both via GitHub -> Actions workflow, or via '!diffcalc' in an issue/PR comment. +# +# ## Requirements +# +# Self-hosted runner with installed: +# - `docker >= 20.10.16` +# - `docker-compose >= 2.5.1` +# - `lbzip2` +# - `jq` +# +# ## Usage +# +# The workflow can be run in two ways: +# 1. Via workflow dispatch. +# 2. By an owner of the repository posting a pull request or issue comment containing `!diffcalc`. +# For pull requests, the workflow will assume the pull request as the target to compare against (i.e. the `OSU_B` variable). +# Any lines in the comment of the form `KEY=VALUE` are treated as variables for the generator. +# +# ## Google Service Account +# +# Spreadsheets are uploaded to a Google Service Account, and exposed with read-only permissions to the wider audience. +# +# 1. Create a project at https://console.cloud.google.com +# 2. Enable the `Google Sheets` and `Google Drive` APIs. +# 3. Create a Service Account +# 4. Generate a key in the JSON format. +# 5. Encode the key as base64 and store as an **actions secret** with name **`DIFFCALC_GOOGLE_CREDENTIALS`** +# +# ## Environment variables +# +# The default environment may be configured via **actions variables**. +# +# Refer to [the sample environment](https://github.com/smoogipoo/diffcalc-sheet-generator/blob/master/.env.sample), and prefix each variable with `DIFFCALC_` (e.g. `DIFFCALC_THREADS`, `DIFFCALC_INNODB_BUFFER_SIZE`, etc...). + +name: Run difficulty calculation comparison run-name: "${{ github.event_name == 'workflow_dispatch' && format('Manual run: {0}', inputs.osu-b) || 'Automatic comment trigger' }}" @@ -326,4 +364,4 @@ jobs: mode: upsert create_if_not_exists: false message: | - Difficulty calculation failed: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} \ No newline at end of file + Difficulty calculation failed: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} From 2e0ca457736cc15939342191ddc3873dda8a2788 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 5 Oct 2023 14:09:58 +0900 Subject: [PATCH 08/14] Remove duplicated usage instructions --- .github/workflows/diffcalc.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index 168f411264..5fde6e2f1a 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -2,8 +2,6 @@ # # Uses [diffcalc-sheet-generator](https://github.com/smoogipoo/diffcalc-sheet-generator) to run two builds of osu and generate an SR/PP/Score comparison spreadsheet. # -# Can be dispatched both via GitHub -> Actions workflow, or via '!diffcalc' in an issue/PR comment. -# # ## Requirements # # Self-hosted runner with installed: From 5a0faaa0b118e4935c05a78e00ec4214c69a6b5f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 Oct 2023 14:30:04 +0900 Subject: [PATCH 09/14] Fix `TestReplayExport` intermittent failure The previous fix was not working as it was checking the path for the prefix `_`, not the filename. See https://github.com/ppy/osu/runs/17415814653#r0s2 which clearly shows this. --- .../Visual/Gameplay/TestScenePlayerLocalScoreImport.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs index feda251744..6a7fab86d6 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.IO; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -173,7 +174,7 @@ namespace osu.Game.Tests.Visual.Gameplay string? filePath = null; // Files starting with _ are temporary, created by CreateFileSafely call. - AddUntilStep("wait for export file", () => filePath = LocalStorage.GetFiles("exports").SingleOrDefault(f => !f.StartsWith("_", StringComparison.Ordinal)), () => Is.Not.Null); + AddUntilStep("wait for export file", () => filePath = LocalStorage.GetFiles("exports").SingleOrDefault(f => !Path.GetFileName(f).StartsWith("_", StringComparison.Ordinal)), () => Is.Not.Null); AddAssert("filesize is non-zero", () => { using (var stream = LocalStorage.GetStream(filePath)) From 76cc2f9f22380d132500d6b5450ae1a76e868e49 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 Oct 2023 14:58:50 +0900 Subject: [PATCH 10/14] Fix `WaveformComparisonDisplay` potentially crashing on invalid track length As seen at https://github.com/ppy/osu/runs/17415814918#r0s2. --- osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs b/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs index 3b3acea935..d598b2cfbf 100644 --- a/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs +++ b/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs @@ -191,6 +191,10 @@ namespace osu.Game.Screens.Edit.Timing private void regenerateDisplay(bool animated) { + // Before a track is loaded, it won't have a valid length, which will break things. + if (!beatmap.Value.Track.IsLoaded) + return; + double index = (displayedTime - selectedGroupStartTime) / timingPoint.BeatLength; // Chosen as a pretty usable number across all BPMs. From 61fd4186af8781af2791e6760d925760beba8f21 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 Oct 2023 22:55:23 +0900 Subject: [PATCH 11/14] Ensure `regenerateDisplay` is eventually performed if originally called before load --- osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs b/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs index d598b2cfbf..856bc7c303 100644 --- a/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs +++ b/osu.Game/Screens/Edit/Timing/WaveformComparisonDisplay.cs @@ -94,7 +94,7 @@ namespace osu.Game.Screens.Edit.Timing controlPointGroups.BindTo(editorBeatmap.ControlPointInfo.Groups); controlPointGroups.BindCollectionChanged((_, _) => updateTimingGroup()); - beatLength.BindValueChanged(_ => regenerateDisplay(true), true); + beatLength.BindValueChanged(_ => Scheduler.AddOnce(regenerateDisplay, true), true); displayLocked.BindValueChanged(locked => { @@ -186,14 +186,17 @@ namespace osu.Game.Screens.Edit.Timing return; displayedTime = time; - regenerateDisplay(animated); + Scheduler.AddOnce(regenerateDisplay, animated); } private void regenerateDisplay(bool animated) { // Before a track is loaded, it won't have a valid length, which will break things. if (!beatmap.Value.Track.IsLoaded) + { + Scheduler.AddOnce(regenerateDisplay, animated); return; + } double index = (displayedTime - selectedGroupStartTime) / timingPoint.BeatLength; From d660395850c6934fc16bb0080cbc8812090750a7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 Oct 2023 23:02:45 +0900 Subject: [PATCH 12/14] Ensure there's no chance of a press being ignored during rewind flow --- osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs index 297ea93637..a8b48450e8 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableSwell.cs @@ -17,6 +17,7 @@ using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Taiko.Skinning.Default; +using osu.Game.Screens.Play; using osu.Game.Skinning; namespace osu.Game.Rulesets.Taiko.Objects.Drawables @@ -269,6 +270,9 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables ProxyContent(); else UnproxyContent(); + + if ((Clock as IGameplayClock)?.IsRewinding == true) + lastPressHandleTime = null; } private bool? lastWasCentre; From 0ae4a0f11f4b2d76b22351535ce3695b39c1fbfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 5 Oct 2023 20:37:10 +0200 Subject: [PATCH 13/14] Adjust gameplay element maximum size limits for backwards compatibility The new limits were chosen by sampling across over 4000 skins. The methodology for doing so is described in the following gist: https://gist.github.com/bdach/6228ba41d128b23d1f89142f404108a3 --- .../Skinning/Legacy/LegacyBananaPiece.cs | 2 +- .../Skinning/Legacy/LegacyDropletPiece.cs | 2 +- osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyFruitPiece.cs | 2 +- .../Skinning/Legacy/LegacyMainCirclePiece.cs | 4 ++-- osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs | 2 +- osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs | 4 ++-- .../Skinning/Legacy/OsuLegacySkinTransformer.cs | 6 +++--- .../Skinning/Legacy/LegacyCirclePiece.cs | 5 +++-- 8 files changed, 14 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyBananaPiece.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyBananaPiece.cs index c6f32e2014..ae530e94fc 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyBananaPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyBananaPiece.cs @@ -9,7 +9,7 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy { public partial class LegacyBananaPiece : LegacyCatchHitObjectPiece { - private static readonly Vector2 banana_max_size = new Vector2(128); + private static readonly Vector2 banana_max_size = new Vector2(160); protected override void LoadComplete() { diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyDropletPiece.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyDropletPiece.cs index c6c0839fba..a121d20d3d 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyDropletPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyDropletPiece.cs @@ -9,7 +9,7 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy { public partial class LegacyDropletPiece : LegacyCatchHitObjectPiece { - private static readonly Vector2 droplet_max_size = new Vector2(82, 103); + private static readonly Vector2 droplet_max_size = new Vector2(160); public LegacyDropletPiece() { diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyFruitPiece.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyFruitPiece.cs index 62097d79bd..3a8b5b427a 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyFruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyFruitPiece.cs @@ -9,7 +9,7 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy { internal partial class LegacyFruitPiece : LegacyCatchHitObjectPiece { - private static readonly Vector2 fruit_max_size = new Vector2(128); + private static readonly Vector2 fruit_max_size = new Vector2(160); protected override void LoadComplete() { diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs index 8990204931..3ec914596a 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs @@ -67,7 +67,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy // expected behaviour in this scenario is not showing the overlay, rather than using hitcircleoverlay.png. InternalChildren = new[] { - CircleSprite = new LegacyKiaiFlashingDrawable(() => new Sprite { Texture = skin.GetTexture(circleName)?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS) }) + CircleSprite = new LegacyKiaiFlashingDrawable(() => new Sprite { Texture = skin.GetTexture(circleName)?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS * 2) }) { Anchor = Anchor.Centre, Origin = Anchor.Centre, @@ -76,7 +76,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Child = OverlaySprite = new LegacyKiaiFlashingDrawable(() => skin.GetAnimation(@$"{circleName}overlay", true, true, frameLength: 1000 / 2d, maxSize: OsuHitObject.OBJECT_DIMENSIONS)) + Child = OverlaySprite = new LegacyKiaiFlashingDrawable(() => skin.GetAnimation(@$"{circleName}overlay", true, true, frameLength: 1000 / 2d, maxSize: OsuHitObject.OBJECT_DIMENSIONS * 2)) { Anchor = Anchor.Centre, Origin = Anchor.Centre, diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs index 25de6d2381..9048a92e13 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs @@ -39,7 +39,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy var skin = skinSource.FindProvider(s => s.GetTexture(lookupName) != null); - InternalChild = arrow = (skin?.GetAnimation(lookupName, true, true, maxSize: OsuHitObject.OBJECT_DIMENSIONS) ?? Empty()).With(d => + InternalChild = arrow = (skin?.GetAnimation(lookupName, true, true, maxSize: OsuHitObject.OBJECT_DIMENSIONS * 2) ?? Empty()).With(d => { d.Anchor = Anchor.Centre; d.Origin = Anchor.Centre; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index c3beb5bc35..412fd30931 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -47,7 +47,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Texture = skin.GetTexture("sliderb-nd")?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS), + Texture = skin.GetTexture("sliderb-nd")?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS * 2), Colour = new Color4(5, 5, 5, 255), }, LegacyColourCompatibility.ApplyWithDoubledAlpha(animationContent.With(d => @@ -59,7 +59,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Texture = skin.GetTexture("sliderb-spec")?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS), + Texture = skin.GetTexture("sliderb-spec")?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS * 2), Blending = BlendingParameters.Additive, }, }; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs index ea6f6fe6ce..13b8fa912e 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs @@ -42,14 +42,14 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy return this.GetAnimation("sliderscorepoint", false, false); case OsuSkinComponents.SliderFollowCircle: - var followCircleContent = this.GetAnimation("sliderfollowcircle", true, true, true, maxSize: new Vector2(308f)); + var followCircleContent = this.GetAnimation("sliderfollowcircle", true, true, true, maxSize: OsuHitObject.OBJECT_DIMENSIONS * 3); if (followCircleContent != null) return new LegacyFollowCircle(followCircleContent); return null; case OsuSkinComponents.SliderBall: - var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: "", maxSize: OsuHitObject.OBJECT_DIMENSIONS); + var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: "", maxSize: OsuHitObject.OBJECT_DIMENSIONS * 2); // todo: slider ball has a custom frame delay based on velocity // Math.Max((150 / Velocity) * GameBase.SIXTY_FRAME_TIME, GameBase.SIXTY_FRAME_TIME); @@ -139,7 +139,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy if (!this.HasFont(LegacyFont.HitCircle)) return null; - return new LegacySpriteText(LegacyFont.HitCircle, OsuHitObject.OBJECT_DIMENSIONS) + return new LegacySpriteText(LegacyFont.HitCircle, OsuHitObject.OBJECT_DIMENSIONS * 2) { // stable applies a blanket 0.8x scale to hitcircle fonts Scale = new Vector2(0.8f), diff --git a/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyCirclePiece.cs b/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyCirclePiece.cs index c94016d2b1..4dce3f1d4a 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyCirclePiece.cs +++ b/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyCirclePiece.cs @@ -23,6 +23,7 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Legacy public partial class LegacyCirclePiece : CompositeDrawable, IHasAccentColour { private static readonly Vector2 circle_piece_size = new Vector2(128); + private static readonly Vector2 max_circle_sprite_size = new Vector2(160); private Drawable backgroundLayer = null!; private Drawable? foregroundLayer; @@ -54,9 +55,9 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Legacy string prefix = ((drawableHitObject.HitObject as TaikoStrongableHitObject)?.IsStrong ?? false) ? big_hit : normal_hit; - return skin.GetAnimation($"{prefix}{lookup}", true, false, maxSize: circle_piece_size) ?? + return skin.GetAnimation($"{prefix}{lookup}", true, false, maxSize: max_circle_sprite_size) ?? // fallback to regular size if "big" version doesn't exist. - skin.GetAnimation($"{normal_hit}{lookup}", true, false, maxSize: circle_piece_size); + skin.GetAnimation($"{normal_hit}{lookup}", true, false, maxSize: max_circle_sprite_size); } // backgroundLayer is guaranteed to exist due to the pre-check in TaikoLegacySkinTransformer. From 96bb8ed150e2828c82efcd8a8641019a64dbf857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 5 Oct 2023 22:33:49 +0200 Subject: [PATCH 14/14] Apply feedback regarding maximum osu! element sizings - `default-N` number sprites maximum size increased by 1.25x to a total of 320x320 to counteract the 0.8x factor applied onto them when displayed on a hitcircle. - `sliderb` and parts' maximum size increased to 384x384, to match `sliderfollowcircle`, as the two are apparently sometimes used interchangeably by skinners to achieve different visual effects. --- .../Skinning/Legacy/LegacySliderBall.cs | 5 ++--- .../Legacy/OsuLegacySkinTransformer.cs | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index 412fd30931..5535903c73 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -7,7 +7,6 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Rulesets.Objects.Drawables; -using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Skinning; using osuTK.Graphics; @@ -47,7 +46,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Texture = skin.GetTexture("sliderb-nd")?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS * 2), + Texture = skin.GetTexture("sliderb-nd")?.WithMaximumSize(OsuLegacySkinTransformer.MAX_FOLLOW_CIRCLE_AREA_SIZE), Colour = new Color4(5, 5, 5, 255), }, LegacyColourCompatibility.ApplyWithDoubledAlpha(animationContent.With(d => @@ -59,7 +58,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Texture = skin.GetTexture("sliderb-spec")?.WithMaximumSize(OsuHitObject.OBJECT_DIMENSIONS * 2), + Texture = skin.GetTexture("sliderb-spec")?.WithMaximumSize(OsuLegacySkinTransformer.MAX_FOLLOW_CIRCLE_AREA_SIZE), Blending = BlendingParameters.Additive, }, }; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs index 13b8fa912e..88a4e17120 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs @@ -23,6 +23,16 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy /// public const float LEGACY_CIRCLE_RADIUS = OsuHitObject.OBJECT_RADIUS - 5; + /// + /// The maximum allowed size of sprites that reside in the follow circle area of a slider. + /// + /// + /// The reason this is extracted out to a constant, rather than be inlined in the follow circle sprite retrieval, + /// is that some skins will use `sliderb` elements to emulate a slider follow circle with slightly different visual effects applied + /// (`sliderb` is always shown and doesn't pulsate; `sliderfollowcircle` isn't always shown and pulsates). + /// + public static readonly Vector2 MAX_FOLLOW_CIRCLE_AREA_SIZE = OsuHitObject.OBJECT_DIMENSIONS * 3; + public OsuLegacySkinTransformer(ISkin skin) : base(skin) { @@ -42,14 +52,14 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy return this.GetAnimation("sliderscorepoint", false, false); case OsuSkinComponents.SliderFollowCircle: - var followCircleContent = this.GetAnimation("sliderfollowcircle", true, true, true, maxSize: OsuHitObject.OBJECT_DIMENSIONS * 3); + var followCircleContent = this.GetAnimation("sliderfollowcircle", true, true, true, maxSize: MAX_FOLLOW_CIRCLE_AREA_SIZE); if (followCircleContent != null) return new LegacyFollowCircle(followCircleContent); return null; case OsuSkinComponents.SliderBall: - var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: "", maxSize: OsuHitObject.OBJECT_DIMENSIONS * 2); + var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: "", maxSize: MAX_FOLLOW_CIRCLE_AREA_SIZE); // todo: slider ball has a custom frame delay based on velocity // Math.Max((150 / Velocity) * GameBase.SIXTY_FRAME_TIME, GameBase.SIXTY_FRAME_TIME); @@ -139,10 +149,11 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy if (!this.HasFont(LegacyFont.HitCircle)) return null; - return new LegacySpriteText(LegacyFont.HitCircle, OsuHitObject.OBJECT_DIMENSIONS * 2) + const float hitcircle_text_scale = 0.8f; + return new LegacySpriteText(LegacyFont.HitCircle, OsuHitObject.OBJECT_DIMENSIONS * 2 / hitcircle_text_scale) { // stable applies a blanket 0.8x scale to hitcircle fonts - Scale = new Vector2(0.8f), + Scale = new Vector2(hitcircle_text_scale), }; case OsuSkinComponents.SpinnerBody: