From 5b2d32fbb14fc7a3de4480d6792252af5ae501cb Mon Sep 17 00:00:00 2001 From: hfcRed <101019309+hfcRed@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:16:16 +0200 Subject: [PATCH] Trophy fixes (#3279) --- app/features/admin/admin-constants.ts | 2 +- .../trophies/TrophyRepository.server.test.ts | 65 ++++++++++++++++++ .../trophies/TrophyRepository.server.ts | 21 +++++- .../trophies/actions/trophies.new.server.ts | 4 -- app/features/trophies/components/Trophy.tsx | 18 ++++- app/features/trophies/core/model-analysis.ts | 29 +++++++- .../trophies/loaders/trophies.new.server.ts | 9 +-- .../trophies/routes/trophies.new.test.ts | 68 +++++++++++++++++++ app/features/trophies/routes/trophies.new.tsx | 64 +++++++++++++---- app/features/trophies/trophies-utils.test.ts | 6 ++ e2e/trophies.spec.ts | 5 ++ locales/da/trophies.json | 2 + locales/de/trophies.json | 2 + locales/en/trophies.json | 2 + locales/es-ES/trophies.json | 2 + locales/es-US/trophies.json | 2 + locales/fr-CA/trophies.json | 2 + locales/fr-EU/trophies.json | 2 + locales/he/trophies.json | 2 + locales/it/trophies.json | 2 + locales/ja/trophies.json | 2 + locales/ko/trophies.json | 2 + locales/nl/trophies.json | 2 + locales/pl/trophies.json | 2 + locales/pt-BR/trophies.json | 2 + locales/ru/trophies.json | 2 + locales/zh/trophies.json | 2 + 27 files changed, 294 insertions(+), 29 deletions(-) create mode 100644 app/features/trophies/routes/trophies.new.test.ts diff --git a/app/features/admin/admin-constants.ts b/app/features/admin/admin-constants.ts index a8526ca30..c306e5e7f 100644 --- a/app/features/admin/admin-constants.ts +++ b/app/features/admin/admin-constants.ts @@ -8,7 +8,7 @@ export const STAFF_IDS = [11329, 9719, 9342, 20774, 23094]; // hfcRed export const DEV_IDS = [27883]; // hfcRed Dreamy Cafy -export const QA_IDS: number[] = [27883, 38781, 10654]; +export const QA_IDS: number[] = [27883, 38176, 10654]; export const STAFF_DISCORD_IDS = [ "138757634500067328", diff --git a/app/features/trophies/TrophyRepository.server.test.ts b/app/features/trophies/TrophyRepository.server.test.ts index 96b835e5a..1f80ad677 100644 --- a/app/features/trophies/TrophyRepository.server.test.ts +++ b/app/features/trophies/TrophyRepository.server.test.ts @@ -252,6 +252,71 @@ describe("trophy list tiers", () => { } }); +describe("existsByName", () => { + let ownerId: number; + let approverIds: number[]; + let organizationId: number; + + beforeEach(async () => { + const owner = await UserFactory.create(); + ownerId = owner.id; + approverIds = (await UserFactory.createMany(2)).map((user) => user.id); + organizationId = (await TournamentOrganizationFactory.create({ ownerId })) + .id; + }); + + test("updating a trophy keeping its name does not collide with its accepted submission", async () => { + await TrophyFactory.createPending( + { name: "Winner's Cup", organizationId, submitterUserId: ownerId }, + { approverUserIds: approverIds }, + ); + + const trophy = await db + .selectFrom("Trophy") + .select("id") + .where("name", "=", "Winner's Cup") + .executeTakeFirstOrThrow(); + + expect( + await TrophyRepository.existsByName({ + name: "Winner's Cup", + excludeTrophyId: trophy.id, + }), + ).toBe(false); + }); + + test("an existing trophy's name still blocks new submissions", async () => { + await TrophyFactory.create({ name: "Winner's Cup" }); + + expect(await TrophyRepository.existsByName({ name: "Winner's Cup" })).toBe( + true, + ); + }); + + test("a submission awaiting review blocks the name", async () => { + await TrophyFactory.createPending({ + name: "Contested Cup", + organizationId, + submitterUserId: ownerId, + }); + + expect(await TrophyRepository.existsByName({ name: "Contested Cup" })).toBe( + true, + ); + }); + + test("a declined submission does not block the name", async () => { + await TrophyFactory.createPending( + { name: "Declined Cup", organizationId, submitterUserId: ownerId }, + { declinedBy: { userId: approverIds[0], reason: "reason" } }, + ); + + expect(await TrophyRepository.existsByName({ name: "Declined Cup" })).toBe( + false, + ); + }); +}); + describe("user deletion", () => { test("keeps their trophies and drops their approvals", async () => { const submitter = await UserFactory.create(); diff --git a/app/features/trophies/TrophyRepository.server.ts b/app/features/trophies/TrophyRepository.server.ts index e99bc2aa9..b0ef622e2 100644 --- a/app/features/trophies/TrophyRepository.server.ts +++ b/app/features/trophies/TrophyRepository.server.ts @@ -443,6 +443,10 @@ export async function findOrganizationIdById(trophyId: number) { return row?.organizationId ?? null; } +/** + * Checks whether a trophy name is taken by an existing trophy or a submission still + * awaiting review. Accepted submissions don't count. + */ export async function existsByName(args: { name: string; excludeTrophyId?: number; @@ -464,7 +468,22 @@ export async function existsByName(args: { .selectFrom("PendingTrophy") .select("id") .where("name", "=", args.name) - .where("declinedAt", "is", null); + .where("declinedAt", "is", null) + .where((eb) => + eb( + eb + .selectFrom("PendingTrophyApproval") + .select(({ fn }) => fn.countAll().as("count")) + .whereRef( + "PendingTrophyApproval.pendingTrophyId", + "=", + "PendingTrophy.id", + ) + .$asScalar(), + "<", + TROPHY_APPROVALS_REQUIRED, + ), + ); if (args.excludeTrophyId !== undefined) { pendingQuery = pendingQuery.where( diff --git a/app/features/trophies/actions/trophies.new.server.ts b/app/features/trophies/actions/trophies.new.server.ts index 521f82788..785ab227c 100644 --- a/app/features/trophies/actions/trophies.new.server.ts +++ b/app/features/trophies/actions/trophies.new.server.ts @@ -20,7 +20,6 @@ import { trophyFormSchema, } from "../trophies-schemas"; import { - canAccessTrophies, canEditTrophy, canReviewTrophies, compressTrophyModel, @@ -28,9 +27,6 @@ import { export const action: ActionFunction = async ({ request }) => { const user = requireUser(); - if (!canAccessTrophies(user)) { - throw new Response(null, { status: 404 }); - } const isJson = request.headers.get("Content-Type") === "application/json"; diff --git a/app/features/trophies/components/Trophy.tsx b/app/features/trophies/components/Trophy.tsx index e8116479b..c88556a29 100644 --- a/app/features/trophies/components/Trophy.tsx +++ b/app/features/trophies/components/Trophy.tsx @@ -1,6 +1,10 @@ import { clsx } from "clsx"; import { Ban } from "lucide-react"; -import { PicoCAD2Context, PicoCAD2Viewer } from "picocad2-web"; +import { + PicoCAD2Context, + PicoCAD2Viewer, + type RenderStats, +} from "picocad2-web"; import { createContext, useCallback, @@ -84,6 +88,7 @@ export function Trophy({ disableCameraControls, staticOnSoftwareRendering, pill, + onRenderStats, }: { model: string; className?: string; @@ -94,6 +99,7 @@ export function Trophy({ disableCameraControls?: boolean; staticOnSoftwareRendering?: boolean; pill?: React.ReactNode; + onRenderStats?: (stats: RenderStats) => void; }) { const ctxValue = useContext(TrophyCtx); const context = ctxValue?.context; @@ -102,6 +108,9 @@ export function Trophy({ const viewerRef = useRef(null); const [error, setError] = useState(false); + const onRenderStatsRef = useRef(onRenderStats); + onRenderStatsRef.current = onRenderStats; + const prevModelRef = useRef(model); if (prevModelRef.current !== model) { prevModelRef.current = model; @@ -151,8 +160,15 @@ export function Trophy({ return; } + if (context && onRenderStats) { + viewer.onFrame = () => { + onRenderStatsRef.current?.({ ...context.stats }); + }; + } + viewer.cameraMode = "spin"; viewer.cameraModeSpeed = 5; + viewer.animation.setTime(0); viewer.startRenderLoop(false); if (disableCameraControls) return; diff --git a/app/features/trophies/core/model-analysis.ts b/app/features/trophies/core/model-analysis.ts index 83eca936e..b8d9a4792 100644 --- a/app/features/trophies/core/model-analysis.ts +++ b/app/features/trophies/core/model-analysis.ts @@ -1,4 +1,9 @@ -import type { Color3, ExtrasOptions, ViewerSettings } from "picocad2-web"; +import type { + Color3, + ExtrasOptions, + RenderStats, + ViewerSettings, +} from "picocad2-web"; export interface TrophyModelAnalysis { cameraTargetCentered: boolean; @@ -50,6 +55,28 @@ export function analyzeTrophyModel(model: string): TrophyModelAnalysis | null { } } +/** + * Animations can toggle meshes on and off, so the peak across frames + * is what the draw call and poly metrics should show. + */ +export function mergePeakRenderStats( + previous: RenderStats | null, + frame: RenderStats, +) { + if ( + previous && + frame.drawCalls <= previous.drawCalls && + frame.polyCount <= previous.polyCount + ) { + return previous; + } + + return { + drawCalls: Math.max(frame.drawCalls, previous?.drawCalls ?? 0), + polyCount: Math.max(frame.polyCount, previous?.polyCount ?? 0), + }; +} + function isCameraTargetCentered(state: ModelState) { const target = state.settings.camera.target; return target[0] === 0 && target[2] === 0; diff --git a/app/features/trophies/loaders/trophies.new.server.ts b/app/features/trophies/loaders/trophies.new.server.ts index 59d8f5809..da0d47be7 100644 --- a/app/features/trophies/loaders/trophies.new.server.ts +++ b/app/features/trophies/loaders/trophies.new.server.ts @@ -3,19 +3,12 @@ import { requireUser } from "~/features/auth/core/user.server"; import type { SerializeFrom } from "~/utils/remix"; import * as TrophyRepository from "../TrophyRepository.server"; import { TROPHY_APPROVALS_REQUIRED } from "../trophies-constants"; -import { - canAccessTrophies, - canEditAnyTrophy, - canReviewTrophies, -} from "../trophies-utils"; +import { canEditAnyTrophy, canReviewTrophies } from "../trophies-utils"; export type NewTrophyLoaderData = SerializeFrom; export const loader = async (_args: LoaderFunctionArgs) => { const user = requireUser(); - if (!canAccessTrophies(user)) { - throw new Response(null, { status: 404 }); - } const canReview = canReviewTrophies(user); diff --git a/app/features/trophies/routes/trophies.new.test.ts b/app/features/trophies/routes/trophies.new.test.ts new file mode 100644 index 000000000..f544bcd51 --- /dev/null +++ b/app/features/trophies/routes/trophies.new.test.ts @@ -0,0 +1,68 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { REGULAR_USER_TEST_ID } from "~/db/seed/constants"; +import * as TournamentOrganizationFactory from "~/db/seed/factories/TournamentOrganizationFactory"; +import * as TrophyFactory from "~/db/seed/factories/TrophyFactory"; +import * as UserFactory from "~/db/seed/factories/UserFactory"; +import { ADMIN_ID } from "~/features/admin/admin-constants"; +import { decompressFromBase64 } from "~/utils/compression"; +import { wrappedAction, wrappedLoader } from "~/utils/Test"; +import { action } from "../actions/trophies.new.server"; +import { + loader, + type NewTrophyLoaderData, +} from "../loaders/trophies.new.server"; +import * as TrophyRepository from "../TrophyRepository.server"; +import type { trophyFormSchema } from "../trophies-schemas"; + +// remove file once feature is released + +vi.mock("~/features/trophies/trophies-constants", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("~/features/trophies/trophies-constants") + >()), + TROPHIES_RELEASED: false, +})); + +const newTrophyLoader = wrappedLoader({ loader }); +const submitAction = wrappedAction({ + action, + isJsonSubmission: true, +}); + +describe("trophy submissions before release", () => { + beforeEach(async () => { + await UserFactory.createAdmin(); + await UserFactory.createRegular(); + }); + + test("a regular user can open the submission page", async () => { + const data = await newTrophyLoader({ user: "regular" }); + + expect(data.canReview).toBe(false); + expect(data.currentUserId).toBe(REGULAR_USER_TEST_ID); + }); + + test("a regular user can submit a trophy", async () => { + const organization = await TournamentOrganizationFactory.create({ + ownerId: ADMIN_ID, + }); + + const result = await submitAction( + { + _action: "CREATE", + name: "Regular Trophy", + model: decompressFromBase64(TrophyFactory.MODELS[0]) ?? "", + organizationId: organization.id, + description: null, + }, + { user: "regular" }, + ); + + expect(result).toBe(null); + + const pending = + await TrophyRepository.pendingBySubmitter(REGULAR_USER_TEST_ID); + expect(pending.length).toBe(1); + expect(pending[0].name).toBe("Regular Trophy"); + }); +}); diff --git a/app/features/trophies/routes/trophies.new.tsx b/app/features/trophies/routes/trophies.new.tsx index c038773f3..1bf57b1d9 100644 --- a/app/features/trophies/routes/trophies.new.tsx +++ b/app/features/trophies/routes/trophies.new.tsx @@ -1,5 +1,6 @@ import clsx from "clsx"; import { Check, Clipboard, Dot, Trash2, TriangleAlert, X } from "lucide-react"; +import type { RenderStats } from "picocad2-web"; import * as React from "react"; import { Trans, useTranslation } from "react-i18next"; import { @@ -41,6 +42,7 @@ import { action } from "../actions/trophies.new.server"; import { Trophy, TrophyContextProvider } from "../components/Trophy"; import { analyzeTrophyModel, + mergePeakRenderStats, type TrophyModelAnalysis, } from "../core/model-analysis"; import { @@ -356,9 +358,24 @@ function ModelField({ }) { const { t } = useTranslation(["forms", "trophies"]); const [preview, setPreview] = React.useState(() => buildModelPreview(value)); + const [peak, setPeak] = React.useState<{ + model: string; + stats: RenderStats; + } | null>(null); useDebounce(() => setPreview(buildModelPreview(value)), 500, [value]); + const reportRenderStats = (stats: RenderStats) => { + const model = preview.compressedModel; + setPeak((prev) => { + const previousStats = prev?.model === model ? prev.stats : null; + const merged = mergePeakRenderStats(previousStats, stats); + return merged === previousStats ? prev : { model, stats: merged }; + }); + }; + + const peakStats = peak?.model === preview.compressedModel ? peak.stats : null; + return (
))} ) : null} - + ); } @@ -424,7 +442,13 @@ function buildModelPreview(model: string) { }; } -function ModelSpecs({ analysis }: { analysis: TrophyModelAnalysis | null }) { +function ModelSpecs({ + analysis, + peakStats, +}: { + analysis: TrophyModelAnalysis | null; + peakStats: RenderStats | null; +}) { const { t } = useTranslation(["trophies"]); const enforcedStatus = (passes: boolean) => @@ -432,6 +456,9 @@ function ModelSpecs({ analysis }: { analysis: TrophyModelAnalysis | null }) { const recommendedStatus = (withinLimit: boolean) => analysis ? (withinLimit ? "pass" : "warn") : null; + const drawCalls = peakStats?.drawCalls ?? analysis?.drawCalls ?? 0; + const polyCount = peakStats?.polyCount ?? analysis?.polyCount ?? 0; + return (
@@ -448,6 +475,8 @@ function ModelSpecs({ analysis }: { analysis: TrophyModelAnalysis | null }) { {t("trophies:new.specs.centered")} {t("trophies:new.specs.zoom")} {t("trophies:new.specs.angles")} + {t("trophies:new.specs.noStandaloneFlag")} + {t("trophies:new.specs.noTrademarked")}
@@ -457,13 +486,12 @@ function ModelSpecs({ analysis }: { analysis: TrophyModelAnalysis | null }) {
    (null); + const reportRenderStats = (stats: RenderStats) => + setPeakStats((prev) => mergePeakRenderStats(prev, stats)); + + const drawCalls = peakStats?.drawCalls ?? analysis?.drawCalls ?? 0; + const polyCount = peakStats?.polyCount ?? analysis?.polyCount ?? 0; + return (