Move warship by a touch (of magic)

[VariableVince]

Description:

This adds moving warships by tapping them on a touchscreen. Now you can steer them just like you already could with a mouse.

Also has some earlier returns, doing checks only when needed, prevent as much duplicate checks and a bugfix.

In onMouseUp:

• early return if no OceanTile. Before, function findWarshipsNearCell would still go look for warships even if ocean wasn't clicked.
• Move const nearbyWarShips down. It isn't needed when this.selectedUnit is true.
• Remove unnecessary const clickedWarship.
• Move getting clickRef from function findWarshipsNearCell into onMouseUp. Because it is needed in case of this.selectedUnit too, within onMouseUp.
  Getting a valid clickRef for this.selectedUnit fixes: Runtime error when clicking outside the map after selecting a warship. The isValidCoord/Ref check was missing for this.selectedUnit.

For findWarshipsNearCell:

• moved the cell/tile checks out to onMouseUp, the only caller of the function.
• did NOT rename findWarshipsNearCell. Although it now uses tileRef as input. Renaming can cause merge issues so i only do this when needed.

Added onTouch:

• Tests if we need to look for warships to select/move or if we can open Radial Menu.
• Prevent as much duplicated checks as possible. So if no there's no Ocean Tile found, just send the radial menu event, which checks isValidCoord anyway. isOceanTile itself works fine even if it's no valid cell (proven by this.selectedUnit working all this time in onMouseUp without an isValidCoord test).

Screencap on mobile, shows selecting and moving warships, no runtime error when clicking outside the map after selecting a warship, while radial menu still opens as normal:

Warship.move.on.mobile.Radial.menu.keeps.working.mp4

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

tryout33

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

Walkthrough

Adds a new exported TouchEvent and changes short touch-tap handling to emit it. UnitLayer gains touch handling, refactors click handling to accept a TileRef (clickRef), and reuses mouse-up logic for movement/selection or opens a radial context menu on ocean/no-action taps.

Changes

Cohort / File(s)	Summary
TouchEvent Introduction src/client/InputHandler.ts	Adds export class TouchEvent implements GameEvent and emits TouchEvent for short touch taps in onPointerUp (uses event.x/event.y, preserves preventDefault).
Touch Input Handling & ClickRef Refactor src/client/graphics/layers/UnitLayer.ts	Imports TouchEvent/ContextMenuEvent; adds onTouch handler; renames findWarshipsNearCell parameter to clickRef: TileRef; extends onMouseUp to accept optional clickRef and nearbyWarships; onTouch converts screen→world coords, validates clickRef, delegates to onMouseUp for selection/move or emits ContextMenuEvent when no actionable nearby units (ocean/none).

Sequence Diagram(s)

sequenceDiagram
    participant User as User (touch)
    participant Input as InputHandler
    participant Layer as UnitLayer
    participant Game as Game Logic

    User->>Input: pointer up (touch)
    alt short tap (dist < 10)
        Input->>Input: create TouchEvent(x,y)
        Input->>Layer: emit TouchEvent
        Layer->>Layer: screen→world coords → clickRef
        alt valid clickRef & nearby warships
            Layer->>Layer: call onMouseUp(clickRef, nearbyWarships)
            alt unit selected
                Layer->>Game: MoveWarshipIntentEvent
                Layer->>Layer: deselect unit
            else select nearest warship
                Layer->>Layer: select warship
            end
        else no actionable warships
            Layer->>Game: ContextMenuEvent (radial)
        end
    else long press / drag
        Input->>Game: other pointer handling
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Potential focus areas:

• Correctness of screen→world coordinate conversion and TileRef validation in onTouch.
• Interaction and parameter passing between onTouch and the extended onMouseUp.
• Ensuring TouchEvent payload (x,y) matches consumers' expectations and preserves preventDefault semantics.
• Call sites for findWarshipsNearCell / any signature changes.

Possibly related PRs

• Improve the alternate view by adding geopolitical colors for territories and hover-based highlighting #1320 — also adds pointer-related GameEvent exports and pointer event emissions; likely overlaps in touch/pointer handling design.

Suggested labels

Feature - Frontend, UI/UX

Suggested reviewers

• evanpelle
• scottanderson

Poem

A tap becomes a gentle ping,
TouchEvent wakes the ocean ring,
ClickRef maps the salted blue,
Warships move or menus brew,
Small code ripple, fresh and true. 🎉

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is well-detailed and directly related to the changeset, explaining the touch input implementation, refactoring rationale, bug fixes, and including testing confirmation and screenshots.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title clearly refers to the main change: adding touchscreen support to move warships, matching the PR's core objective of enabling warship movement via touch input.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/client/graphics/layers/UnitLayer.ts (1)

161-174: Consider simplifying the delegation pattern.

Lines 163 and 170-171 create MouseUpEvent objects that are immediately passed to onMouseUp, but the event coordinates are ignored since clickRef (and optionally nearbyWarships) are provided.

This pattern is a bit confusing. Consider either:

• Extracting the shared logic into a helper method that takes clickRef and nearbyWarships directly
• Or documenting why the event object is necessary

Example refactoring:

private handleWarshipInteraction(
  clickRef: TileRef,
  nearbyWarships?: UnitView[]
) {
  if (this.selectedUnit) {
    this.eventBus.emit(
      new MoveWarshipIntentEvent(this.selectedUnit.id(), clickRef),
    );
    this.eventBus.emit(new UnitSelectionEvent(this.selectedUnit, false));
    return;
  }

  nearbyWarships ??= this.findWarshipsNearCell(clickRef);
  if (nearbyWarships.length > 0) {
    this.eventBus.emit(new UnitSelectionEvent(nearbyWarships[0], true));
  }
}

private onTouch(event: TouchEvent) {
  // ... validation ...
  
  if (this.selectedUnit || nearbyWarships.length > 0) {
    this.handleWarshipInteraction(clickRef, nearbyWarships);
  } else {
    this.eventBus.emit(new ContextMenuEvent(event.x, event.y));
  }
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 76bd70a and b4d6dfa.

📒 Files selected for processing (2)

• src/client/InputHandler.ts (2 hunks)
• src/client/graphics/layers/UnitLayer.ts (4 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-10-18T11:00:57.142Z

Learnt from: NewYearNewPhil
Repo: openfrontio/OpenFrontIO PR: 2230
File: src/client/graphics/GameRenderer.ts:269-277
Timestamp: 2025-10-18T11:00:57.142Z
Learning: In src/client/graphics/GameRenderer.ts, the GameRecapCapture implementation does not use setCaptureRenderEnabled on layers. Instead, it uses RecapCaptureSurface.capture() to render capture layers (TerrainLayer, TerritoryLayer, RailroadLayer, StructureIconsLayer, UnitLayer) directly to an off-screen canvas without requiring layer-level capture mode methods.

Applied to files:

• src/client/graphics/layers/UnitLayer.ts

📚 Learning: 2025-06-22T21:51:14.990Z

Learnt from: devalnor
Repo: openfrontio/OpenFrontIO PR: 1248
File: src/client/graphics/layers/TerritoryInfoLayer.ts:20-20
Timestamp: 2025-06-22T21:51:14.990Z
Learning: In TerritoryInfoLayer.ts, the highlightedTerritory field uses both null and undefined intentionally: undefined represents initial state or inactive layer (Ctrl released), while null represents active layer with no territory being highlighted at cursor position. This distinction is important for proper state change detection.

Applied to files:

• src/client/graphics/layers/UnitLayer.ts

🧬 Code graph analysis (2)

src/client/InputHandler.ts (1)

src/core/EventBus.ts (1)

• GameEvent (1-1)

src/client/graphics/layers/UnitLayer.ts (3)

src/client/InputHandler.ts (4)

• TouchEvent (21-26)
• MouseUpEvent (8-13)
• UnitSelectionEvent (31-36)
• ContextMenuEvent (52-57)

src/core/game/GameMap.ts (1)

• TileRef (3-3)

src/client/Transport.ts (1)

• MoveWarshipIntentEvent (164-169)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Deploy to openfront.dev

🔇 Additional comments (4)

src/client/InputHandler.ts (2)

21-26: LGTM! Clean event class definition.

The TouchEvent class follows the same pattern as other event classes in this file. The structure is simple and type-safe.

Note: While the name could theoretically conflict with the native DOM TouchEvent, this shouldn't cause issues in practice due to module scoping.

---

474-474: Coordinate properties are handled correctly.

ContextMenuEvent class stores coordinates as x and y properties. UnitLayer's new onTouch handler correctly creates ContextMenuEvent with event.x and event.y from TouchEvent, matching how TouchEvent itself is created. Consumers like PlayerInfoOverlay and MainRadialMenu both access the normalized x and y properties, so the coordinate mapping is transparent and consistent.

src/client/graphics/layers/UnitLayer.ts (2)

10-12: LGTM! Event wiring follows existing patterns.

The imports and event handler registration are clean and consistent with the existing MouseUpEvent handler.

Also applies to: 78-78

---

87-90: Good refactoring to use TileRef.

Changing from a cell object to a TileRef makes the signature cleaner and more type-safe. The JSDoc is correctly updated to match.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/client/InputHandler.ts (1)

21-26: Consider renaming to avoid collision with native DOM TouchEvent.

The name TouchEvent conflicts with the native DOM API's TouchEvent type. This could cause confusion when importing or debugging. A more specific name like TouchTapEvent or GameTouchEvent would clarify its purpose and avoid potential naming conflicts.

Apply this diff to rename the class:

-export class TouchEvent implements GameEvent {
+export class TouchTapEvent implements GameEvent {
   constructor(
     public readonly x: number,
     public readonly y: number,
   ) {}
 }

Then update line 485:

-        this.eventBus.emit(new TouchEvent(event.x, event.y));
+        this.eventBus.emit(new TouchTapEvent(event.x, event.y));

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5398dcd and 419e7df.

📒 Files selected for processing (1)

• src/client/InputHandler.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/client/InputHandler.ts (1)

src/core/EventBus.ts (1)

• GameEvent (1-1)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Deploy to openfront.dev

[evanpelle]

Thanks!