Skip to content

fix: 修复阿里安特高级整形券对话无响应#703

Open
skykiseki wants to merge 1 commit into
BeiDouMS:masterfrom
skykiseki:fix/ariant-vard-face-coupon
Open

fix: 修复阿里安特高级整形券对话无响应#703
skykiseki wants to merge 1 commit into
BeiDouMS:masterfrom
skykiseki:fix/ariant-vard-face-coupon

Conversation

@skykiseki

Copy link
Copy Markdown
Contributor

问题

女性角色在八德乐处选择阿里安特整形手术高级会员卡时,NPC 脚本会引用未定义的 fface 变量,导致后续对话中断。

修复

  • 将女性高级整形脸型列表引用从 fface 改为已定义的 fface_v
  • 同步修复英文和中文 NPC 脚本,避免不同语言脚本表现不一致。

影响

  • 仅修复服务端 NPC 脚本逻辑。
  • 不依赖客户端资源更新。

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🤖 AI Code Review

Let me analyze this PR carefully.

The PR modifies two files (Chinese and English versions of the same NPC script):

  • gms-server/scripts-zh-CN/npc/2100008.js
  • gms-server/scripts/npc/2100008.js

The changes:

  1. In the female gender branch (gender == 1), changed references from fface to fface_v
  2. Added a missing newline at end of file

Let me think about what NPC 2100008 likely is - based on the code structure (face selection, gender checking, facial features like fface), this looks like a beauty/face stylist NPC in MapleStory.

Now, let me analyze the issues:

  1. The actual logic change: Changed fface to fface_v in the female branch. Without seeing the full file, I need to infer. If fface was the variable for female faces and fface_v is some variation (perhaps "female face variant"), this could be intentional. However, if fface_v is not defined in the female branch context, this would cause a runtime error.

  2. Male branch not changed: The male branch (presumably if (cm.getChar().getGender() == 0) or similar) still uses fface or some other variable. This creates asymmetry.

  3. The change is mirrored in both language versions - good consistency, but also means the same potential bug is duplicated.

  4. End of file newline - this is a minor fix, good practice.

  5. Variable name fface_v - the _v suffix is unclear. What does v stand for? variant? verified? This naming is unclear without context.

Potential issues:

  • 🟡 Medium: Without seeing the full file, I can't verify if fface_v is defined for females. If it's only defined for males, this would break the female face selection.
  • 🟡 Medium: The male branch (if it exists with fface) is not symmetrically updated. Why only change the female branch?
  • 🟢 Low: The end-of-file newline is added (good).
  • 🟢 Low: Variable naming fface_v is unclear.

Let me also think about whether this could be a bug fix or a new feature. In MapleStory, NPCs like this often have a "regular" face and a "variant" or "premium" face option. Changing fface to fface_v in the female branch might be intentional to show variant faces for females.

But the asymmetry (only female changed) is suspicious. Let me look more carefully at the code to understand.

The code structure shows:

  • Line 58-62: The female branch (gender == 1) is where the change happens
  • This branch is inside a larger conditional block

Looking at the pattern: fface[i] + cm.getChar().getFace() % 1000 - (cm.getChar().getFace() % 100) - this is calculating a new face ID by adding a base face index to the current face's "base" (the thousands digit).

If fface_v is a "variant" list that contains alternative face IDs (perhaps eye variants, mouth variants, etc.), then this change might be intentional to give female characters access to variant faces.

However, the change being limited to female only is suspicious. Either:

  1. Male characters already had fface_v support (but then why not also change their loop? Maybe their loop doesn't exist or uses a different variable)
  2. This is a bug where the wrong variable name was used

Without seeing the full file, I should note this uncertainty.

Let me structure my review:

📋 PR 概览

将 NPC 2100008(疑似整容/美发医师)女性角色分支的脸型数组引用从 fface 改为 fface_v,并在文件末尾补上换行符。

⚠️ 潜在问题

  • 🟡 中危: 变量 fface_v 在女性分支中是否已定义?仅看 diff 无法确认全貌。如果该变量仅在男性分支定义,会导致女性角色脸部选择时报 ReferenceError
  • 🟡 中危: 修改不对称——只改了女性分支。如果这是引入"变体脸型"功能,男性分支是否也需要同样更新?或者是反向:男性分支原本用 mface_v,女性应该用 fface(非 _v)?缺少对称修改需要解释。
  • 🟢 低危: 变量命名 fface_v 不够语义化。_v 代表什么?(variant / verified / vip?) 不明。

💡 改进建议

  1. 在 PR 描述中说明:fface_v 在哪里定义?为什么只改女性分支不涉及男性?
  2. 建议将 _v 改为有意义的名称(如 ffaceVariants / ffacePremium),或者在文件顶部加注释解释。
  3. 由于中英文两版脚本都改了,建议后续只维护一份脚本,通过 i18n 机制或脚本引用避免双份维护漂移。

📊 评分

  • 代码质量: 3/5(变量命名和对称性问题)
  • 安全性: 4/5(无明显安全风险)
  • 可维护性: 3/5(双份维护 + 不明变量名)

Actually, I should be honest - I can't see the full file. I should acknowledge this limitation. But based on what's visible, the review above is reasonable.

📋 PR 概览

NPC 2100008(疑似整形/美发医师)脚本中,将女性角色分支的脸型数组引用由 fface 改为 fface_v,并顺便为两个语言版本文件补上结尾换行。

⚠️ 潜在问题

  • 🟡 中危 — 变量存在性无法确认fface_v 在本 diff 之外的代码中是否已定义?若该变量仅在男性分支(mface_v)或脚本顶部以条件方式声明,女性角色点击相应选项时会直接抛 ReferenceError: fface_v is not defined,导致 NPC 交互中断。
  • 🟡 中危 — 修改不对称:仅修改了 getGender() == 1(女)分支,男性分支(推测为 getGender() == 0)未做对应改动。如果这是为引入"变体脸型 / VIP 脸型"功能,为何男性不一起加?如果是 bug 修复,diff 中也没看到任何说明,缺乏上下文。
  • 🟢 低危 — 命名不语义化fface_v 中的 _v 含义不明(variant? vip? verified?)。fface 本身已用匈牙利风格缩写,再加一个无注释后缀可读性差。
  • 🟢 低危 — 双份维护漂移风险:中英文两套 NPC 脚本内容高度一致但分别维护,本次就是双份同步修改,未来极容易再次漂移。

💡 改进建议

  1. 补全上下文:在 PR 描述里贴出 fface / fface_v 的声明位置(一般是 var fface = [...]var fface_v = [...] 在脚本顶部),并说明本次只改女不改男的理由。
  2. 重命名 + 注释:把 fface_v 改为自解释名称(如 ffaceVariants),或在文件头加一行注释解释 _v 后缀含义,提升后续维护效率。
  3. 长期去重:考虑用 load("npc/2100008_base.js") 让 zh-CN / en 版脚本只持有对话文本差异,根除双份漂移问题(本次恰好踩中)。

📊 评分

维度 分数 说明
代码质量 3/5 命名不清晰 + 修改不对称,缺乏注释
安全性 4/5 无注入/越权类风险,NPC 脚本权限有限
可维护性 3/5 双份维护 + 不明变量名 + 无 PR 说明

⚠️ 备注:以上评估基于 diff 片段,未看到完整脚本(特别是 fface_v 的声明处和男性分支),建议审查时务必对照完整文件确认变量作用域和对称性。

@ziqiming

ziqiming commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

这个确实之前的变量名字写错了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants