feat(naval): shipyard vessel construction, wind vector navigation, and cannon broadside volleys

[angelTomo9]

Summary

Implements a naval shipyard, coastal war galleon, and ocean navigation engine for OpenAO MMORPG.

Features

• Shipyard vessel construction across multiple classes (Fishing Boat, Cargo Clipper, War Galleon, Ghost Frigate)
• Hull material durability scaling (Oak, Reinforced Ironwood, Cursed Ghostwood)
• Wind-vector trigonometric sailing speed calculations with cargo hold weight penalties
• Multi-cannon broadside volleys with reload cooldowns and armor mitigation
• Drydock repair facilities for damaged vessels
• Full unit test coverage under Vitest

[gitar-bot]

⚠️ Bug: Wind efficiency inverted vs. documented "wind origin" semantics

The docstring states windDegrees is the "Wind origin direction" (the direction the wind blows FROM). With that convention, a ship whose heading equals the wind-origin bearing is sailing directly INTO the wind (a headwind), yet the code gives that case angleDiff = 0 and the maximum 1.25x tailwind boost, while sailing away from the wind origin (angleDiff = 180) yields the 0.35x headwind penalty. The physics is reversed relative to the documented input. Either flip the efficiency curve (use (1 - Math.cos(rad)) / 2) or fix the docstring to say windDegrees is the wind's travel/heading-to direction.

Invert the cosine term so a wind originating from directly ahead is a headwind (0.35x) and one from astern is a tailwind (1.25x), matching the documented "wind origin" semantics.:

// windDegrees = direction wind blows FROM; tailwind when it opposes heading.
const rad = (angleDiff * Math.PI) / 180;
const windEfficiency = 0.35 + 0.90 * ((1 - Math.cos(rad)) / 2);

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Bug: Null defender reported as sunk (isTargetSunk: true)

When defender is null/undefined, the volley returns isTargetSunk: true and remainingTargetHp: 0, conflating "invalid/missing target" with "target destroyed." A caller that treats isTargetSunk as a kill confirmation could wrongly credit a sink. Only set isTargetSunk: true when the defender actually exists and is sunk; for a missing defender report isTargetSunk: false with the descriptive reason.

Fix:

if (!defender) {
    return { damageDealt: 0, isTargetSunk: false, remainingTargetHp: 0, isOnCooldown: false, reason: "Target vessel is invalid." };
}
if (defender.isSunk) {
    return { damageDealt: 0, isTargetSunk: true, remainingTargetHp: 0, isOnCooldown: false, reason: "Target vessel is already sunken." };
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Quality: Inconsistent handling of invalid enum inputs

constructVessel throws on an unknown vesselClass but silently falls back to a 1.0 multiplier (?? 1.0) for an unknown hullMaterial. This asymmetry means a typo'd/invalid hull material is accepted and produces an OAK-equivalent ship with no signal to the caller. Either validate both (throw on unknown hull) or document that unknown hulls default to 1.0, so behavior is predictable.

Fix:

const hullFactor = HULL_MULTIPLIERS[hullMaterial];
if (hullFactor === undefined) {
    throw new Error(`Unsupported hull material: ${String(hullMaterial)}`);
}
const totalMaxHp = Math.floor(bp.baseMaxHp * hullFactor);

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Quality: repairVessel returns currentHp: 0 for sunk vessel

When rejecting a repair on a sunk vessel, the function returns currentHp: 0 regardless of the vessel's actual currentHp. This is misleading for callers that read currentHp from the result. Return the vessel's real currentHp on the failure path.

Fix:

if (!vessel || vessel.isSunk) {
    return { success: false, healedHp: 0, currentHp: vessel?.currentHp ?? 0, reason: "Cannot drydock repair a sunken vessel." };
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 4 findings

Implements naval shipyard construction, wind-based sailing mechanics, and cannon volleys for the OpenAO MMORPG. Wind efficiency calculation is inverted relative to the documented "wind origin" semantics—a ship sailing into the wind receives a tailwind boost instead of a headwind penalty. Additionally, null defenders are incorrectly reported as sunk, hull material validation is inconsistent with vessel class validation, and repairVessel returns misleading HP values on failed repairs. These issues should be resolved before merge.

⚠️ Bug: Wind efficiency inverted vs. documented "wind origin" semantics

📄 api/src/lib/navalShipyardTradeVessel.ts:117-131

The docstring states windDegrees is the "Wind origin direction" (the direction the wind blows FROM). With that convention, a ship whose heading equals the wind-origin bearing is sailing directly INTO the wind (a headwind), yet the code gives that case angleDiff = 0 and the maximum 1.25x tailwind boost, while sailing away from the wind origin (angleDiff = 180) yields the 0.35x headwind penalty. The physics is reversed relative to the documented input. Either flip the efficiency curve (use (1 - Math.cos(rad)) / 2) or fix the docstring to say windDegrees is the wind's travel/heading-to direction.

Invert the cosine term so a wind originating from directly ahead is a headwind (0.35x) and one from astern is a tailwind (1.25x), matching the documented "wind origin" semantics.

// windDegrees = direction wind blows FROM; tailwind when it opposes heading.
const rad = (angleDiff * Math.PI) / 180;
const windEfficiency = 0.35 + 0.90 * ((1 - Math.cos(rad)) / 2);

💡 Bug: Null defender reported as sunk (isTargetSunk: true)

📄 api/src/lib/navalShipyardTradeVessel.ts:160-162

When defender is null/undefined, the volley returns isTargetSunk: true and remainingTargetHp: 0, conflating "invalid/missing target" with "target destroyed." A caller that treats isTargetSunk as a kill confirmation could wrongly credit a sink. Only set isTargetSunk: true when the defender actually exists and is sunk; for a missing defender report isTargetSunk: false with the descriptive reason.

Fix

if (!defender) {
    return { damageDealt: 0, isTargetSunk: false, remainingTargetHp: 0, isOnCooldown: false, reason: "Target vessel is invalid." };
}
if (defender.isSunk) {
    return { damageDealt: 0, isTargetSunk: true, remainingTargetHp: 0, isOnCooldown: false, reason: "Target vessel is already sunken." };
}

💡 Quality: Inconsistent handling of invalid enum inputs

📄 api/src/lib/navalShipyardTradeVessel.ts:95-100

constructVessel throws on an unknown vesselClass but silently falls back to a 1.0 multiplier (?? 1.0) for an unknown hullMaterial. This asymmetry means a typo'd/invalid hull material is accepted and produces an OAK-equivalent ship with no signal to the caller. Either validate both (throw on unknown hull) or document that unknown hulls default to 1.0, so behavior is predictable.

Fix

const hullFactor = HULL_MULTIPLIERS[hullMaterial];
if (hullFactor === undefined) {
    throw new Error(`Unsupported hull material: ${String(hullMaterial)}`);
}
const totalMaxHp = Math.floor(bp.baseMaxHp * hullFactor);

💡 Quality: repairVessel returns currentHp: 0 for sunk vessel

📄 api/src/lib/navalShipyardTradeVessel.ts:208-210

When rejecting a repair on a sunk vessel, the function returns currentHp: 0 regardless of the vessel's actual currentHp. This is misleading for callers that read currentHp from the result. Return the vessel's real currentHp on the failure path.

Fix

if (!vessel || vessel.isSunk) {
    return { success: false, healedHp: 0, currentHp: vessel?.currentHp ?? 0, reason: "Cannot drydock repair a sunken vessel." };
}

🤖 Prompt for agents

Code Review: Implements naval shipyard construction, wind-based sailing mechanics, and cannon volleys for the OpenAO MMORPG. Wind efficiency calculation is inverted relative to the documented "wind origin" semantics—a ship sailing into the wind receives a tailwind boost instead of a headwind penalty. Additionally, null defenders are incorrectly reported as sunk, hull material validation is inconsistent with vessel class validation, and `repairVessel` returns misleading HP values on failed repairs. These issues should be resolved before merge.

1. ⚠️ Bug: Wind efficiency inverted vs. documented "wind origin" semantics
   Files: api/src/lib/navalShipyardTradeVessel.ts:117-131

   The docstring states `windDegrees` is the "Wind origin direction" (the direction the wind blows FROM). With that convention, a ship whose heading equals the wind-origin bearing is sailing directly INTO the wind (a headwind), yet the code gives that case `angleDiff = 0` and the maximum 1.25x tailwind boost, while sailing away from the wind origin (angleDiff = 180) yields the 0.35x headwind penalty. The physics is reversed relative to the documented input. Either flip the efficiency curve (use `(1 - Math.cos(rad)) / 2`) or fix the docstring to say `windDegrees` is the wind's travel/heading-to direction.

   Fix (Invert the cosine term so a wind originating from directly ahead is a headwind (0.35x) and one from astern is a tailwind (1.25x), matching the documented "wind origin" semantics.):
   // windDegrees = direction wind blows FROM; tailwind when it opposes heading.
   const rad = (angleDiff * Math.PI) / 180;
   const windEfficiency = 0.35 + 0.90 * ((1 - Math.cos(rad)) / 2);

2. 💡 Bug: Null defender reported as sunk (isTargetSunk: true)
   Files: api/src/lib/navalShipyardTradeVessel.ts:160-162

   When `defender` is null/undefined, the volley returns `isTargetSunk: true` and `remainingTargetHp: 0`, conflating "invalid/missing target" with "target destroyed." A caller that treats `isTargetSunk` as a kill confirmation could wrongly credit a sink. Only set `isTargetSunk: true` when the defender actually exists and is sunk; for a missing defender report `isTargetSunk: false` with the descriptive reason.

   Fix:
   if (!defender) {
       return { damageDealt: 0, isTargetSunk: false, remainingTargetHp: 0, isOnCooldown: false, reason: "Target vessel is invalid." };
   }
   if (defender.isSunk) {
       return { damageDealt: 0, isTargetSunk: true, remainingTargetHp: 0, isOnCooldown: false, reason: "Target vessel is already sunken." };
   }

3. 💡 Quality: Inconsistent handling of invalid enum inputs
   Files: api/src/lib/navalShipyardTradeVessel.ts:95-100

   `constructVessel` throws on an unknown `vesselClass` but silently falls back to a 1.0 multiplier (`?? 1.0`) for an unknown `hullMaterial`. This asymmetry means a typo'd/invalid hull material is accepted and produces an OAK-equivalent ship with no signal to the caller. Either validate both (throw on unknown hull) or document that unknown hulls default to 1.0, so behavior is predictable.

   Fix:
   const hullFactor = HULL_MULTIPLIERS[hullMaterial];
   if (hullFactor === undefined) {
       throw new Error(`Unsupported hull material: ${String(hullMaterial)}`);
   }
   const totalMaxHp = Math.floor(bp.baseMaxHp * hullFactor);

4. 💡 Quality: repairVessel returns currentHp: 0 for sunk vessel
   Files: api/src/lib/navalShipyardTradeVessel.ts:208-210

   When rejecting a repair on a sunk vessel, the function returns `currentHp: 0` regardless of the vessel's actual `currentHp`. This is misleading for callers that read `currentHp` from the result. Return the vessel's real `currentHp` on the failure path.

   Fix:
   if (!vessel || vessel.isSunk) {
       return { success: false, healedHp: 0, currentHp: vessel?.currentHp ?? 0, reason: "Cannot drydock repair a sunken vessel." };
   }

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

---

Important

Your trial ends in 5 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}