Skip to content

Rework of NPC conversation system#4211

Draft
Henrybk wants to merge 1 commit into
masterfrom
NPC_conversation_rework
Draft

Rework of NPC conversation system#4211
Henrybk wants to merge 1 commit into
masterfrom
NPC_conversation_rework

Conversation

@Henrybk
Copy link
Copy Markdown
Contributor

@Henrybk Henrybk commented May 13, 2026

Summary

This PR reworks OpenKore's NPC conversation handling so that NPC dialog is treated as a first-class runtime state instead of a loosely shared pair of globals.

A new central module, NPC::Conversation, is now the single source of truth for NPC conversation state and interaction flow. Packet receive code, command code, Task::TalkNPC, and eventMacro state checks now go through that module instead of directly reading or mutating %talk or $ai_v{'npc_talk'}.

Problem

NPC conversations were previously coordinated through shared global state, mainly:

  • %talk
  • $ai_v{'npc_talk'}

That made the conversation flow fragile and hard to reason about:

  • multiple code paths read and wrote the same state directly
  • state transitions were duplicated across receive, command, and task logic
  • stale dialog state could leak between conversations
  • plugins/macros had event access, but not reliable state access
  • it was easy to send invalid replies when local state and server state drifted

What this PR changes

1. Add a central NPC conversation module

Introduces:

  • src/NPC/Conversation.pm

This module owns the normalized NPC conversation state, including:

  • whether a conversation is active
  • current NPC id / name
  • current prompt state
  • accumulated text and text lines
  • response options
  • current expected input type
  • sequence id / timestamps
  • waiting-for-server state
  • compatibility sync for legacy globals

It also provides send-side helpers such as:

  • start
  • continue
  • select_response
  • select_response_text
  • select_response_regex
  • send_number
  • send_text
  • choose_buy_or_sell
  • close
  • cancel
  • reset
  • debug_string

2. Migrate packet receive handling

Updated receive-side NPC packet handling to route through NPC::Conversation instead of mutating %talk / $ai_v{'npc_talk'} directly.

Main areas updated:

  • src/Network/Receive.pm
  • NPC text packets
  • continue packets
  • menu/response packets
  • number input packets
  • text input packets
  • close/clear packets
  • shop / store / sell / cash-dealer flows
  • NPC image state

3. Migrate command/task interaction

Updated command and task logic to consume the centralized state instead of reading legacy globals directly.

Main areas updated:

  • src/Commands.pm
  • src/Task/TalkNPC.pm
  • src/Actor/NPC.pm
  • src/Misc.pm
  • src/AI/CoreLogic.pm
  • src/Task/MapRoute.pm
  • src/functions.pl
  • src/Network/Send.pm

This preserves existing behavior while moving orchestration into the conversation module.

4. Add direct NPC conversation commands

Adds direct state-based commands:

  • npcTalkContinue
  • npcTalkSelect
  • npcTalkSelectRegex
  • npcTalkNumber
  • npcTalkText
  • npcTalkClose
  • npcTalkCancel
  • npcTalkReset
  • npcTalkDebug

These commands now use NPC::Conversation and do not touch %talk directly.

5. Add eventMacro state support

Adds state-style NPC conditions for eventMacro so macros can inspect the currently open NPC dialog instead of relying only on NpcMsg event hooks.

New condition modules include:

  • npcTalkActive
  • npcTalkState
  • npcTalkText
  • npcTalkTextRegex
  • npcTalkHasResponses
  • npcTalkResponseCount
  • npcTalkResponse
  • npcTalkResponseRegex
  • npcTalkExpectsContinue
  • npcTalkExpectsResponse
  • npcTalkExpectsNumber
  • npcTalkExpectsText
  • npcTalkNpcName
  • npcTalkNpcId

Existing NpcMsg* behavior is preserved for compatibility.

6. Add tests and guardrails

Adds:

  • src/test/NPCConversationTest.pm
  • src/test/NPCConversationStaticTest.pm

The static test enforces the new rule that production code outside NPC::Conversation must not directly access:

  • %talk
  • $talk{...}
  • $ai_v{'npc_talk'}

7. Add docs and rAthena test script

Adds:

  • docs/NPCConversation.md
  • docs/rathena-npc-conversation-test.txt

The docs describe the new state model, API, hooks, commands, eventMacro usage, and migration guidance for plugin authors.

Compatibility notes

  • %talk and $ai_v{'npc_talk'} still exist as compatibility mirrors, but they are now synchronized only inside NPC::Conversation.
  • Existing npc_talk / npc_talk_done style hook flows remain available.
  • Existing talk / talknpc behavior is preserved while using the new module internally.
  • Menu response handling preserves OpenKore’s historical indexing behavior.

Testing

Verified:

  • static no-direct-access guard passes via NPCConversationStaticTest
  • grep checks only leave intentional compatibility/test/declaration hits
  • git diff --check passes

Not fully verified in this environment:

  • full Perl unit test run is currently blocked by local XSTools.dll loading/runtime issues
  • live rAthena manual flow coverage was prepared via docs/rathena-npc-conversation-test.txt but not executed in this session

Follow-up

Potential follow-up work after this lands:

  • remove legacy %talk / $ai_v{'npc_talk'} mirrors entirely once downstream compatibility risk is gone
  • add broader live integration coverage around shop/storage/quest NPC flows
  • migrate any external plugins still reading legacy globals onto NPC::Conversation

@Henrybk Henrybk marked this pull request as draft May 13, 2026 19:32
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.

1 participant