fix: align NPC public lore and dialog exit#110
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39d09ef384
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| if (targetId && !isNearbyActor(world, targetId)) { | ||
| decision.target_id = ""; | ||
| } | ||
| decision.say = sanitizePublicNpcSpeech(decision.say); |
There was a problem hiding this comment.
The new sanitizer is applied to every say decision unconditionally, so even lore that is intentionally unlocked for specialist NPCs or quest scenes is rewritten to public-safe euphemisms. In those contexts the agent can no longer deliver required explanations (for example, explicit reincarnation or transfer details), which conflicts with the same commit's prompt rule that these terms are allowed when context explicitly unlocks them.
Useful? React with 👍 / 👎.
| text = text.replace(/\bReincarnation\b/g, "A clinic rumor"); | ||
| text = text.replace(/\breincarnation\b/g, "clinic rumor"); |
There was a problem hiding this comment.
Several restricted-term rewrites are case-sensitive (/g), so model output like FRAME, FRAMES, or REINCARNATION bypasses the boundary filter and can still leak the exact terms this guardrail is meant to suppress. This is reproducible with emphasized or adversarially prompted uppercase responses, and it undermines the reliability of the new public-speech boundary.
Useful? React with 👍 / 👎.
| text = text.replace(/\bReincarnation\b/g, "A clinic rumor"); | ||
| text = text.replace(/\breincarnation\b/g, "clinic rumor"); |
There was a problem hiding this comment.
The blanket reincarnation replacement also rewrites literal names and titles, so lines that should reference concrete places or factions (for example, Reincarnation Ward) are transformed into malformed text like A clinic rumor Ward. This can corrupt navigation or quest guidance whenever NPC speech needs to cite canonical proper nouns instead of discussing mechanics.
Useful? React with 👍 / 👎.
| CharacterWeaponStyle.TwoHandSword => new CharacterAnimationCommand(CharacterAnimationCommandKind.State, "2Handed - Idle - Idle"), | ||
| CharacterWeaponStyle.TwoHandSpear => new CharacterAnimationCommand(CharacterAnimationCommandKind.State, "Spearman - Idle - Idle"), | ||
| CharacterWeaponStyle.TwoHandAxe => new CharacterAnimationCommand(CharacterAnimationCommandKind.State, "Hammer - Idle - Idle"), | ||
| CharacterWeaponStyle.TwoHandBow => new CharacterAnimationCommand(CharacterAnimationCommandKind.State, "Archer - Idle - Idle"), |
There was a problem hiding this comment.
The newly added idle mappings use short labels like 2Handed - Idle - Idle, but this animation system resolves state hashes via Animator.HasState and the existing intent registry uses full-path names (for example Base Layer...) for that reason. With short labels, TryPlayStateImmediately will usually fail to find the state, so exiting dialog won't reliably snap back to idle as intended.
Useful? React with 👍 / 👎.
1 participant
Summary
Local code review
Verification
npm run buildinbackend/nakamanpm testinbackend/nakamagit diff --cached --check