From fcb5e85b1375e737a8419abcac55219a7da1cf8b Mon Sep 17 00:00:00 2001 From: Jage9 Date: Tue, 24 Feb 2026 18:48:08 -0500 Subject: [PATCH] refactor: complete server-first item schema wiring and plugin contract checks --- client/public/version.js | 2 +- client/src/items/itemPropertyEditor.ts | 8 ++-- client/src/items/itemPropertyPresentation.ts | 3 +- client/src/items/itemRegistry.ts | 48 ++++++++++++------- client/src/main.ts | 2 +- client/src/network/protocol.ts | 13 +++-- client/src/state/gameState.ts | 2 +- docs/protocol-notes.md | 4 +- plans/item-architecture-refactor-plan.md | 5 +- server/app/item_catalog.py | 19 ++++---- server/app/item_service.py | 4 +- server/app/items/types/clock/definition.py | 2 +- server/app/items/types/piano/definition.py | 4 +- .../items/types/radio_station/definition.py | 4 +- server/app/items/types/widget/definition.py | 2 +- server/app/models.py | 6 +-- server/app/server.py | 13 +++-- server/tests/test_item_plugin_contract.py | 32 +++++++++++++ server/tests/test_item_schema_ui.py | 7 +-- server/tests/test_server_message_handling.py | 21 ++++++++ 20 files changed, 132 insertions(+), 69 deletions(-) create mode 100644 server/tests/test_item_plugin_contract.py diff --git a/client/public/version.js b/client/public/version.js index 6164bd4..390a48c 100644 --- a/client/public/version.js +++ b/client/public/version.js @@ -1,5 +1,5 @@ // Maintainer-controlled web client version. // Format: YYYY.MM.DD Rn (example: 2026.02.20 R2) -window.CHGRID_WEB_VERSION = "2026.02.24 R227"; +window.CHGRID_WEB_VERSION = "2026.02.24 R228"; // Optional display timezone for timestamps. Falls back to America/Detroit if unset/invalid. window.CHGRID_TIME_ZONE = "America/Detroit"; diff --git a/client/src/items/itemPropertyEditor.ts b/client/src/items/itemPropertyEditor.ts index 6037e7e..60572a4 100644 --- a/client/src/items/itemPropertyEditor.ts +++ b/client/src/items/itemPropertyEditor.ts @@ -23,7 +23,7 @@ type EditorDeps = { getItemPropertyValue: (item: WorldItem, key: string) => string; itemPropertyLabel: (key: string) => string; isItemPropertyEditable: (item: WorldItem, key: string) => boolean; - getItemPropertyOptionValues: (key: string) => string[] | undefined; + getItemPropertyOptionValues: (itemType: WorldItem['type'], key: string) => string[] | undefined; openItemPropertyOptionSelect: (item: WorldItem, key: string) => void; describeItemPropertyHelp: (item: WorldItem, key: string) => string; getItemPropertyMetadata: ( @@ -98,7 +98,7 @@ export function createItemPropertyEditor(deps: EditorDeps): { deps.sfxUiCancel(); return; } - const options = deps.getItemPropertyOptionValues(selectedKey); + const options = deps.getItemPropertyOptionValues(item.type, selectedKey); if (options && options.length > 0) { const currentRaw = String(item.params[selectedKey] ?? '').trim().toLowerCase(); const currentIndex = Math.max( @@ -177,7 +177,7 @@ export function createItemPropertyEditor(deps: EditorDeps): { deps.sfxUiBlip(); return; } - if (deps.getItemPropertyOptionValues(selectedKey)) { + if (deps.getItemPropertyOptionValues(item.type, selectedKey)) { deps.openItemPropertyOptionSelect(item, selectedKey); return; } @@ -306,7 +306,7 @@ export function createItemPropertyEditor(deps: EditorDeps): { } else if (valueType === 'number') { if (!submitNumericParam(propertyKey)) return; } else if (valueType === 'list') { - const options = deps.getItemPropertyOptionValues(propertyKey) ?? []; + const options = deps.getItemPropertyOptionValues(item.type, propertyKey) ?? []; if (options.length === 0) { deps.updateStatus(`${deps.itemPropertyLabel(propertyKey)} has no options.`); deps.sfxUiCancel(); diff --git a/client/src/items/itemPropertyPresentation.ts b/client/src/items/itemPropertyPresentation.ts index e581a80..9548429 100644 --- a/client/src/items/itemPropertyPresentation.ts +++ b/client/src/items/itemPropertyPresentation.ts @@ -153,7 +153,7 @@ export function createItemPropertyPresentation(deps: PresentationDeps): { const stepText = metadata.range.step !== undefined ? ` step ${metadata.range.step}` : ''; parts.push(`Range: ${metadata.range.min} to ${metadata.range.max}${stepText}.`); } else { - const options = getItemPropertyOptionValues(key); + const options = getItemPropertyOptionValues(item.type, key); if (options && options.length > 0) { parts.push(`Options: ${options.join(', ')}.`); } @@ -205,4 +205,3 @@ export function createItemPropertyPresentation(deps: PresentationDeps): { validateNumericItemPropertyInput, }; } - diff --git a/client/src/items/itemRegistry.ts b/client/src/items/itemRegistry.ts index fefc5b4..5b054e2 100644 --- a/client/src/items/itemRegistry.ts +++ b/client/src/items/itemRegistry.ts @@ -4,8 +4,10 @@ export type ItemPropertyValueType = 'boolean' | 'text' | 'number' | 'list' | 'so export type ItemPropertyMetadata = { valueType?: ItemPropertyValueType; + label?: string; tooltip?: string; maxLength?: number; + options?: string[]; visibleWhen?: Record; range?: { min: number; @@ -20,8 +22,8 @@ type UiDefinitionsPayload = { type: ItemType; label?: string; tooltip?: string; + capabilities?: string[]; editableProperties?: string[]; - propertyOptions?: Record; propertyMetadata?: Record; globalProperties?: Record; }>; @@ -30,8 +32,8 @@ let itemTypeSequence: ItemType[] = []; let itemTypeLabels: Partial> = {}; let itemTypeTooltips: Partial> = {}; let itemTypeEditableProperties: Partial> = {}; +let itemTypeCapabilities: Partial> = {}; let itemTypeGlobalProperties: Partial>> = {}; -let optionItemPropertyValues: Partial> = {}; let itemTypePropertyMetadata: Partial>> = {}; export let EDITABLE_ITEM_PROPERTY_KEYS = new Set( @@ -54,6 +56,9 @@ function normalizePropertyMetadataRecord(raw: Record | undefine if (valueObj.valueType === 'boolean' || valueObj.valueType === 'text' || valueObj.valueType === 'number' || valueObj.valueType === 'list' || valueObj.valueType === 'sound') { metadata.valueType = valueObj.valueType; } + if (typeof valueObj.label === 'string' && valueObj.label.trim().length > 0) { + metadata.label = valueObj.label.trim(); + } if (typeof valueObj.tooltip === 'string' && valueObj.tooltip.trim().length > 0) { metadata.tooltip = valueObj.tooltip.trim(); } @@ -63,6 +68,12 @@ function normalizePropertyMetadataRecord(raw: Record | undefine metadata.maxLength = Math.floor(maxLength); } } + if (Array.isArray(valueObj.options)) { + const options = valueObj.options.filter((entry): entry is string => typeof entry === 'string' && entry.trim().length > 0); + if (options.length > 0) { + metadata.options = options; + } + } if (valueObj.visibleWhen && typeof valueObj.visibleWhen === 'object') { const visibleWhen: Record = {}; for (const [conditionKey, conditionValue] of Object.entries(valueObj.visibleWhen as Record)) { @@ -95,7 +106,7 @@ function normalizePropertyMetadataRecord(raw: Record | undefine /** Returns current timezone option list used by clock item properties. */ export function getClockTimeZoneOptions(): string[] { - return [...(optionItemPropertyValues.timeZone ?? [])]; + return [...(getItemPropertyMetadata('clock', 'timeZone')?.options ?? [])]; } /** Returns default timezone used by clock items when no override is set. */ @@ -124,8 +135,8 @@ export function getItemPropertyMetadata(itemType: ItemType, key: string): ItemPr } /** Returns option-list values for list-based properties, if defined. */ -export function getItemPropertyOptionValues(key: string): string[] | undefined { - return optionItemPropertyValues[key]; +export function getItemPropertyOptionValues(itemType: ItemType, key: string): string[] | undefined { + return itemTypePropertyMetadata[itemType]?.[key]?.options; } /** Returns human-facing label for an item type. */ @@ -133,8 +144,17 @@ export function itemTypeLabel(type: ItemType): string { return itemTypeLabels[type] ?? type; } +/** Returns server-defined capabilities for one item type, if provided. */ +export function getItemTypeCapabilities(itemType: ItemType): string[] { + return [...(itemTypeCapabilities[itemType] ?? [])]; +} + /** Returns human-facing label for a property key. */ export function itemPropertyLabel(key: string): string { + const metadataLabel = Object.values(itemTypePropertyMetadata) + .map((entry) => entry?.[key]?.label) + .find((label) => typeof label === 'string' && label.trim().length > 0); + if (metadataLabel) return metadataLabel; if (key === 'use24Hour') return 'use 24 hour format'; if (key === 'emitRange') return 'emit range'; if (key === 'mediaVolume') return 'media volume'; @@ -218,8 +238,8 @@ export function applyServerItemUiDefinitions(uiDefinitions: UiDefinitionsPayload itemTypeLabels = {}; itemTypeTooltips = {}; itemTypeEditableProperties = {}; + itemTypeCapabilities = {}; itemTypeGlobalProperties = {}; - optionItemPropertyValues = {}; itemTypePropertyMetadata = {}; rebuildEditablePropertyKeySet(); return false; @@ -233,8 +253,8 @@ export function applyServerItemUiDefinitions(uiDefinitions: UiDefinitionsPayload const nextLabels: Partial> = {}; const nextTooltips: Partial> = {}; const nextEditable: Partial> = {}; + const nextCapabilities: Partial> = {}; const nextGlobals: Partial>> = {}; - const nextOptions: Partial> = {}; const nextPropertyMetadata: Partial>> = {}; for (const definition of uiDefinitions.itemTypes) { @@ -249,6 +269,9 @@ export function applyServerItemUiDefinitions(uiDefinitions: UiDefinitionsPayload if (Array.isArray(definition.editableProperties) && definition.editableProperties.length > 0) { nextEditable[itemType] = definition.editableProperties.filter((entry) => typeof entry === 'string'); } + if (Array.isArray(definition.capabilities) && definition.capabilities.length > 0) { + nextCapabilities[itemType] = definition.capabilities.filter((entry) => typeof entry === 'string'); + } if (definition.propertyMetadata && typeof definition.propertyMetadata === 'object') { nextPropertyMetadata[itemType] = normalizePropertyMetadataRecord(definition.propertyMetadata); } @@ -261,15 +284,6 @@ export function applyServerItemUiDefinitions(uiDefinitions: UiDefinitionsPayload } nextGlobals[itemType] = normalized; } - if (definition.propertyOptions && typeof definition.propertyOptions === 'object') { - for (const [propertyKey, values] of Object.entries(definition.propertyOptions)) { - if (!Array.isArray(values) || values.length === 0) continue; - const normalizedValues = values.filter((entry) => typeof entry === 'string'); - if (normalizedValues.length > 0) { - nextOptions[propertyKey] = normalizedValues; - } - } - } } const discoveredOrder: ItemType[] = []; @@ -281,8 +295,8 @@ export function applyServerItemUiDefinitions(uiDefinitions: UiDefinitionsPayload itemTypeLabels = nextLabels; itemTypeTooltips = nextTooltips; itemTypeEditableProperties = nextEditable; + itemTypeCapabilities = nextCapabilities; itemTypeGlobalProperties = nextGlobals; - optionItemPropertyValues = nextOptions; itemTypePropertyMetadata = nextPropertyMetadata; itemTypeSequence = explicitOrder ?? discoveredOrder; rebuildEditablePropertyKeySet(); diff --git a/client/src/main.ts b/client/src/main.ts index 59d94fb..c2562a5 100644 --- a/client/src/main.ts +++ b/client/src/main.ts @@ -886,7 +886,7 @@ function useItem(item: WorldItem): void { /** Opens option-list selection mode for list-based item properties. */ function openItemPropertyOptionSelect(item: WorldItem, key: string): void { - const options = getItemPropertyOptionValues(key); + const options = getItemPropertyOptionValues(item.type, key); if (!options || options.length === 0) { return; } diff --git a/client/src/network/protocol.ts b/client/src/network/protocol.ts index 6b45a6a..02919cd 100644 --- a/client/src/network/protocol.ts +++ b/client/src/network/protocol.ts @@ -2,7 +2,7 @@ import { z } from 'zod'; export const itemSchema = z.object({ id: z.string(), - type: z.enum(['radio_station', 'dice', 'wheel', 'clock', 'widget', 'piano']), + type: z.string().min(1), title: z.string(), x: z.number().int(), y: z.number().int(), @@ -42,21 +42,24 @@ export const welcomeMessageSchema = z.object({ .optional(), uiDefinitions: z .object({ - itemTypeOrder: z.array(z.enum(['radio_station', 'dice', 'wheel', 'clock', 'widget', 'piano'])), + itemTypeOrder: z.array(z.string().min(1)), itemTypes: z.array( z.object({ - type: z.enum(['radio_station', 'dice', 'wheel', 'clock', 'widget', 'piano']), + type: z.string().min(1), label: z.string().optional(), tooltip: z.string().optional(), editableProperties: z.array(z.string()), - propertyOptions: z.record(z.string(), z.array(z.string())).optional(), + capabilities: z.array(z.string()).optional(), propertyMetadata: z .record( z.string(), z.object({ valueType: z.enum(['boolean', 'text', 'number', 'list', 'sound']).optional(), + label: z.string().optional(), tooltip: z.string().optional(), maxLength: z.number().int().positive().optional(), + options: z.array(z.string()).optional(), + visibleWhen: z.record(z.string(), z.union([z.string(), z.number(), z.boolean()])).optional(), range: z .object({ min: z.number(), @@ -193,7 +196,7 @@ export type OutgoingMessage = | { type: 'update_nickname'; nickname: string } | { type: 'chat_message'; message: string } | { type: 'ping'; clientSentAt: number } - | { type: 'item_add'; itemType: 'radio_station' | 'dice' | 'wheel' | 'clock' | 'widget' | 'piano' } + | { type: 'item_add'; itemType: string } | { type: 'item_pickup'; itemId: string } | { type: 'item_drop'; itemId: string; x: number; y: number } | { type: 'item_delete'; itemId: string } diff --git a/client/src/state/gameState.ts b/client/src/state/gameState.ts index dc55a5f..152aea6 100644 --- a/client/src/state/gameState.ts +++ b/client/src/state/gameState.ts @@ -2,7 +2,7 @@ export const GRID_SIZE = 41; export const HEARING_RADIUS = 20; export const MOVE_COOLDOWN_MS = 200; -export type ItemType = 'radio_station' | 'dice' | 'wheel' | 'clock' | 'widget' | 'piano'; +export type ItemType = string; export type WorldItem = { id: string; diff --git a/docs/protocol-notes.md b/docs/protocol-notes.md index db71ce4..fe128f9 100644 --- a/docs/protocol-notes.md +++ b/docs/protocol-notes.md @@ -51,9 +51,9 @@ This is a behavior guide for packet semantics beyond raw schemas. - `welcome.uiDefinitions`: server-provided item UI definitions: - `itemTypeOrder`: add-item menu order - `itemTypes[].tooltip`: item-level tooltip/help text + - `itemTypes[].capabilities`: server-declared actions supported by the type - `itemTypes[].editableProperties`: editable property keys by item type - - `itemTypes[].propertyOptions`: menu options for property keys (for example clock `timeZone`) - - `itemTypes[].propertyMetadata`: property-level metadata (`valueType`, optional `range`, optional `tooltip`, optional `maxLength`, optional `visibleWhen`) + - `itemTypes[].propertyMetadata`: property-level metadata (`valueType`, optional `label`, optional `range`, optional `tooltip`, optional `maxLength`, optional `options`, optional `visibleWhen`) - `itemTypes[].globalProperties`: non-editable global values (`useSound`, `emitSound`, `useCooldownMs`, `emitRange`, `directional`, `emitSoundSpeed`, `emitSoundTempo`) - Client item UI requires this metadata from the server; there is no fallback item definition map. diff --git a/plans/item-architecture-refactor-plan.md b/plans/item-architecture-refactor-plan.md index d19c2e8..cf2d59d 100644 --- a/plans/item-architecture-refactor-plan.md +++ b/plans/item-architecture-refactor-plan.md @@ -338,7 +338,7 @@ Implementation: - Keep compile-time unions only where they materially improve safety and can be generated/centralized. Acceptance criteria: - Adding a new item plugin does not require editing multiple type-literal lists in unrelated files. -- New type appears in add/edit flows after server metadata update with minimal client changes (or none for generic items). +- New type appears in add/edit flows after server metadata update with minimal client changes. 3. **Include `capabilities` in `welcome.uiDefinitions.itemTypes` (medium).** Scope: @@ -360,8 +360,7 @@ Problem today: Implementation: - Server emits list options at `propertyMetadata[key].options`. - Client reads options from metadata first. -- Deprecate/remove separate `propertyOptions` map after migration. -- Keep transition shim only during one deploy window if needed. +- Remove separate `propertyOptions` map. Acceptance criteria: - One canonical place (`propertyMetadata`) defines type, range, tooltip, options, and visibility for each property. - New list property requires server-only metadata changes for options. diff --git a/server/app/item_catalog.py b/server/app/item_catalog.py index 00cbfce..e274107 100644 --- a/server/app/item_catalog.py +++ b/server/app/item_catalog.py @@ -3,11 +3,11 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Literal, cast +from typing import TypeAlias, cast from .items.registry import ITEM_MODULES, ITEM_TYPE_ORDER -ItemType = Literal["radio_station", "dice", "wheel", "clock", "widget", "piano"] +ItemType: TypeAlias = str ITEM_TYPE_SEQUENCE: tuple[ItemType, ...] = cast(tuple[ItemType, ...], ITEM_TYPE_ORDER) ITEM_TYPE_LABELS: dict[ItemType, str] = {item_type: ITEM_MODULES[item_type].LABEL for item_type in ITEM_TYPE_SEQUENCE} ITEM_TYPE_EDITABLE_PROPERTIES: dict[ItemType, tuple[str, ...]] = { @@ -75,15 +75,6 @@ ITEM_DEFINITIONS: dict[ItemType, ItemDefinition] = { for item_type in ITEM_TYPE_SEQUENCE } -ITEM_PROPERTY_OPTIONS: dict[str, tuple[str, ...]] = { - "mediaEffect": RADIO_EFFECT_OPTIONS, - "emitEffect": RADIO_EFFECT_OPTIONS, - "mediaChannel": RADIO_CHANNEL_OPTIONS, - "timeZone": CLOCK_TIME_ZONE_OPTIONS, - "instrument": PIANO_INSTRUMENT_OPTIONS, - "voiceMode": PIANO_VOICE_MODE_OPTIONS, -} - ITEM_TYPE_TOOLTIPS: dict[ItemType, str] = { item_type: ITEM_MODULES[item_type].TOOLTIP for item_type in ITEM_TYPE_SEQUENCE } @@ -125,6 +116,12 @@ def get_item_definition(item_type: ItemType) -> ItemDefinition: return ITEM_DEFINITIONS[item_type] +def is_known_item_type(item_type: str) -> bool: + """Return whether a string item type id exists in discovered plugins.""" + + return item_type in ITEM_DEFINITIONS + + def get_item_use_cooldown_ms(item_type: ItemType) -> int: """Return validated global use cooldown in milliseconds for an item type.""" diff --git a/server/app/item_service.py b/server/app/item_service.py index 9ae7488..91f4b74 100644 --- a/server/app/item_service.py +++ b/server/app/item_service.py @@ -8,8 +8,6 @@ import time import uuid from copy import deepcopy from pathlib import Path -from typing import Literal - from .client import ClientConnection from .item_catalog import get_item_definition from .models import PersistedWorldItem, WorldItem @@ -36,7 +34,7 @@ class ItemService: return int(time.time() * 1000) - def default_item(self, client: ClientConnection, item_type: Literal["radio_station", "dice", "wheel", "clock", "widget", "piano"]) -> WorldItem: + def default_item(self, client: ClientConnection, item_type: str) -> WorldItem: """Create a new server-authoritative item at the caller's position.""" item_def = get_item_definition(item_type) diff --git a/server/app/items/types/clock/definition.py b/server/app/items/types/clock/definition.py index cbd8009..2e30c2d 100644 --- a/server/app/items/types/clock/definition.py +++ b/server/app/items/types/clock/definition.py @@ -60,6 +60,6 @@ PARAM_KEYS: tuple[str, ...] = ("timeZone", "use24Hour") PROPERTY_METADATA: dict[str, dict[str, object]] = { "title": {"valueType": "text", "tooltip": "Display name spoken and shown for this item.", "maxLength": 80}, - "timeZone": {"valueType": "list", "tooltip": "Timezone used when the clock speaks time."}, + "timeZone": {"valueType": "list", "tooltip": "Timezone used when the clock speaks time.", "options": list(TIME_ZONE_OPTIONS)}, "use24Hour": {"valueType": "boolean", "tooltip": "Use 24 hour format instead of AM/PM."}, } diff --git a/server/app/items/types/piano/definition.py b/server/app/items/types/piano/definition.py index 04fe432..402e424 100644 --- a/server/app/items/types/piano/definition.py +++ b/server/app/items/types/piano/definition.py @@ -64,8 +64,8 @@ DEFAULT_ENVELOPE_BY_INSTRUMENT: dict[str, tuple[int, int, int, int, str, int]] = PROPERTY_METADATA: dict[str, dict[str, object]] = { "title": {"valueType": "text", "tooltip": "Display name spoken and shown for this item.", "maxLength": 80}, - "instrument": {"valueType": "list", "tooltip": "Instrument voice used when playing this piano."}, - "voiceMode": {"valueType": "list", "tooltip": "Mono plays one note at a time; poly allows chords."}, + "instrument": {"valueType": "list", "tooltip": "Instrument voice used when playing this piano.", "options": list(INSTRUMENT_OPTIONS)}, + "voiceMode": {"valueType": "list", "tooltip": "Mono plays one note at a time; poly allows chords.", "options": list(VOICE_MODE_OPTIONS)}, "octave": { "valueType": "number", "tooltip": "Shifts played notes in octaves. -1 is one octave down.", diff --git a/server/app/items/types/radio_station/definition.py b/server/app/items/types/radio_station/definition.py index 24b8c55..8662e9c 100644 --- a/server/app/items/types/radio_station/definition.py +++ b/server/app/items/types/radio_station/definition.py @@ -55,8 +55,8 @@ PROPERTY_METADATA: dict[str, dict[str, object]] = { "tooltip": "Playback media volume percent for this radio.", "range": {"min": 0, "max": 100, "step": 1}, }, - "mediaChannel": {"valueType": "list", "tooltip": "Select how the station audio channels are rendered."}, - "mediaEffect": {"valueType": "list", "tooltip": "Select the active radio effect."}, + "mediaChannel": {"valueType": "list", "tooltip": "Select how the station audio channels are rendered.", "options": list(CHANNEL_OPTIONS)}, + "mediaEffect": {"valueType": "list", "tooltip": "Select the active radio effect.", "options": list(EFFECT_OPTIONS)}, "mediaEffectValue": { "valueType": "number", "tooltip": "Amount for the selected effect.", diff --git a/server/app/items/types/widget/definition.py b/server/app/items/types/widget/definition.py index 087252e..929bf08 100644 --- a/server/app/items/types/widget/definition.py +++ b/server/app/items/types/widget/definition.py @@ -83,7 +83,7 @@ PROPERTY_METADATA: dict[str, dict[str, object]] = { "tooltip": "Playback tempo percent for emitted sound. 50 is normal, 0 is half, 100 is double. Using speed and tempo together may sound weird.", "range": {"min": 0, "max": 100, "step": 0.1}, }, - "emitEffect": {"valueType": "list", "tooltip": "Effect applied to emitted sound."}, + "emitEffect": {"valueType": "list", "tooltip": "Effect applied to emitted sound.", "options": list(EFFECT_OPTIONS)}, "emitEffectValue": { "valueType": "number", "tooltip": "Amount for emit effect.", diff --git a/server/app/models.py b/server/app/models.py index f40c245..a6b601b 100644 --- a/server/app/models.py +++ b/server/app/models.py @@ -42,7 +42,7 @@ class PingPacket(BasePacket): class ItemAddPacket(BasePacket): type: Literal["item_add"] - itemType: Literal["radio_station", "dice", "wheel", "clock", "widget", "piano"] + itemType: str = Field(min_length=1) class ItemPickupPacket(BasePacket): @@ -173,7 +173,7 @@ class NicknameResultPacket(BasePacket): class WorldItem(BaseModel): id: str - type: Literal["radio_station", "dice", "wheel", "clock", "widget", "piano"] + type: str = Field(min_length=1) title: str x: int y: int @@ -191,7 +191,7 @@ class WorldItem(BaseModel): class PersistedWorldItem(BaseModel): model_config = ConfigDict(extra="ignore") id: str - type: Literal["radio_station", "dice", "wheel", "clock", "widget", "piano"] + type: str = Field(min_length=1) title: str x: int y: int diff --git a/server/app/server.py b/server/app/server.py index a23d6b5..515dee3 100644 --- a/server/app/server.py +++ b/server/app/server.py @@ -25,14 +25,15 @@ from .config import load_config from .item_catalog import ( CLOCK_DEFAULT_TIME_ZONE, CLOCK_TIME_ZONE_OPTIONS, - ITEM_PROPERTY_OPTIONS, ITEM_TYPE_EDITABLE_PROPERTIES, ITEM_TYPE_LABELS, ITEM_TYPE_PROPERTY_METADATA, ITEM_TYPE_SEQUENCE, ITEM_TYPE_TOOLTIPS, + get_item_definition, get_item_global_properties, get_item_use_cooldown_ms, + is_known_item_type, ) from .item_type_handlers import get_item_type_handler from .item_service import ItemService @@ -708,18 +709,13 @@ class SignalingServer: item_types: list[dict] = [] for item_type in ITEM_TYPE_SEQUENCE: editable = list(ITEM_TYPE_EDITABLE_PROPERTIES.get(item_type, ("title",))) - property_options: dict[str, list[str]] = {} - for key in editable: - options = ITEM_PROPERTY_OPTIONS.get(key) - if options: - property_options[key] = list(options) item_types.append( { "type": item_type, "label": ITEM_TYPE_LABELS.get(item_type, item_type), "tooltip": ITEM_TYPE_TOOLTIPS.get(item_type), + "capabilities": list(get_item_definition(item_type).capabilities), "editableProperties": editable, - "propertyOptions": property_options, "propertyMetadata": ITEM_TYPE_PROPERTY_METADATA.get(item_type, {}), "globalProperties": get_item_global_properties(item_type), } @@ -897,6 +893,9 @@ class SignalingServer: return if isinstance(packet, ItemAddPacket): + if not is_known_item_type(packet.itemType): + await self._send_item_result(client, False, "add", "Unknown item type.") + return item = self.item_service.default_item(client, packet.itemType) self.item_service.add_item(item) await self._broadcast_item(item) diff --git a/server/tests/test_item_plugin_contract.py b/server/tests/test_item_plugin_contract.py new file mode 100644 index 0000000..9bc3f4a --- /dev/null +++ b/server/tests/test_item_plugin_contract.py @@ -0,0 +1,32 @@ +from __future__ import annotations + +from pathlib import Path + +from app.items.registry import ITEM_PLUGINS + + +def test_item_plugins_expose_expected_contract() -> None: + for plugin in ITEM_PLUGINS: + module = plugin.module + assert isinstance(plugin.type, str) and plugin.type + assert isinstance(plugin.order, int) + assert callable(getattr(module, "validate_update", None)) + assert callable(getattr(module, "use_item", None)) + assert isinstance(getattr(module, "LABEL", ""), str) + assert isinstance(getattr(module, "TOOLTIP", ""), str) + assert isinstance(getattr(module, "EDITABLE_PROPERTIES", ()), tuple) + assert isinstance(getattr(module, "CAPABILITIES", ()), tuple) + assert isinstance(getattr(module, "DEFAULT_PARAMS", {}), dict) + assert isinstance(getattr(module, "PROPERTY_METADATA", {}), dict) + + +def test_item_plugin_folders_have_required_files() -> None: + base_dir = Path(__file__).resolve().parents[1] / "app" / "items" / "types" + for plugin in ITEM_PLUGINS: + type_dir = base_dir / plugin.type + assert type_dir.is_dir() + assert (type_dir / "definition.py").is_file() + assert (type_dir / "validator.py").is_file() + assert (type_dir / "actions.py").is_file() + assert (type_dir / "module.py").is_file() + assert (type_dir / "plugin.py").is_file() diff --git a/server/tests/test_item_schema_ui.py b/server/tests/test_item_schema_ui.py index 22ccb71..4234bd2 100644 --- a/server/tests/test_item_schema_ui.py +++ b/server/tests/test_item_schema_ui.py @@ -33,15 +33,16 @@ def test_ui_definitions_are_complete_for_all_item_types() -> None: assert isinstance(entry.get("type"), str) assert isinstance(entry.get("label"), str) assert isinstance(entry.get("editableProperties"), list) + assert isinstance(entry.get("capabilities"), list) assert isinstance(entry.get("propertyMetadata"), dict) - assert isinstance(entry.get("propertyOptions"), dict) assert isinstance(entry.get("globalProperties"), dict) editable_properties = entry["editableProperties"] + capabilities = entry["capabilities"] property_metadata = entry["propertyMetadata"] - property_options = entry["propertyOptions"] global_properties = entry["globalProperties"] + assert capabilities assert required_global_keys.issubset(set(global_properties.keys())) for property_key in editable_properties: if property_key == "title": @@ -50,7 +51,7 @@ def test_ui_definitions_are_complete_for_all_item_types() -> None: metadata = property_metadata[property_key] assert isinstance(metadata, dict) if metadata.get("valueType") == "list": - options = property_options.get(property_key) + options = metadata.get("options") assert isinstance(options, list) assert options diff --git a/server/tests/test_server_message_handling.py b/server/tests/test_server_message_handling.py index 652d466..453843e 100644 --- a/server/tests/test_server_message_handling.py +++ b/server/tests/test_server_message_handling.py @@ -83,3 +83,24 @@ async def test_broadcast_fanout_is_concurrent(monkeypatch: pytest.MonkeyPatch) - assert ws1 in send_started_at assert ws2 in send_started_at assert abs(send_started_at[ws1] - send_started_at[ws2]) < 0.02 + + +@pytest.mark.asyncio +async def test_item_add_rejects_unknown_type(monkeypatch: pytest.MonkeyPatch) -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = _fake_ws() + client = ClientConnection(websocket=ws, id="u1", nickname="tester", x=5, y=6) + server.clients[ws] = client + + send_payloads: list[object] = [] + + async def fake_send(websocket: ServerConnection, packet: object) -> None: + send_payloads.append(packet) + + monkeypatch.setattr(server, "_send", fake_send) + + await server._handle_message(client, json.dumps({"type": "item_add", "itemType": "not_a_type"})) + + assert send_payloads + assert send_payloads[-1].ok is False + assert "unknown item type" in send_payloads[-1].message.lower()