Consider named params struct for bc_cloak_tick to prevent f32 arg swap #180

Source

CodeRabbit review on PR #179 (nitpick/trivial, raised in rounds 2 and 3).

Findings

1. Round 2: bc_cloak_tick(bc_ship_state_t *ship, f32 cloak_efficiency, f32 dt) has two adjacent f32 parameters that could be accidentally swapped. CodeRabbit suggests wrapping them in a named struct (e.g., bc_cloak_params).

2. Round 3: Same concern — suggests reordering parameters to (ship, dt, cloak_efficiency) to match other tick functions, or adding inline parameter comments at all call sites.

Reasoning for deferral

• Every other function in the codebase uses direct parameters — introducing a params struct or reordering would be inconsistent with the established API style.
• Other tick functions (bc_combat_charge_tick, bc_repair_tick) also take multiple f32 args without structs.
• There are currently only 2 call sites (main.c simulation tick and test_dynamic_battle.c), both correct.
• The function declaration comment in combat.h clearly documents each parameter's meaning and range.
• The main.c call site uses a descriptive local clk_eff variable, making the intent clear.
• This would require changing the header, implementation, and all call sites for a hypothetical future mistake.
• Better addressed at a broader level if the project adopts a params-struct convention for multi-float APIs.

If implemented

Option A (struct):

• Define typedef struct { f32 cloak_efficiency; f32 dt; } bc_cloak_params_t;
• Change bc_cloak_tick to accept const bc_cloak_params_t *params
• Update all call sites

Option B (reorder):

• Change signature to bc_cloak_tick(bc_ship_state_t *ship, f32 dt, f32 cloak_efficiency)
• Update all call sites and tests