Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 8 additions & 8 deletions package-lock.json

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

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
"@dnd-kit/sortable": "^10.0.0",
"@fontsource/figtree": "^5.1.0",
"@hookform/resolvers": "^3.9.0",
"@ianpaschal/combat-command-components": "^1.8.3",
"@ianpaschal/combat-command-game-systems": "^1.4.0",
"@ianpaschal/combat-command-components": "^1.8.4",
"@ianpaschal/combat-command-game-systems": "^1.4.1",
"@mapbox/search-js-core": "^1.0.0-beta.25",
"@radix-ui/colors": "^3.0.0",
"@react-hook/window-size": "^3.1.1",
Expand Down
14 changes: 4 additions & 10 deletions src/components/MatchResultDetailFields/MatchResultDetailFields.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ import { CompatibleFormData } from './MatchResultDetailFields.types';

export interface MatchResultDetailFieldsProps {
className?: string;
disableFactions?: boolean;
}

export const MatchResultDetailFields = ({
className,
}: MatchResultDetailFieldsProps): JSX.Element => {
export const MatchResultDetailFields = (props: MatchResultDetailFieldsProps): JSX.Element => {
const { reset, watch, getFieldState } = useFormContext<CompatibleFormData>();
const gameSystem = watch('gameSystem');

Expand All @@ -36,20 +35,15 @@ export const MatchResultDetailFields = ({
*/
const playerOptions = usePlayerOptions();

const sharedProps = {
className,
playerOptions,
};

if (gameSystem === GameSystem.FlamesOfWarV4) {
return (
<FlamesOfWarV4MatchResultDetailFields {...sharedProps} />
<FlamesOfWarV4MatchResultDetailFields {...props} playerOptions={playerOptions} />
);
}

if (gameSystem === GameSystem.TeamYankeeV2) {
return (
<TeamYankeeV2MatchResultDetailFields {...sharedProps} />
<TeamYankeeV2MatchResultDetailFields {...props} playerOptions={playerOptions} />
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ import styles from './FlamesOfWarV4MatchResultDetailFields.module.scss';
export interface FlamesOfWarV4MatchResultDetailFieldsProps {
className?: string;
playerOptions: InputSelectOption<number>[];
disableFactions?: boolean;
}

export const FlamesOfWarV4MatchResultDetailFields = ({
className,
playerOptions,
disableFactions = false,
}: FlamesOfWarV4MatchResultDetailFieldsProps): JSX.Element => {
const [showScoreOverride, setShowScoreOverride] = useState<boolean>(false);

Expand All @@ -35,6 +37,8 @@ export const FlamesOfWarV4MatchResultDetailFields = ({
const { details } = values;
const gameSystemConfig = validateGameSystemConfig(GameSystem.FlamesOfWarV4, values.gameSystemConfig);

const factionOptions = getFactionOptions();
const battlePlanOptions = getBattlePlanOptions();
const missionOptions = useMissionOptions(gameSystemConfig, details.player0BattlePlan, details.player1BattlePlan);

// TODO: Don't allow winner 'None' for certain outcome types.
Expand Down Expand Up @@ -80,24 +84,22 @@ export const FlamesOfWarV4MatchResultDetailFields = ({
return (
<div className={clsx(styles.FlamesOfWarV4MatchResultDetailFields, className)}>
<div className={styles.FlamesOfWarV4MatchResultDetailFields_Player0Section}>
{/* TODO: AUTO-FILTER OPTIONS TO 1 USING LIST INFO */}
<FormField name="details.player0Faction" label="Faction">
<InputSelect options={getFactionOptions()} />
<FormField name="details.player0Faction" label="Faction" disabled={disableFactions}>
<InputSelect options={factionOptions} />
</FormField>
<FormField name="details.player0BattlePlan" label="Battle Plan">
<InputSelect options={getBattlePlanOptions()} />
<InputSelect options={battlePlanOptions} />
</FormField>
<FormField name="details.player0UnitsLost" label="Units Lost">
<InputText type="number" />
</FormField>
</div>
<div className={styles.FlamesOfWarV4MatchResultDetailFields_Player1Section}>
{/* TODO: AUTO-FILTER OPTIONS TO 1 USING LIST INFO */}
<FormField name="details.player1Faction" label="Faction">
<InputSelect options={getFactionOptions()} />
<FormField name="details.player1Faction" label="Faction" disabled={disableFactions}>
<InputSelect options={factionOptions} />
</FormField>
<FormField name="details.player1BattlePlan" label="Battle Plan">
<InputSelect options={getBattlePlanOptions()} />
<InputSelect options={battlePlanOptions} />
</FormField>
<FormField name="details.player1UnitsLost" label="Units Lost">
<InputText type="number" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ import styles from './TeamYankeeV2MatchResultDetailFields.module.scss';
export interface TeamYankeeV2MatchResultDetailFieldsProps {
className?: string;
playerOptions: InputSelectOption<number>[];
disableFactions?: boolean;
}

export const TeamYankeeV2MatchResultDetailFields = ({
className,
playerOptions,
disableFactions = false,
}: TeamYankeeV2MatchResultDetailFieldsProps): JSX.Element => {
const [showScoreOverride, setShowScoreOverride] = useState<boolean>(false);

Expand All @@ -35,6 +37,8 @@ export const TeamYankeeV2MatchResultDetailFields = ({
const { details } = values;
const gameSystemConfig = validateGameSystemConfig(GameSystem.TeamYankeeV2, values.gameSystemConfig);

const factionOptions = getFactionOptions();
const battlePlanOptions = getBattlePlanOptions();
const missionOptions = useMissionOptions(gameSystemConfig, details.player0BattlePlan, details.player1BattlePlan);

// TODO: Don't allow winner 'None' for certain outcome types.
Expand Down Expand Up @@ -80,24 +84,22 @@ export const TeamYankeeV2MatchResultDetailFields = ({
return (
<div className={clsx(styles.TeamYankeeV2MatchResultDetailFields, className)}>
<div className={styles.TeamYankeeV2MatchResultDetailFields_Player0Section}>
{/* TODO: AUTO-FILTER OPTIONS TO 1 USING LIST INFO */}
<FormField name="details.player0Faction" label="Faction">
<InputSelect options={getFactionOptions()} />
<FormField name="details.player0Faction" label="Faction" disabled={disableFactions}>
<InputSelect options={factionOptions} />
</FormField>
<FormField name="details.player0BattlePlan" label="Battle Plan">
<InputSelect options={getBattlePlanOptions()} />
<InputSelect options={battlePlanOptions} />
</FormField>
<FormField name="details.player0UnitsLost" label="Units Lost">
<InputText type="number" />
</FormField>
</div>
<div className={styles.TeamYankeeV2MatchResultDetailFields_Player1Section}>
{/* TODO: AUTO-FILTER OPTIONS TO 1 USING LIST INFO */}
<FormField name="details.player1Faction" label="Faction">
<InputSelect options={getFactionOptions()} />
<FormField name="details.player1Faction" label="Faction" disabled={disableFactions}>
<InputSelect options={factionOptions} />
</FormField>
<FormField name="details.player1BattlePlan" label="Battle Plan">
<InputSelect options={getBattlePlanOptions()} />
<InputSelect options={battlePlanOptions} />
</FormField>
<FormField name="details.player1UnitsLost" label="Units Lost">
<InputText type="number" />
Expand Down
45 changes: 22 additions & 23 deletions src/components/MatchResultForm/MatchResultForm.schema.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,31 @@
import { DeepPartial } from 'react-hook-form';
import { GameSystem } from '@ianpaschal/combat-command-game-systems/common';
import { GameSystem, getGameSystem } from '@ianpaschal/combat-command-game-systems/common';
import { matchResultDetails as flamesOfWarV4MatchResultDetails } from '@ianpaschal/combat-command-game-systems/flamesOfWarV4';
import { z } from 'zod';

import { MatchResult, UserId } from '~/api';
import { gameSystemConfig, getGameSystemConfigDefaultValues } from '~/components/GameSystemConfigFields';
import { matchResultDetails } from '~/components/MatchResultDetailFields';

export const matchResultFormSchema = z.object({

// Handled by <TournamentPlayersForm /> or <SingleMatchPlayersForm />
player0Placeholder: z.optional(z.string()),
player0UserId: z.optional(z.string().transform((val) => val.length ? val as UserId : undefined)),
player1Placeholder: z.optional(z.string()),
player1UserId: z.optional(z.string().transform((val) => val.length ? val as UserId : undefined)),

// Handled by <MatchResultDetailsForm />
details: matchResultDetails,

// Handled by <GameSystemConfigForm />
gameSystemConfig,

// Non-editable
gameSystem: z.nativeEnum(GameSystem),
playedAt: z.union([z.string(), z.number()]), // TODO: not visible, enable later
});
import { getGameSystemConfigDefaultValues } from '~/components/GameSystemConfigFields';

export const getMatchResultFormSchema = (gameSystem: GameSystem) => {
const { matchResultDetails, gameSystemConfig } = getGameSystem(gameSystem);
return z.object({

// Handled by <TournamentPlayersForm /> or <SingleMatchPlayersForm />
player0Placeholder: z.optional(z.string()),
player0UserId: z.optional(z.string().transform((val) => val.length ? val as UserId : undefined)),
player1Placeholder: z.optional(z.string()),
player1UserId: z.optional(z.string().transform((val) => val.length ? val as UserId : undefined)),

// Non-editable
gameSystem: z.nativeEnum(GameSystem),
playedAt: z.union([z.string(), z.number()]), // TODO: not visible, enable later
}).extend({
details: matchResultDetails.schema,
gameSystemConfig: gameSystemConfig.schema,
});
};

export type MatchResultSubmitData = z.infer<typeof matchResultFormSchema>;
export type MatchResultSubmitData = z.infer<ReturnType<typeof getMatchResultFormSchema>>;

export type MatchResultFormData = DeepPartial<MatchResultSubmitData>;

Expand Down
19 changes: 15 additions & 4 deletions src/components/MatchResultForm/MatchResultForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { Label } from '~/components/generic/Label';
import { Separator } from '~/components/generic/Separator';
import { MatchResultDetailFields } from '~/components/MatchResultDetailFields';
import { MatchResultDetails } from '~/components/MatchResultDetails';
import { toast } from '~/components/ToastProvider';
import { useAsyncState } from '~/hooks/useAsyncState';
import { useCreateMatchResult, useUpdateMatchResult } from '~/services/matchResults';
import { useGetActiveTournamentPairingsByUser } from '~/services/tournamentPairings';
Expand All @@ -36,8 +37,8 @@ import { TournamentPlayerFields } from './components/TournamentPlayerFields';
import { usePlayerDisplayNames } from './MatchResultForm.hooks';
import {
defaultValues,
getMatchResultFormSchema,
MatchResultFormData,
matchResultFormSchema,
} from './MatchResultForm.schema';

import styles from './MatchResultForm.module.scss';
Expand Down Expand Up @@ -109,7 +110,7 @@ export const MatchResultForm = ({
defaultValues: {
...defaultValues,
...(matchResult ? (() => {
const result = matchResultFormSchema.safeParse(matchResult);
const result = getMatchResultFormSchema(matchResult.gameSystem as GameSystem).safeParse(matchResult);
if (!result.success) {
console.error('MatchResultForm schema parsing failed:', result.error);
console.error('MatchResult data:', matchResult);
Expand All @@ -131,7 +132,14 @@ export const MatchResultForm = ({
const selectedGameSystem = form.watch('gameSystem');

const onSubmit: SubmitHandler<MatchResultFormData> = (formData): void => {
const data = validateForm(matchResultFormSchema, formData, form.setError);
if (!formData.gameSystem) {
toast.error('Cannot Submit Match Result', {
description: 'Data could not be validated because game system is not set.',
});
return;
}
const schema = getMatchResultFormSchema(formData.gameSystem);
const data = validateForm(schema, formData, form.setError);
if (data) {
if (matchResult) {
updateMatchResult({ ...data, id: matchResult._id, playedAt: matchResult.playedAt });
Expand Down Expand Up @@ -213,7 +221,10 @@ export const MatchResultForm = ({
</>
)}
{isTournament ? (
<TournamentPlayerFields tournamentPairingId={tournamentPairingId} />
<TournamentPlayerFields
tournament={tournament}
tournamentPairingId={tournamentPairingId}
/>
) : (
<>
<FormField name="gameSystem" label="Game System" disabled={gameSystemOptions.length < 2}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useEffect } from 'react';
import { useEffect, useMemo } from 'react';
import { useFormContext } from 'react-hook-form';

import { TournamentPairingId } from '~/api';
import { Tournament, TournamentPairingId } from '~/api';
import { FormField } from '~/components/generic/Form';
import { InputSelect } from '~/components/generic/InputSelect';
import { InputText } from '~/components/generic/InputText';
Expand All @@ -12,36 +12,62 @@ import styles from './TournamentPlayerFields.module.scss';

export interface TournamentPlayerFieldsProps {
tournamentPairingId: TournamentPairingId;
tournament?: Tournament | null;
}

export const TournamentPlayerFields = ({
tournamentPairingId,
tournament,
}: TournamentPlayerFieldsProps): JSX.Element => {
const { setValue, watch } = useFormContext();
const { player0UserId, player1UserId } = watch();
const player0UserId = watch('player0UserId');
const player1UserId = watch('player1UserId');
const player0Faction = watch('details.player0Faction');
const player1Faction = watch('details.player1Faction');

const { data: selectedPairing } = useGetTournamentPairing({ id: tournamentPairingId });

const player0Label = selectedPairing?.tournamentCompetitor0 ? `${selectedPairing.tournamentCompetitor0.displayName} Player` : 'Player 1';
const player1Label = selectedPairing?.tournamentCompetitor1 ? `${selectedPairing.tournamentCompetitor1.displayName} Player` : 'Bye Placeholder';
const player0Label = tournament?.useTeams && selectedPairing?.tournamentCompetitor0 ? `${selectedPairing.tournamentCompetitor0.displayName} Player` : 'Player 1';
const player1Label = tournament?.useTeams && selectedPairing?.tournamentCompetitor1 ? `${selectedPairing.tournamentCompetitor1.displayName} Player` : 'Player 2';

const player0Options = getCompetitorPlayerOptions(selectedPairing?.tournamentCompetitor0);
const player1Options = getCompetitorPlayerOptions(selectedPairing?.tournamentCompetitor1);
Comment on lines 33 to 34
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

player0Options / player1Options are recomputed every render, causing effects to re-evaluate unnecessarily.

getCompetitorPlayerOptions returns a new array reference on each render, so the useEffect hooks listing these as dependencies will re-run every cycle. The guard conditions prevent infinite loops, but wrapping these in useMemo (as you did for the registrations) would be more efficient and idiomatic.

Proposed fix
-  const player0Options = getCompetitorPlayerOptions(selectedPairing?.tournamentCompetitor0);
-  const player1Options = getCompetitorPlayerOptions(selectedPairing?.tournamentCompetitor1);
+  const player0Options = useMemo(
+    () => getCompetitorPlayerOptions(selectedPairing?.tournamentCompetitor0),
+    [selectedPairing?.tournamentCompetitor0],
+  );
+  const player1Options = useMemo(
+    () => getCompetitorPlayerOptions(selectedPairing?.tournamentCompetitor1),
+    [selectedPairing?.tournamentCompetitor1],
+  );

Also applies to: 44-48

🤖 Prompt for AI Agents
In
`@src/components/MatchResultForm/components/TournamentPlayerFields/TournamentPlayerFields.tsx`
around lines 33 - 34, player0Options and player1Options are being recreated each
render because getCompetitorPlayerOptions returns new arrays, causing dependent
useEffect hooks to re-run; wrap both calls in useMemo using
selectedPairing?.tournamentCompetitor0 and
selectedPairing?.tournamentCompetitor1 as their dependency inputs so the arrays
are stable between renders (similar to how registrations is memoized). Update
the symbols player0Options, player1Options, getCompetitorPlayerOptions and use
the selectedPairing?tournamentCompetitor0/1 dependencies to ensure effects only
run when the underlying competitor changes.


const player0Registration = useMemo(() => (
selectedPairing?.tournamentCompetitor0?.registrations.find((r) => r.userId === player0UserId)
), [selectedPairing?.tournamentCompetitor0, player0UserId]);
const player1Registration = useMemo(() => (
selectedPairing?.tournamentCompetitor1?.registrations.find((r) => r.userId === player1UserId)
), [selectedPairing?.tournamentCompetitor1, player1UserId]);

// Automatically set "Player 1" if possible
useEffect(() => {
if (player0Options && player0Options.length === 1 && player0UserId !== player0Options[0].value) {
setValue('player0UserId', player0Options[0].value);
}
}, [player0Options, player0UserId, setValue]);

// Automatically set "Player 1" faction if possible
useEffect(() => {
if (player0Registration && player0Registration.factions.length > 0 && !player0Faction) {
setValue('details.player0Faction', player0Registration.factions[0]);
}
}, [player0Registration, player0Faction, setValue]);

// Automatically set "Player 2" if possible
useEffect(() => {
if (player1Options && player1Options.length === 1 && player1UserId !== player1Options[0].value) {
setValue('player1UserId', player1Options[0].value);
}
}, [player1Options, player1UserId, setValue]);

// Automatically set "Player 2" faction if possible
useEffect(() => {
if (player1Registration && player1Registration.factions.length > 0 && !player1Faction) {
setValue('details.player1Faction', player1Registration.factions[0]);
}
}, [player1Registration, player1Faction, setValue]);

if (!selectedPairing) {
return <div>Loading...</div>;
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/validateForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const validateForm = <T extends ZodTypeAny, TFieldValues extends FieldVal
result.error.issues.forEach((issue) => {
const fieldPath = issue.path.join('.') as Path<TFieldValues>;
setError(fieldPath, { message: issue.message });
toast.error('Error', { description: issue.message });
toast.error('Error', { description: 'Please check the form for errors.' });
});
}
return result.data;
Expand Down