Code review of Task 2 (commit 07bbc0c7) found two real issues:
1. The GetGuestRoomResultEvent handler did not handle parser.roomEnter,
so after the consumer migration (Tasks 5-8) a successful room entry
would no longer dismiss the door dialog. Fix: reset to INITIAL when
parser.roomEnter is true, before the roomForward branch.
2. The test suite was order-dependent — the useBetween singleton
persisted state across tests, so 'exposes the initial NONE snapshot'
passed only because it ran first. Fix: beforeEach renders the hook
once, calls reset(), then unmounts; afterEach calls cleanup().
Plus one new test case verifying the roomEnter -> reset behavior.
- Add `src/hooks/rooms/widgets/useDoorState.ts`: useBetween-based
singleton wrapping DoorbellMessageEvent / RoomDoorbellAcceptedEvent /
FlatAccessDeniedMessageEvent / GenericErrorEvent /
GetGuestRoomResultEvent; all 5 handlers wrapped in useCallback([])
so their references are stable across useBetween tick() calls and
the effect dep-array never triggers re-registration.
- Add `src/hooks/rooms/widgets/useDoorState.test.tsx`: 11-case Vitest
suite (initial state, 5 event transitions, 2 no-op guards,
GetGuestRoomResultEvent doorbell/password paths, reset()).
- Extend `src/nitro-renderer.mock.ts`: new MessageEvent base class with
callBack/type/getParser; DoorbellMessageEvent / RoomDoorbellAcceptedEvent /
FlatAccessDeniedMessageEvent / GenericErrorEvent / GetGuestRoomResultEvent
concrete stubs; RoomDataParser.DOORBELL_STATE + PASSWORD_STATE; separate
msgListeners map (cleared independently of NitroEvent listeners so
useBetween subscriptions survive between test cases); WeakMap wrapper
for correct removeMessageEvent; GetCommunication routes to msgListeners.
All 11 useDoorState tests pass; full suite 453/456 (3 pre-existing
FloorplanCanvasSVG jsdom/SVG-CTM failures unrelated to this task).
Complete modernization of the floor-plan editor. Three layered
changes shipped together since they share state shapes and the
test infrastructure stubs.
1) React rewrite (state + hooks + views + tests)
Drops the FloorplanEditorContext singleton + legacy view
components and replaces them with a pure-React reducer
architecture:
- state/ — typed FloorplanState + FloorplanAction union,
pure reducer covering PAINT_TILE / ERASE_TILE /
ADJUST_HEIGHT / SET_DOOR / SET_DOOR_DIR / SET_THICKNESS /
SET_WALL_HEIGHT / BRUSH_SET / SELECT_RECT / SELECT_ALL /
CLEAR_SELECTION / SQUARE_SELECT_TOGGLE / IMPORT_STRING /
APPLY_REMOTE_DIFF / APPLY_REMOTE_SNAPSHOT. Source-tagged
('local' | 'remote') so the editor can distinguish user
edits from server pushes. Co-located encoding helpers
(parseTilemap / serializeTilemap) and area-counter
selectors.
- hooks/ — useFloorplanReducer (wraps useReducer with a
history stack + loadFromServer + undo/redo), useTool
(pointer events -> dispatch), usePointerToTile (screen
-> tile projection that respects the viewBox origin so
pan/zoom stays accurate).
- views/ — FloorplanCanvasSVG, FloorplanHeightPicker,
FloorplanToolbar, FloorplanOptionsPanel,
FloorplanImportExport, FloorplanTile,
FloorplanPreviewSVG (alternative iso preview kept as a
fallback view, not wired into the main layout).
- Co-located Vitest suites for every module above (encoding,
reducer, selectors, hooks, views, integration). 100+ new
test cases.
2) Live in-room preview (NEW capability)
useFloorplanLiveSync drives client-side preview of the edit
directly into the active room — every tile / door / wall
height / thickness change is applied through
GetRoomMessageHandler().applyFloorModelLocally (new public
method on the renderer, see paired renderer PR) with
zero server traffic during editing. The wire
UpdateFloorPropertiesMessageComposer is only sent when the
user explicitly clicks Save. Thickness slider additionally
calls RoomEngine.updateRoomInstancePlaneThickness for
zero-latency wall/floor-depth feedback while dragging.
Toggle 'Live preview ON / OFF' in the bottom strip (default
ON) lets the user opt out if they want to keep changes
contained to the editor's own preview until Save.
Revert button re-applies the original snapshot locally so
the room snaps back to where it was when the editor opened.
3) UX polish
- Undo / Redo (Ctrl+Z, Ctrl+Shift+Z / Ctrl+Y) backed by a
100-step history stack inside useFloorplanReducer. Local
mutating actions push history; brush/selection UI bumps
and remote dispatches bypass it; loadFromServer wipes the
stack.
- Zoom 40-600 % with Ctrl+wheel, +/- buttons, % label.
Shift+drag or middle-mouse drag pans the canvas.
- Auto-fit on first paint: computes the screen-space
bounding box of the painted (non-blocked) tiles, picks the
zoom that just contains them with a 5 % margin, pans so
the room sits in the viewport centre. Default view is now
'room fills the canvas' instead of 'room is a dot at the
top-centre of a huge empty canvas'. Clicking the % label
re-runs the fit; crosshair button keeps zoom and recentres
the pan only.
- Door direction control: arrows + door icon triplet
(8-way rotate by single click on prev/next, full cycle
forward on the icon itself). Wall and floor thickness
collapse from two 4-button rows into two compact
segmented selectors (active state in emerald). Saves
significant horizontal space.
- Habbo floor pattern tile (~186 B PNG, vendored from
habbofurni.com/images/furni_floor.png) tiled as the
canvas background with image-rendering: pixelated so the
texture stays crisp at every zoom level. Replaces the
solid black background.
Test infrastructure
nitro-renderer.mock grows constructors / proxies / functions
for everything the new floor-editor tests transitively
import (floor composers + events, RoomEngineEvent,
ILinkEventTracker, convertNumbersForSaving /
convertSettingToNumber, GetRoomMessageHandler,
GetTicker, GetRenderer, NitroTicker, RoomPreviewer with a
sufficiently real .updatePreviewModel / dispose surface,
and a TextureUtils.createRenderTexture that returns an
object with a no-op .destroy). test-setup adds a no-op
ResizeObserver polyfill (jsdom doesn't ship one and the
optional FloorplanRoomPreview observes its container) and
a draggable-windows-container portal root for tests that
mount NitroCardView.
Files: 44 changed (mostly new). yarn typecheck 0 errors,
yarn test 341/341 green.
Root cause of last session's "(intermediate value)() is undefined" at
ToolbarView.tsx:46:
use-between 1.x ships its own React-dispatcher proxy (ownDispatcher
in node_modules/use-between/release/index.esm.js:54-169) that
re-implements only useState, useReducer, useEffect, useLayoutEffect,
useCallback, useMemo, useRef and useImperativeHandle. It does NOT
implement useSyncExternalStore. When the inner state function of
useBetween(stateFn) calls useSyncExternalStore (directly or via
useExternalSnapshot / useUserDataSnapshot), React resolves the
dispatcher to use-between's proxy, finds .useSyncExternalStore
missing, and calls undefined() — that's the exact production crash
in Firefox. Chrome reports the same as
"dispatcher.useSyncExternalStore is not a function".
Neither the vite alias (790ad2b) nor the defensive renderer-method
guards (c35a2d4) could fix it — both addressed downstream symptoms
(stale dist / missing manager methods) but the dispatcher is upstream
of both. That's why every retry kept reproducing the same error.
Fix is structural: snapshot hooks (useUserDataSnapshot,
useIsUserIgnored, etc.) MUST run outside any useBetween scope. Three
re-applied migrations:
- useSessionInfo: snapshot read moved into the outer wrapper. The
inner useSessionInfoState (useBetween-shared) now contains only
use-between-safe hooks: useState, useMessageEvent, plain actions.
userFigure / userRespectRemaining / petRespectRemaining come from
useUserDataSnapshot() OUTSIDE useBetween, so useSyncExternalStore
installs against the real React dispatcher.
- useChatWidget.ownUserId: direct snapshot read. useChatWidget is
exported as `useChatWidget = useChatWidgetState` (NOT wrapped in
useBetween), so this hook never sat inside the broken scope — the
precautionary rollback was unnecessary in retrospect. Gains
session-change reactivity (e.g. reconnect under a different user id).
- AvatarInfoWidgetAvatarView Ignore/Unignore: useIsUserIgnored(name)
read directly in the component body. Same reasoning as
useChatWidget — never inside useBetween. The menu auto-flips
Ignore <-> Unignore while the popup is open.
Added regression guard at src/hooks/session/useSessionSnapshots.test.tsx
with two cases: (1) useSyncExternalStore inside useBetween throws,
(2) useSyncExternalStore outside useBetween in the same render works.
Pins the constraint so future migrations cannot reintroduce the bad
shape silently.
Verification: yarn typecheck clean, yarn test 209/209 (207 baseline
+ 2 new regression cases), no consumer surface changes — every
destructured field (userFigure, userRespectRemaining, respectUser,
petRespectRemaining, respectPet, chatStyleId, updateChatStyleId) is
still returned with the same name and shape.
The migrations of useSessionInfo, useChatWidget.ownUserId and the
AvatarInfo Ignore/Unignore menu to the new useSessionSnapshots hooks
were correct in code but produce a persistent runtime error in the
user's deployment:
TypeError: (intermediate value)() is undefined
ToolbarView ToolbarView.tsx:46
The error fires from React's render loop on the first paint —
ToolbarView is the first mounted consumer of useSessionInfo, which is
why it carries the boundary message. Two attempted fixes did not
resolve it on the user's side:
- 790ad2b: vite alias forcing @nitrots/nitro-renderer to source index.ts
- c35a2d4: defensive typeof guards on every Manager method call inside
useSessionSnapshots (so a missing method degrades to a frozen default
rather than calling undefined)
Both are correct defenses but the error persists, meaning the failure
mode is upstream of those guards. Rather than burn more cycles
remote-debugging, roll back the three consumer migrations:
- useSessionInfo: restored to the pre-71a0eee shape — 5 useState
fields driven by useMessageEvent<UserInfoEvent, FigureUpdateEvent,
UserSettingsEvent>. The five consumers (ToolbarView, HcCenterView,
ChatInputView, AvatarInfoPetTrainingPanelView, InfoStandWidgetPetView,
AvatarInfo{Avatar,Pet,OwnPet}View) get the same destructured shape
they had before this session.
- useChatWidget.ownUserId: restored to `GetSessionDataManager()?.userId`
(synchronous, captured at mount). Loses the session-change reactivity
but matches the previous, working behaviour.
- AvatarInfoWidgetAvatarView Ignore/Unignore: restored to
`avatarInfo.isIgnored` (captured by AvatarInfoUtilities at click
time, not reactive). Loses the live-toggle if the user is
ignored/unignored while the popup is open — known small regression,
worth it for stability.
Kept intact:
- The useSessionSnapshots.ts hook file itself, with defensive guards,
so the API stays available for any future opt-in consumer.
- 790ad2b vite alias for the umbrella, still useful as defence in
depth for future migrations.
- All the other non-snapshot modernizations from this session
(usePetPackageWidget reducer, useWordQuizWidget bug fix,
useChatCommandSelector Zustand store, useAvatarInfoWidget typed
globalThis accessor).
Verification: yarn typecheck clean, yarn test 207/207, yarn build green.
The toolbar should boot without the error now — the call chain on the
first paint no longer touches the new useExternalSnapshot / snapshot
getter path.
Two small modernization wins on the previously skip-motivated god-hooks.
Neither hook lends itself to the data/actions split, but both had
concrete imperative-style residue worth tidying:
== useChatWidget
Replace `const ownUserId = GetSessionDataManager()?.userId || -1;` with
`useUserDataSnapshot().userId`. The previous read happened at hook mount
and stayed pinned to whatever userId the manager held at that point —
a session change (re-login without page reload) would silently corrupt
the outgoing-translation owner check below. With the snapshot hook,
the value updates reactively via SESSION_DATA_UPDATED and the
useNitroEvent re-registration picks up the fresh ownUserId for every
incoming chat event.
== useAvatarInfoWidget
Two tidy points:
- CLICK_USER_DEBOUNCE_MS (the 120ms window during which a directional
click suppresses the context menu) lifted from inside the hook body
to a module-level const. It's never going to change at runtime and
doesn't depend on hook state — keeping it inside meant it was
redeclared on every render.
- The `(globalThis as any).__nitroAvatarClickControl` read replaced by
a typed `getAvatarClickControl()` helper backed by a proper
`NitroAvatarClickControl` interface. Same runtime behaviour; type
channel no longer goes through `any`, and the symbol is documented
in one place above the hook.
Public APIs of both hooks unchanged. Suite: 207/207.
Two module-level `let` declarations (cachedServerCommands +
globalListenerRegistered) were tracking the AvailableCommandsEvent
listener state outside React. The pattern was a React Compiler
violation flagged elsewhere in the codebase (the navigatorRoomCreator
fix was the canonical precedent — see commit fd1835c).
Move both into a per-hook Zustand store
(`useChatCommandStore`) following the same convention as
`useWiredCreatorToolsUiStore` and `useRoomCreatorStore`. The store
keeps the cached server-pushed CommandDefinitions plus a
single-shot isListenerRegistered flag that prevents the in-hook
useMessageEvent and the module-level pre-mount listener from
double-registering.
`CLIENT_COMMANDS` stays at module scope — it's a const array,
React Compiler is fine with constant data.
Behavioural change: zero. The pre-mount registration still tries
once at module load (covering the case where the server's
AvailableCommands lands before any React widget mounts). The in-hook
useMessageEvent still covers later mounts and rank-change refreshes.
Every push goes through `setServerCommands`, so all consumers see
the same data.
Side benefit: a future test can now `useChatCommandStore.setState({
serverCommands: [...], isListenerRegistered: true })` to seed a
deterministic fixture without monkey-patching the module.
Public API of useChatCommandSelector unchanged; the one consumer
(ChatInputView) reads the same destructured fields. Verified via grep.
Suite: 207/207.
Two bugs and one tidy in the word-quiz widget hook.
== Bug 1: stale-closure read in setUserAnswers updater
setUserAnswers(prevValue => {
if(!prevValue.has(userData.roomIndex)) {
const newValue = new Map(userAnswers); // <- WRONG: reads
// ^^^^^^^ the closed-over
// state, not prevValue
newValue.set(userData.roomIndex, ...);
return newValue;
}
return prevValue;
});
The functional updater is supposed to read the *latest* state (its
`prevValue` argument), not the closed-over `userAnswers` from the
render that registered this listener. The old code mixed both:
`prevValue.has(...)` for the check but `new Map(userAnswers)` for the
copy. Under rapid successive ANSWERED events for different users
within the same tick, the second update would copy a stale map and
drop the first user's entry. Fixed: use prevValue throughout.
== Bug 2: questionClearTimeout stored in useState
The timeout handle is a side-channel value, not display state. Storing
it in useState meant every (re)schedule triggered a re-render even
though no widget reads it. It also let the cleanup effect close over
a stale handle if the unmount fired between the schedule and the
state commit. Moved to useRef + a small `scheduleQuestionClear(delay)`
helper that consolidates the clear-then-set pattern (was duplicated
across FINISHED and QUESTION handlers).
== Tidy
- The duration-zero branch of QUESTION now explicitly clears any
pending timeout instead of falling through to a `setTimeout(..., null)`
no-op path.
- Cleanup effect rewritten as a single arrow-return for brevity.
Public API of useWordQuizWidget unchanged. Suite: 207/207.
The hook tracked five related useState fields driving the pet-package
naming dialog (isVisible / objectId / objectType / petName / errorResult).
They transitioned in lockstep on the two RoomSessionPetPackageEvent
types and the inline change handler — textbook state-machine territory.
Collapse into a single useReducer with four explicit transitions:
- 'open' → REQUESTED event lands; flips visible, records target
- 'close' → REQUESTED-result success OR user dismiss; resets to INITIAL
- 'set-name' → input change; updates petName AND clears any error
(the previous code had this side effect inlined in
onChangePetName as `if(errorResult.length) setErrorResult('')`,
now it's part of the reducer contract)
- 'set-error' → REQUESTED-result with validation failure; sets the label
Plus extract `getPetPackageNameError(code)` to a top-level exported
pure function (was an inline closure named getErrorResultForCode).
The mapping is server-protocol contract, not UI state — moving it out
of the hook means it's testable, reusable, and won't be recreated on
every render.
Public API of usePetPackageWidget is unchanged — the one consumer
(PetPackageWidgetView) reads the same destructured fields. Verified
via grep.
Tests: 4 new cases on getPetPackageNameError covering code 0 / 1-4 /
falsy / unknown-fallback. Suite: 207/207 (was 203/203).
The Jest-style __mocks__/ folder added one indirection for a single
file. Move the stub to src/nitro-renderer.mock.ts at src/ root next to
test-setup.ts, drop the folder, repoint the vitest alias, and update
the lone test that imports the helpers directly (useDoorbellState).
Same behaviour, one fewer directory.
Eliminate the parallel `tests/` tree. Each `*.test.ts` / `*.test.tsx`
now sits in the same directory as the module it covers, mirroring its
filename (`Foo.ts` ↔ `Foo.test.ts`). The renderer-SDK mock used by
component / hook tests moves to `src/__mocks__/nitro-renderer.ts` and
the Vitest setup file becomes `src/test-setup.ts` — both still wired
through `vitest.config.mts` exactly as before, only the paths changed.
All 13 suites + 178/178 cases still pass. The production build is
unaffected: rollup only follows imports from `src/index.tsx` and never
crosses into `.test.ts` files, so test code is naturally tree-shaken
out of the bundle. `yarn build` output is byte-for-byte the same on
the user-facing chunks.
tsconfig drops the now-redundant `tests` include entry. CLAUDE.md
'Layout convention' replaces the old `tests/` row with three rows
documenting the new co-located convention, the `__mocks__/` directory
and the `test-setup.ts` entry; ARCHITECTURE.md picks up the same
update. The 'DO NOT CHANGE' qualifier on the layout is preserved —
this rewrite IS the change, decided deliberately to make tests a
first-class part of the source tree rather than a sibling project.
The three reducers that drive the InfoStand pilot
(applyUserBadgesUpdate / applyUserFigureUpdate /
applyFavouriteGroupUpdate, in src/hooks/rooms/widgets/avatarInfo.reducers.ts)
have been live for ~10 commits without coverage. They encode
non-trivial branches: 'state not AvatarInfoUser' bail-out,
'event for different user / roomIndex' bail-out, dedup-equality
bail-out, and the clearGroup logic (status === -1 || habboGroupId <= 0).
Add tests pinning every branch.
Two import-tightening tweaks made the reducer module itself
testable in jsdom without dragging the renderer SDK in:
- Renderer event types are now type-only imports — they're erased
at compile time, so the runtime module load of @nitrots/nitro-renderer
is skipped. The reducer body only reads plain event fields (no
) so this is safe.
- AvatarInfoUser / dedupeBadges / IAvatarInfo come from concrete file
paths instead of '../../../api' (the barrel pulls in Pixi-bound
modules via the renderer side-imports).
Tests cover each branch by constructing AvatarInfoUser via the
actual class (so the instanceof guard hits) and casting plain event
objects through for the typed parameter.
Net Vitest count: 99 -> 113 (8 test files).
Continues the proposal #4 split pattern (doorbell, poll, furni-chooser,
user-chooser, friend-request) for the chat-input widget. Splits the
334-line useChatInputWidget along the natural seam:
- useChatInputState — selectedUsername / floodBlocked / floodBlockedSeconds
/ isTyping / isIdle state plus the three event listeners
(FLOOD_EVENT, ObjectSelected, ObjectDeselected) and the three lifecycle
effects (flood-countdown, idle-auto-clear, typing-indicator sync).
- useChatInputActions — sendChat(text, chatType, recipientName, styleId).
Carries the slash-command handler (":shake", ":rotate", ":zoom",
":screenshot", ":pickall", etc.) and the chat-vs-shout-vs-whisper
dispatch path, with the optional outgoing-translation hook.
- useChatInputWidget — deprecated shim that composes both into the
historical { selectedUsername, floodBlocked, floodBlockedSeconds,
setIsTyping, setIsIdle, sendChat } shape so ChatInputView keeps
working unchanged.
Bonus while in here:
- Guarded all roomSession reads in actions with optional chaining
(the hook can be called during the brief no-room window between
enter and leave).
- Dropped the useless 'if(isIdle)' inside the idle effect body — the
early return guard above it already covers that branch.
IGetImageListener.imageReady(result: IImageResult) takes a single
IImageResult object (with .id, .data, .image), but three call sites in
the client still used the old 3-arg destructure '(id, texture, image)
=> ...'. The renderer's RoomEngine.ts already passes
'new ImageResult(...)' to the listener, so the runtime payload matches
the new contract; the old call-site shape just type-errored.
Migrated:
- LayoutPetImageView (pet thumbnail loader)
- LayoutRoomObjectImageView (furniture thumbnail loader)
- useFurniturePresentWidget (gift box image generator)
Also tightened imageFailed handlers from 'imageFailed: null' to a
proper no-op arrow — the interface requires a callback.
The two helper functions buildWallItem and buildFloorItem took
roomObject as 'any', so 'model.getValue<number>(...)' became an
untyped-function-with-type-args error under tsgo (six hits). Typing
the param as IRoomObject (the renderer's public interface — model is
already typed there) fixes them all at once.
The fallback chain for ownerName was guarded by
'sessionDataManager.getUserData ? sessionDataManager.getUserData(ownerId)?.name : null'
— SessionDataManager.getUserData() does NOT exist on the renderer
(documented in Nitro_Render_V3/CLAUDE.md), so that branch was always
dead. Dropping it removes the four tsgo errors and the misleading
condition.
Net tsgo error count: 90 -> 80.
InfoStandWidgetUserView previously subscribed to three room-session
events (RSUBE_BADGES, USER_FIGURE, FAVOURITE_GROUP_UPDATE) and pushed
the result back to its parent via a setAvatarInfo prop, with each
handler running CloneObject(prev) before patching one field. Three
issues with that shape:
- CloneObject was deep-cloning the whole AvatarInfoUser shape blindly
with no class-prototype awareness;
- the three listeners raced on shallow merges across the same prev
reference in StrictMode dev;
- the subscriptions lived outside the state owner, forcing a prop
callback barrier per event.
The subscriptions are now in useAvatarInfoWidget — the actual owner of
avatarInfo — and call three pure reducers extracted to
src/hooks/rooms/widgets/avatarInfo.reducers.ts (applyUserBadgesUpdate,
applyUserFigureUpdate, applyFavouriteGroupUpdate). Each reducer returns
the same reference when the event doesn't apply so React bail-outs work.
The clone now constructs a fresh AvatarInfoUser preserving prototype.
dedupeBadges is extracted to its own pure module under src/api/avatar/
so Vitest can cover it without pulling in the renderer.
InfoStandWidgetUserView loses the setAvatarInfo prop (parent updated)
and the CloneObject import.
Stesso pattern di doorbell / poll / furni-chooser / user-chooser:
flat split sotto src/hooks/rooms/widgets/, no co-location dentro
src/components/.
Split
- src/hooks/rooms/widgets/useFriendRequestState.ts (new):
activeRequests state + displayedRequests derived (filter su
dismissedRequestIds) + due bridge events (user added/removed) +
un useEffect che riallinea activeRequests quando cambia il set
di requests dal friends-store. Esporta anche il tipo
ActiveFriendRequest per consumi futuri.
Plus: ?. su roomSession e userDataManager per evitare il bug
pattern "session è null in transition" (vedi PetTrainingPanel,
precedentemente fixato).
- src/hooks/rooms/widgets/useFriendRequestActions.ts (new):
hideFriendRequest. Thin adapter sul friends-store
(setDismissedRequestIds), nessuna subscription.
- src/hooks/rooms/widgets/useFriendRequestWidget.ts: deprecated
shim che compone i due e preserva
{ displayedRequests, hideFriendRequest } per il consumer
FriendRequestWidgetView.
Verifica
- yarn eslint sui 4 file toccati: 1 errore pre-esistente
(set-state-in-effect sul useEffect che ri-derive activeRequests
da requests — già nel file originale, baseline invariata).
- yarn test: 49/49 passing.
- yarn tsc: clean.
Sequence widget split adesso a 5 (doorbell, poll, furni-chooser,
user-chooser, friend-request). Rimangono: usePetPackageWidget,
useWordQuizWidget, useChatInputWidget, useChatWidget,
useAvatarInfoWidget, useFilterWordsWidget.
Speculare di useFurniChooserWidget — stesso split + stesso layout (flat
in src/hooks/rooms/widgets/). User chooser è il gemello del furni
chooser nella shape: items list popolata da room scan + due bridge
events (added/removed) + selectItem imperativo.
Split
- src/hooks/rooms/widgets/useUserChooserState.ts (new):
items + onClose + populateChooser + useUserAddedEvent +
useUserRemovedEvent. Helper buildUserItem dedupa la costruzione
di RoomObjectItem fra populateChooser e l'add handler (~20
righe di duplicazione in meno).
Plus: aggiunto ?. su roomSession e userDataManager (lo stesso
bug pattern del PetTrainingPanel fixato altrove).
- src/hooks/rooms/widgets/useUserChooserActions.ts (new):
selectItem puro.
- src/hooks/rooms/widgets/useUserChooserWidget.ts: kept as a
deprecated shim that composes both and preserves
{ items, onClose, selectItem, populateChooser } per il consumer
UserChooserWidgetView.
Verifica
- yarn eslint sui 4 file toccati: 0 errors / 0 warnings.
- yarn test: 49/49 passing.
- yarn tsc: clean.
Sequenza god-hook split adesso a 4 (doorbell, poll, furni-chooser,
user-chooser). Rimangono: useFriendRequestWidget, usePetPackageWidget,
useWordQuizWidget, useChatInputWidget, useChatWidget,
useAvatarInfoWidget, useFilterWordsWidget.
Apply the same data/actions split pattern (proposal #4) to
useFurniChooserWidget, the largest god-hook still on the widgets
side (161 LOC). Layout follows the main branch convention:
flat files under src/hooks/rooms/widgets/, no per-feature subfolder,
no co-location of hooks inside src/components/.
Split
- src/hooks/rooms/widgets/useFurniChooserState.ts (new): owns the
items array, the populateChooser action that scans the current
room, the two RoomEngine event bridges (added/removed), and
onClose. Helper buildWallItem/buildFloorItem dedupes the two
copies of the RoomObjectItem construction that used to live
inline in both populateChooser and the added-event handler
(~50 lines of duplication removed).
- src/hooks/rooms/widgets/useFurniChooserActions.ts (new): the
one pure imperative action — selectItem — that doesn't need to
subscribe to anything.
- src/hooks/rooms/widgets/useFurniChooserWidget.ts: kept as a
deprecated shim that composes both and returns the same
{ items, onClose, selectItem, populateChooser } shape so
FurniChooserWidgetView (the only consumer) doesn't change.
Layout note
- This is consistent with the main branch: each widget hook is a
flat file under src/hooks/rooms/widgets/ (no <feature>/ subfolder),
while the view sits under src/components/room/widgets/<feature>/.
- The parallel feat/react19-hooks-adapter branch chose the opposite
convention (hooks co-located inside src/components/...). Per the
team decision recorded in docs/ARCHITECTURE.md proposal #3, this
repo stays on the flat-hooks-folder layout.
Verification
- yarn tsc on the touched files: 6 TS2347 errors after the split,
12 before — the buildWallItem/buildFloorItem helpers actually
*reduce* the local sandbox TS2347 surface (the renderer SDK is
not installed locally, so `roomObject.model.getValue<T>` is
flagged as "untyped function with type arg"; merging the two
callsites into one helper halves the count).
- yarn eslint on the touched files: 0 errors, 0 warnings.
- yarn test: 49/49 passing.
Follow-up to the previous commit's poll split. The compat shim
usePollWidget used to call usePollSubscriptions() inside its body so
the three RoomSessionPollEvent listeners were still registered for
existing consumers — but that meant:
- listeners would be re-registered per consumer (today nobody, since
useWordQuizWidget was already migrated to usePollActions);
- the lifetime of the subscriptions was tied to a leaf widget instead
of the room session;
- a render of a component using the shim had the side effect of
attaching three global event listeners.
Move
- src/components/room/widgets/RoomWidgetsView.tsx now calls
usePollSubscriptions() once at the top of the room-widget tree. The
bridge from RoomSessionPollEvent (OFFER/ERROR/CONTENT) to the UI
event bus is now mounted for exactly the lifetime of an in-room
session, regardless of which leaf widget renders.
- src/hooks/rooms/widgets/usePollWidget.ts (compat shim) is reduced
to a one-liner that just returns usePollActions(). It is still
deprecated; remove once nothing imports it.
Verification
- yarn eslint on the two touched files: 1 pre-existing error
(the same FC<{}> in RoomWidgetsView that was there before — baseline
unchanged; I deliberately did not touch it in this commit to keep
the diff minimal).
- yarn test: 22/22 still passing.
- grep confirms usePollWidget has zero in-tree consumers; the only
importer is the barrel re-export.
usePollWidget bundled two unrelated responsibilities:
- three useNitroEvent listeners that bridge RoomSessionPollEvent
(OFFER / ERROR / CONTENT) onto the UI event bus via DispatchUiEvent
— pure side-effects, zero local state, should mount once;
- three imperative actions (startPoll, rejectPoll, answerPoll) that
every consumer wants, but which shouldn't re-register the listeners.
In practice the only consumer of usePollWidget was useWordQuizWidget,
which needed only `answerPoll` — but pulled in the three subscriptions
as a side effect every time the word-quiz widget rendered. That's the
classic god-hook anti-pattern this proposal targets.
Split (mirrors the doorbell pattern already in place):
- src/hooks/rooms/widgets/usePollSubscriptions.ts (new): the three
bridge listeners, returns void. Should be mounted ONCE at the
highest stable level above poll-aware UI (room widgets root). For
now still mounted by the shim — follow-up PR can move it.
- src/hooks/rooms/widgets/usePollActions.ts (new): the three
imperative actions. Defensive `?.` on roomSession so a poll action
during a room transition no longer crashes.
- src/hooks/rooms/widgets/usePollWidget.ts: kept as a deprecated shim
that composes both — preserves the old `{ startPoll, rejectPoll,
answerPoll }` shape so existing consumers don't break.
- src/hooks/rooms/widgets/useWordQuizWidget.ts: migrated to import
usePollActions directly. The word-quiz widget no longer registers
poll subscriptions transitively — its render no longer has the side
effect of subscribing to three renderer events.
Doc
- docs/ARCHITECTURE.md "What's already in place": records both god-hook
splits (doorbell + poll), the now-enabled React Query and Zustand,
and the test infrastructure. Removes the "not yet enabled" markers
for #2 and #5.
- "How to pick the next refactor PR": rewritten to reflect that the
foundations are done. New priority order:
1. migrate useCatalog's read-only fetches to useNitroQuery,
2. hoist usePollSubscriptions to room-session level,
3. split useCatalog along the doorbell/poll lines,
4. broaden Vitest coverage,
5. per-tab WiredCreatorToolsView split.
Verification
- yarn eslint on the touched files: 0 errors / 0 warnings.
- yarn test: 22/22 passing, 2 files, ~1.0s.
- Existing useWordQuizWidget consumers (RoomWidgetsView ->
WordQuizWidgetView) unaffected — they import from the barrel which
still re-exports the same shape.
https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
Decision: the src/features/<feature>/ layout introduced as proposal #3
(pilot on doorbell in 8ec9d27) is not the convention the team wants. The
existing src/components/<area>/ + src/hooks/<area>/ split is the one
that stays.
What's reverted
- src/features/doorbell/ is removed entirely. The doorbell view and the
two hooks move back under the classic paths:
src/features/doorbell/views/DoorbellWidgetView.tsx
-> src/components/room/widgets/doorbell/DoorbellWidgetView.tsx
src/features/doorbell/hooks/useDoorbellState.ts
-> src/hooks/rooms/widgets/useDoorbellState.ts
src/features/doorbell/hooks/useDoorbellActions.ts
-> src/hooks/rooms/widgets/useDoorbellActions.ts
- The compat shims that lived in those classic paths are dropped now
that the real files are back.
- src/hooks/rooms/widgets/index.ts adds the two new hooks alongside the
existing useDoorbellWidget shim (kept as a deprecated wrapper so any
external consumer importing the old shape via the barrel keeps
working).
What's preserved
- The split between data and actions (proposal #4) — useDoorbellState
and useDoorbellActions remain two separate hooks. This was the actual
improvement, and it's independent of where the files sit.
- The bug fixes from 8ec9d27 (close button race, optimistic-remove
rollback) — both still present, just in the new path.
- src/state/createNitroStore.ts and src/api/nitro-query/createNitroQuery.ts
are left where they are. They aren't feature folders; they're
cross-cutting framework code (Zustand skeleton, React Query adapter
prototype) that any feature can consume.
Doc
- docs/ARCHITECTURE.md section #3 is rewritten to record the decision
rather than recommend the layout. It now describes the convention to
follow:
* views under src/components/<area>/<feature>/
* hooks under src/hooks/<area>/<feature?>/ (siblings, not subfolders
per widget)
* sibling .types/.constants/.helpers files for view-specific code
(e.g. WiredCreatorTools.*.ts)
- "What's already in place" and "Recently fixed" sections updated to
point at the new paths.
- "How to pick the next refactor PR" no longer mentions feature-folder
migration as an option.
Note: the five extra feature folders started this session (reconnect,
nitropedia, ads, hc-center, campaign) were never committed; they only
existed in the working tree and have been restored from HEAD.
Verification
- find src/features -type f -> 0 (directory removed).
- npx tsc --noEmit on all touched files: clean (only the project-wide
pre-existing TS2307 about @nitrots/nitro-renderer not installed
locally remains, same as before).
- npx eslint on all touched files: 0 errors, 0 warnings.
https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
This is the structural plan promised in the previous session, with concrete
pilots for all five proposals + the bonus error-boundary work.
== docs/ARCHITECTURE.md (new, ~370 lines)
Living document describing:
- where the project stands today (event-bus pattern friction with React 19,
god-hooks, oversized files);
- the five proposed structural improvements with the why/how/status of each;
- what's already in place across this branch;
- recommended order for the next refactor PRs.
This is the deliverable the rest of this commit references.
== Proposal #3 + #4 pilots: src/features/doorbell/ (new)
Concrete feature-folder migration on the doorbell widget (chosen because
it's small enough to migrate end-to-end in one commit).
src/features/doorbell/
index.ts public API
views/DoorbellWidgetView.tsx
hooks/useDoorbellState.ts reduces 3 events into a users array (data only)
hooks/useDoorbellActions.ts answer(name, flag) (imperative actions only)
The split (data vs actions) is the pattern proposal #4 wants applied to
useCatalog/useChat/useWiredTools later. The original useDoorbellWidget had
both concerns + a buggy `useEffect(() => setIsVisible(!!users.length), [users])`
derive-state-in-effect. The new view computes visibility in render.
Compat shims kept so existing imports keep working:
- src/components/room/widgets/doorbell/DoorbellWidgetView.tsx -> 1-line re-export
- src/hooks/rooms/widgets/useDoorbellWidget.ts -> deprecated wrapper around
the two new hooks, returning the same { users, answer } shape.
== Proposal #2 prototype: src/api/nitro-query/ (new)
Adapter outline for wrapping composer/parser request-response pairs in
TanStack Query. Not yet enabled because @tanstack/react-query is not in
package.json. The file documents the activation steps:
yarn add @tanstack/react-query @tanstack/react-query-devtools
+ mount QueryClientProvider in src/index.tsx
awaitNitroResponse() throws with a helpful pointer to the doc section if
called before activation, so accidental adoption fails loudly.
== Proposal #5 skeleton: src/state/createNitroStore.ts (new)
Same pattern: skeleton + activation instructions. Not yet enabled because
zustand is not in package.json.
yarn add zustand
+ replace the throw with `import { create } from 'zustand'; export const createNitroStore = create;`
The doc inside the file shows the recommended slice shape and points to
the suggested first migration target (the let isCreatingRoom singleton in
NavigatorRoomCreatorView).
== Bonus: WidgetErrorBoundary
src/common/error-boundary/WidgetErrorBoundary.tsx wraps react-error-boundary
with a sensible default (silent fallback, NitroLogger.error). Re-exported
from src/common/index.ts.
Applied as the umbrella around RoomWidgetsView's children — a widget
crash in a room (e.g. malformed pet data) now degrades gracefully
instead of unmounting the whole UI.
== Verification
- yarn eslint on all new + modified files: 0 errors / 0 warnings introduced.
RoomWidgetsView still has its 1 pre-existing FC<{}> error (1 before, 1 after).
- yarn tsc on all new files: clean (only project-wide pre-existing
TS2307 about @nitrots/nitro-renderer not installed locally remains).
- No regressions: existing imports of DoorbellWidgetView and
useDoorbellWidget keep resolving via the compat shims.
== What's NOT in this commit (intentionally)
- Mass adoption of the new patterns elsewhere — left as follow-up PRs in
the order documented in ARCHITECTURE.md "How to pick the next refactor PR".
- Installation of @tanstack/react-query / zustand — explicit team decision,
not the LLM's to make.
- Test infrastructure (Vitest setup) — listed as the #1 missing piece in
the doc, but a separate PR.
https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
Run eslint --fix across src/ to clear ~1900 mechanical lint errors
surfaced by the @typescript-eslint v8 + react-hooks v7 + react-compiler
upgrade in the React 19 modernization PR.
Issues fixed automatically:
- brace-style (Allman): try/catch one-liners reformatted to multi-line
- indent: tab-vs-space and depth corrections
- semi: missing trailing semicolons
- no-trailing-spaces
No semantic changes. Remaining 701 errors are real-code issues
(set-state-in-effect, rules-of-hooks, no-unsafe-* type checks) that
need manual per-file review.
https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
- Catalog page for creating custom prefixes with text, per-letter colors, emoji icon and visual effects
- Emoji picker via @emoji-mart/react with createPortal + Shadow DOM blur fix
- Inventory prefix management (activate/deactivate/delete)
- Chat bubble rendering with multi-color prefix and effect support
- Prefix utilities (getPrefixEffectStyle, parsePrefixColors, PREFIX_EFFECT_KEYFRAMES)
- All UI text in English
Server-authoritative command list via packet 4050, merged with
client-only commands. Supports keyboard navigation, filtering,
and module-level caching to handle login-time packet timing.
Co-Authored-By: medievalshell <medievalshell@users.noreply.github.com>
- Drag & drop badges between slots in InfoStand (own user only)
- Mini badge picker on empty slot click with search
- Swap badges between occupied slots
- Hover animation (scale, glow) on badge slots
- Configurable group slot (user.badges.group.slot.enabled)
- Support for 6 badge slots when group slot disabled
- Race condition fix with localChangeRef
- Fixed-size array logic to prevent badge disappearing
Co-Authored-By: medievalshell <medievalshell@users.noreply.github.com>