You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Replace the 280-line if/elif chain in _handle_key_press() with a declarative key-binding map that dispatches directly to subsystem methods. No event bus, no pub/sub — just a dict lookup and a method call.
Why not a publish-subscribe event bus?
The original proposal was a full pub/sub event bus with typed event classes (ToggleShadowsEvent, CycleLayerEvent, ScreenshotEvent, etc.), subscriber registration, priority ordering, and deferred event queues. This adds indirection without corresponding benefit:
One input source, one consumer. rtxpy has a single keyboard/mouse input source and a single viewer that acts on it. Pub/sub decouples publishers from subscribers — but when there's exactly one of each, the decoupling just makes the code harder to follow. You can't "go to definition" on an event publish to find what handles it.
Event type proliferation. The proposed design had 12+ event dataclasses that are effectively just enum values with extra boilerplate. ToggleShadowsEvent is a class that carries no data — it could be a string key 'toggle_shadows' pointing directly at self.render_settings.shadows = not self.render_settings.shadows.
No plugins or user scripts. The pub/sub pattern's main benefit is extensibility — letting unknown third parties hook into actions. rtxpy has no plugin system and none is planned for Phase 1. If plugins are added later, they can subscribe to a bus then; adding the bus now is premature.
The REPL command queue already works.ViewerProxy._submit() (engine.py:604-631) uses a thread-safe command queue for REPL access. This pattern is correct and simple — lambdas queued and drained on the main thread. No need to generalize it into an event bus.
Deferred events add hidden ordering complexity. With direct dispatch, the order of effects is obvious from reading the code. With deferred events, you need to reason about queue ordering, priority, and when flush_deferred() runs relative to rendering.
What's actually needed: the 280-line if/elif chain is bad code, but the fix is a data-driven dispatch table, not an architectural pattern.
Current State
Monolithic key handler (engine.py:3418-3704):
_handle_key_press() is a 280-line if/elif chain that directly mutates viewer state
Shift-modified keys (uppercase raw_key) are checked first, then lowercase key is dispatched
Movement keys (w/a/s/d/arrows/q/e/pageup/pagedown) are added to _held_keys set
Everything else is an instant action (toggle, cycle, screenshot, etc.)
def_handle_key_press(self, raw_key, key):
# Movement keys → track as heldifkeyinMOVEMENT_KEYS:
self.input.held_keys.add(key)
return# Observer slot selection: 1-8ifkeyin'12345678':
self.observers.select_or_create(int(key))
return# Lookup in binding mapshift=raw_key!=keyandraw_key.isupper()
action=self._key_bindings.get((key, shift))
ifactionisnotNone:
action()
That's it. The 280-line if/elif becomes ~15 lines of dispatch + a declarative table.
Benefits over current code
Readable: scan the binding table to see all shortcuts at a glance
Rebindable: swap entries in the dict to change key assignments
Testable: unit-test that the dispatch table maps correctly without a GPU context
Self-documenting: the help text overlay can be auto-generated from the binding table
REPL command queue stays as-is
The existing ViewerProxy._submit() pattern (lambda queued, drained on main thread) is already correct and simple. No need to merge it into an event bus.
Parent: #57 — Phase 1 Architecture
Summary
Replace the 280-line if/elif chain in
_handle_key_press()with a declarative key-binding map that dispatches directly to subsystem methods. No event bus, no pub/sub — just a dict lookup and a method call.Why not a publish-subscribe event bus?
The original proposal was a full pub/sub event bus with typed event classes (
ToggleShadowsEvent,CycleLayerEvent,ScreenshotEvent, etc.), subscriber registration, priority ordering, and deferred event queues. This adds indirection without corresponding benefit:One input source, one consumer. rtxpy has a single keyboard/mouse input source and a single viewer that acts on it. Pub/sub decouples publishers from subscribers — but when there's exactly one of each, the decoupling just makes the code harder to follow. You can't "go to definition" on an event publish to find what handles it.
Event type proliferation. The proposed design had 12+ event dataclasses that are effectively just enum values with extra boilerplate.
ToggleShadowsEventis a class that carries no data — it could be a string key'toggle_shadows'pointing directly atself.render_settings.shadows = not self.render_settings.shadows.No plugins or user scripts. The pub/sub pattern's main benefit is extensibility — letting unknown third parties hook into actions. rtxpy has no plugin system and none is planned for Phase 1. If plugins are added later, they can subscribe to a bus then; adding the bus now is premature.
The REPL command queue already works.
ViewerProxy._submit()(engine.py:604-631) uses a thread-safe command queue for REPL access. This pattern is correct and simple — lambdas queued and drained on the main thread. No need to generalize it into an event bus.Deferred events add hidden ordering complexity. With direct dispatch, the order of effects is obvious from reading the code. With deferred events, you need to reason about queue ordering, priority, and when
flush_deferred()runs relative to rendering.What's actually needed: the 280-line if/elif chain is bad code, but the fix is a data-driven dispatch table, not an architectural pattern.
Current State
Monolithic key handler (engine.py:3418-3704):
_handle_key_press()is a 280-line if/elif chain that directly mutates viewer stateraw_key) are checked first, then lowercasekeyis dispatchedw/a/s/d/arrows/q/e/pageup/pagedown) are added to_held_keyssetThe actual pattern is simple:
This is a natural key-binding map — the if/elif chain is just a verbose encoding of it.
Layer cycling duplicated across 4 subsystems:
gkey)u/Shift+Ukeys)nkey)Shift+Ckey)Each has its own index, order list, and wrap-around logic — but the pattern is identical.
Proposed Design
Key-binding map
Dispatch replaces if/elif
That's it. The 280-line if/elif becomes ~15 lines of dispatch + a declarative table.
Benefits over current code
REPL command queue stays as-is
The existing
ViewerProxy._submit()pattern (lambda queued, drained on main thread) is already correct and simple. No need to merge it into an event bus.Implementation Notes
_key_bindingsin__init__after subsystem objects are created (depends on Phase 1: Decompose InteractiveViewer via Composition #61 composition)self.camera,self.terrain, etc.) — this naturally enforces the separation of concerns from Phase 1: Decompose InteractiveViewer via Composition #61_key_bindingsto build the help overlay instead of maintaining a separate stringr/R,z/Z), the shift variant is(key, True)and the non-shift is(key, False);,',:,") follow the same pattern — just more entries in the dict_dispatch_dom_event()(notebook.py) can use the same binding table with a platform-specific key translation layerAcceptance Criteria
_handle_key_press()