🧹 Good first issue: two tiny safety fixes (dup id="map-row" + weaponModels length guard)

[longmaolab]

TL;DR (English)

First off — this codebase is genuinely impressive. ~117 weapons, 39 maps, bots with AI, a killcam, a full shop economy, even AI chat... built solo. That's a lot of working machinery, and the fact that it all holds together is the hard part. 🙌

This issue bundles two small, safe fixes — the kind pros do all the time to keep a big project from quietly breaking later. Neither changes how the game plays. Both are great "build confidence" wins.

• (A) Two HTML elements share id="map-row" — and they list different maps. Duplicate IDs are invalid HTML and getElementById only ever finds the first one.
• (B) WEAPONS[] and weaponModels[] are parallel arrays (both 117 entries). Nothing checks they stay the same length, and switchWeapon() doesn't guard the upper bound — so a future drift could crash silently.

---

(A) Duplicate id="map-row" with mismatched map lists 🗺️

Where (verified):

• public/index.html:482 — <div id="map-row" ...> (the big list, 37 maps — includes arena, biosphere)
• public/index.html:537 — <div id="map-row" ...> (a smaller list, 24 maps — includes airport, trenches, chernobyl)

The catch: an id is supposed to be unique on a page. When two elements share one, the browser keeps both but document.getElementById("map-row") only ever returns the first one (line 482). So any JS wiring up the map picker silently operates on just one of the two rows.

It bites harder because the two rows aren't even the same: the second row is missing many maps the first has (arena, biosphere, carrier, overgrowth, orbital_station, foundry, carnival, lockdown, studio, temple, holiday, labyrinth, opera, doomsday, train, dreamscape...), and the first row is missing airport, trenches, chernobyl. So depending on which screen a player is on, the available maps differ — and that's an easy "why can't I pick this map?" bug to chase later.

Tiny fix — give the second one its own name:

<!-- public/index.html:537 -->
<div id="map-row-compact" style="display:flex; gap:6px; justify-content:center; flex-wrap:wrap;">

Then update whatever JS targets that second selector to use map-row-compact. (Quick check: grep -n "map-row" public/*.js to see who references it.)

💡 Bonus, totally optional: if both rows are meant to show the same maps, it's worth syncing the two lists so players see the same options everywhere. But just de-duplicating the id is a complete, valid fix on its own.

---

(B) Guard weaponModels[] so it can't silently drift from WEAPONS[] 🔫

Where (verified):

• public/game.js:8423-8437 — switchWeapon(idx). Line 8424 guards idx < 0 but not the upper bound:

  function switchWeapon(idx) {
    if (idx === null || idx === undefined || idx < 0) return;  // ← no idx >= WEAPONS.length check
    ...
    weaponModels[currentWeaponIdx].visible = false;  // line 8428
    ...
    weaponModels[idx].visible = true;                // line 8431

• WEAPONS[] and weaponModels[] each have exactly 117 entries today, built in matching order. (Even the code comments warn about it: game.js:800 "must stay LAST — mirrored in weaponModels", and game.js:6115 "must match WEAPONS order".)

Why it bites later: these are parallel arrays — weaponModels[i] is the 3D model for WEAPONS[i]. They only work if they're identical length and order. Right now nothing enforces that. The day you add a weapon to WEAPONS[] but forget the matching weaponModels[] entry (super easy in a list of 117!), switchWeapon(116) would hit weaponModels[116].visible on a too-short array → undefined.visible → crash, with no hint about why. The CLAUDE.md notes call this exact footgun out as a repeat offender.

Two tiny, independent guards — either is a win, both is ideal:

1. A bounds check in switchWeapon (game.js:8424):

if (idx === null || idx === undefined || idx < 0 || idx >= WEAPONS.length) return;

That stops a bad index from ever reaching weaponModels[idx].

2. A startup length-assertion (put it right after weaponModels is built, near game.js:6131):

console.assert(
  weaponModels.length === WEAPONS.length,
  `weaponModels (${weaponModels.length}) out of sync with WEAPONS (${WEAPONS.length})`
);

Now if the arrays ever drift, you get a clear console warning the instant the game loads — instead of a mystery crash mid-match. This is the "tripwire" habit that turns a 30-minute bug hunt into a 2-second fix.

---

Suggested steps

1. public/index.html:537 → rename id="map-row" to id="map-row-compact", then grep for JS that referenced it and update.
2. public/game.js:8424 → add || idx >= WEAPONS.length to the guard.
3. public/game.js near 6131 → add the console.assert length tripwire.
4. Sanity check: node --check public/game.js, open the game locally, switch a few weapons, confirm both map screens still work.

None of this touches gameplay — it just makes the project harder to accidentally break. Exactly the kind of small, safe polish that separates a good project from a solid one. You've already built the hard 95%; this is the cheap insurance on top. Nice work, and keep going! 🚀

---

中文详细说明

先说一句：这个项目真的很厉害 — ~117 把武器、39 张地图、有 AI 的机器人、击杀回放、完整的商店经济系统、甚至还有 AI 聊天，全是一个人做出来的。能让这么多系统稳定地跑在一起，本身就是最难的部分。👏

这个 issue 打包了两个又小又安全的修复。它们都不会改变游戏玩法，是那种高手平时随手就做、用来防止大项目以后悄悄出 bug 的小习惯。很适合拿来练手、攒信心。

(A) 重复的 id="map-row"，而且两份地图列表还不一样 🗺️

位置（已核实）：

• public/index.html:482 — <div id="map-row" ...>（大列表，37 张地图，含 arena、biosphere）
• public/index.html:537 — <div id="map-row" ...>（小列表，24 张地图，含 airport、trenches、chernobyl）

问题在哪： HTML 里的 id 必须是全页唯一的。两个元素用了同一个 id 时，浏览器两个都留着，但 document.getElementById("map-row") 永远只返回第一个（482 行）。所以任何给地图选择器绑事件的 JS,实际上只作用到了其中一行。

更麻烦的是这两行内容还不一样:第二行缺了第一行有的很多地图(arena、biosphere、carrier、overgrowth、orbital_station、foundry、carnival、lockdown、studio、temple、holiday、labyrinth、opera、doomsday、train、dreamscape……),而第一行又缺了 airport、trenches、chernobyl。结果就是玩家在不同界面看到的可选地图不一样 — 以后很容易变成"为啥这张图选不了?"的诡异 bug。

小修复 — 给第二个起个单独的名字：

<!-- public/index.html:537 -->
<div id="map-row-compact" style="display:flex; gap:6px; justify-content:center; flex-wrap:wrap;">

然后把引用这第二个选择器的 JS 改成用 map-row-compact。（小技巧：grep -n "map-row" public/*.js 看看谁在用它。）

💡 可选的加分项：如果这两行本来就该显示一样的地图,顺手把两份列表同步一下会更好。但光是把重复的 id 改掉,本身就已经是个完整、有效的修复了。

(B) 给 weaponModels[] 加个护栏，别让它和 WEAPONS[] 悄悄错位 🔫

位置（已核实）：

• public/game.js:8423-8437 — switchWeapon(idx)。第 8424 行只挡了 idx < 0,没有挡上界:

  if (idx === null || idx === undefined || idx < 0) return;  // ← 缺 idx >= WEAPONS.length
  ...
  weaponModels[idx].visible = true;  // 8431 行

• WEAPONS[] 和 weaponModels[] 现在都正好 117 项,按相同顺序构建。代码注释自己都在提醒：game.js:800"must stay LAST — mirrored in weaponModels",game.js:6115"must match WEAPONS order"。

为什么以后会咬你： 这俩是平行数组 — weaponModels[i] 就是 WEAPONS[i] 对应的 3D 模型,只有在长度和顺序完全一致时才正常。可现在没有任何东西来保证这一点。哪天你往 WEAPONS[] 加了一把武器、却忘了在 weaponModels[] 加对应的那一项(117 项的列表里太容易漏了!),switchWeapon(116) 就会去访问一个太短数组的 weaponModels[116].visible → undefined.visible → 崩溃,而且完全看不出为什么崩。CLAUDE.md 里专门把这个坑标成了"反复踩"的老问题。

两个又小又互相独立的护栏,做一个就有收益,两个都做最稳：

1. 在 switchWeapon 里加上界检查（game.js:8424）：

if (idx === null || idx === undefined || idx < 0 || idx >= WEAPONS.length) return;

这样一个越界的下标根本到不了 weaponModels[idx]。

2. 在启动时加一条长度断言（放在 weaponModels 构建完之后,大约 game.js:6131 附近）：

console.assert(
  weaponModels.length === WEAPONS.length,
  `weaponModels (${weaponModels.length}) out of sync with WEAPONS (${WEAPONS.length})`
);

这样一旦两个数组对不上,游戏一加载控制台就会立刻给出清楚的警告 — 而不是等到对局打到一半才神秘崩溃。这种"绊线报警"的习惯,能把一次 30 分钟的找 bug 变成 2 秒钟的修 bug。

建议步骤

1. public/index.html:537 → 把 id="map-row" 改成 id="map-row-compact",再 grep 出引用它的 JS 一起改。
2. public/game.js:8424 → 在判断里加上 || idx >= WEAPONS.length。
3. public/game.js 约 6131 行附近 → 加上那条 console.assert 绊线。
4. 验收：node --check public/game.js,本地打开游戏,切几把武器,确认两个地图界面都正常。

这些改动一点都不碰玩法,只是让项目更不容易被不小心搞坏。最难的 95% 你已经做完了,这只是上面那层便宜的"保险"。做得很棒,继续加油!🚀