feat: allow optional metrics to opt in to scoreID relevance

common/src/config/game-support/sdvx.ts

@@ -101,7 +101,15 @@ export const SDVX_SINGLE_CONF = {
 
 	optionalMetrics: {
 		...FAST_SLOW_MAXCOMBO,
-		exScore: { type: "INTEGER", chartDependentMax: true, formatter: FmtNum },
+		exScore: {
+			type: "INTEGER",
+			chartDependentMax: true,
+			formatter: FmtNum,
+
+			// We want to track the best EXScore a user gets, but it is an optional
+			// metric.N
+			partOfScoreID: true,
+		},
 		gauge: { type: "DECIMAL", validate: p.isBetween(0, 100), formatter: FmtPercent },
 	},
 

common/src/types/internals.ts

@@ -22,7 +22,20 @@ export type INTERNAL_GAME_PT_CONFIG = Readonly<{
 
 	preferredDefaultEnum: string;
 
-	optionalMetrics: Record<string, ConfScoreMetric>;
+	optionalMetrics: Record<
+		string,
+		ConfScoreMetric & {
+			/**
+			 * Should this optional metric be part of a score's unique
+			 * identifier?
+			 *
+			 * This should be used for extreme cases, like when a game introduces
+			 * a new scoring system that still needs to be optional, but players
+			 * don't want to be clobbered.
+			 */
+			partOfScoreID?: boolean;
+		}
+	>;
 
 	scoreRatingAlgs: Record<string, RatingAlgorithmConfig>;
 	sessionRatingAlgs: Record<string, RatingAlgorithmConfig>;

server/src/lib/score-import/framework/score-importing/score-id.test.ts

@@ -2,7 +2,11 @@ import { CreateScoreID, GetWithScoreID } from "./score-id";
 import t from "tap";
 import { dmf } from "test-utils/misc";
 import ResetDBState from "test-utils/resets";
-import { Testing511SPA, TestingIIDXSPDryScore } from "test-utils/test-data";
+import {
+	Testing511SPA,
+	TestingIIDXSPDryScore,
+	TestingSDVXSingleDryScore,
+} from "test-utils/test-data";
 import type { DryScoreData } from "../common/types";
 
 t.test("#GetWithScoreID", async (t) => {
@@ -75,6 +79,86 @@ t.test("#CreateScoreID", (t) => {
 		"ScoreIDs should not produce the same value if a provided metric is different."
 	);
 
+	const sdvxScoreID = CreateScoreID(
+		"sdvx:Single",
+		1,
+		TestingSDVXSingleDryScore,
+		Testing511SPA.chartID
+	);
+
+	t.not(
+		sdvxScoreID,
+		CreateScoreID(
+			"sdvx:Single",
+			1,
+			dmf(TestingSDVXSingleDryScore, {
+				scoreData: { optional: { exScore: 1 } },
+			}),
+			Testing511SPA.chartID
+		),
+		"Hash should be affected by specific optionalMetrics."
+	);
+
+	t.not(
+		CreateScoreID(
+			"sdvx:Single",
+			1,
+			dmf(TestingSDVXSingleDryScore, {
+				scoreData: { optional: { exScore: 1 } },
+			}),
+			Testing511SPA.chartID
+		),
+		CreateScoreID(
+			"sdvx:Single",
+			1,
+			dmf(TestingSDVXSingleDryScore, {
+				scoreData: { optional: { exScore: 100 } },
+			}),
+			Testing511SPA.chartID
+		),
+		"Hash should be affected by specific optionalMetrics."
+	);
+
+	t.equal(
+		CreateScoreID(
+			"sdvx:Single",
+			1,
+			dmf(TestingSDVXSingleDryScore, {
+				scoreData: { optional: { exScore: 1, fast: 18 } },
+			}),
+			Testing511SPA.chartID
+		),
+		CreateScoreID(
+			"sdvx:Single",
+			1,
+			dmf(TestingSDVXSingleDryScore, {
+				scoreData: { optional: { exScore: 1 } },
+			}),
+			Testing511SPA.chartID
+		),
+		"Hash should not be affected by irrelevant optionalMetrics."
+	);
+
+	t.equal(
+		CreateScoreID(
+			"sdvx:Single",
+			1,
+			dmf(TestingSDVXSingleDryScore, {
+				scoreData: { optional: { exScore: undefined } },
+			}),
+			Testing511SPA.chartID
+		),
+		CreateScoreID(
+			"sdvx:Single",
+			1,
+			dmf(TestingSDVXSingleDryScore, {
+				scoreData: { optional: { exScore: null } },
+			}),
+			Testing511SPA.chartID
+		),
+		"Null and Undefined should be the same for optional metrics"
+	);
+
 	t.end();
 });
 
@@ -83,7 +167,7 @@ t.test("#ScoreID Canary", (t) => {
 
 	t.equal(
 		scoreID,
-		"T5da661101a371a32caa57df7f5b43225ba45b273ded5d925da0e64ba6653a9d8",
+		"Tc4f10c2ca8e99478de5ae318a40ee66051f7297ca648415502b6f393a23e6f10",
 		"ScoreID **SHOULD BE DETERMINISTIC**. A change to the ScoreID algorithm is **a major breaking change.**"
 	);
 

server/src/lib/score-import/framework/score-importing/score-id.ts

@@ -2,7 +2,7 @@ import db from "external/mongo/db";
 import { GetGPTConfig } from "tachi-common";
 import crypto from "crypto";
 import type { DryScore } from "../common/types";
-import type { integer, GPTString, ProvidedMetrics } from "tachi-common";
+import type { integer, GPTString, ProvidedMetrics, OptionalMetrics } from "tachi-common";
 
 /**
  * Performs sha256 hashing on the input data.
@@ -13,6 +13,10 @@ function HashScoreIDString(scoreIDString: string) {
 	return crypto.createHash("sha256").update(scoreIDString).digest("hex");
 }
 
+function SortKeysAlphabetically(a: string, b: string) {
+	return a.localeCompare(b, "en-GB");
+}
+
 /**
  * Creates an identifier for this score.
  * This is used to deduplicate repeated scores.
@@ -28,12 +32,29 @@ export function CreateScoreID(
 
 	const gptConfig = GetGPTConfig(gptString);
 
-	for (const m of Object.keys(gptConfig.providedMetrics)) {
+	// @warn
+	// we need to sort these metric keys deterministically instead
+	// of relying on any sort of object-insertion order
+	// as that would throw the checksum out of sync.
+	for (const m of Object.keys(gptConfig.providedMetrics).sort(SortKeysAlphabetically)) {
 		const metric = m as keyof ProvidedMetrics[GPTString];
 
 		elements.push(dryScore.scoreData[metric]);
 	}
 
+	// Also include optional metrics in the checksum if they should be
+	// part of the scoreID.
+	for (const [m, conf] of Object.entries(gptConfig.optionalMetrics).sort((a, b) =>
+		SortKeysAlphabetically(a[0], b[0])
+	)) {
+		const metric = m as keyof OptionalMetrics[GPTString];
+
+		if (conf.partOfScoreID) {
+			// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
+			elements.push(dryScore.scoreData.optional[metric] ?? null);
+		}
+	}
+
 	const hash = HashScoreIDString(elements.join("\0"));
 
 	return `T${hash}`;