From 9e1ce5ab4292e9d707516aa84965530061ab095a Mon Sep 17 00:00:00 2001 From: MorquinDevlar Date: Mon, 27 Jul 2026 11:11:20 +0200 Subject: [PATCH] fix(conversations): initialize LastRound so fresh conversations survive cleanup getConversation has a 2% chance per call of deleting any conversation more than 10 rounds stale, measured as rounds since LastRound. New conversations left LastRound at zero while the round counter starts at 1314000, so a brand-new conversation was always eligible: any lookup that won the maintenance roll deleted it before its first action fired. This is also why TestAttemptConversation_UsesPluginFile fails on roughly 2% of CI runs - the test creates a conversation and immediately looks it up. LastRound now starts at the current round, matching StartRound. Adds a deterministic regression test. --- internal/conversations/conversations.go | 9 ++++++-- internal/conversations/plugin_test.go | 28 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/internal/conversations/conversations.go b/internal/conversations/conversations.go index a7e8e7a84..ea8a09687 100644 --- a/internal/conversations/conversations.go +++ b/internal/conversations/conversations.go @@ -102,8 +102,13 @@ func AttemptConversation(initiatorMobId int, initatorInstanceId int, initiatorNa MobInstanceId1: initatorInstanceId, MobInstanceId2: participantInstanceId, StartRound: util.GetRoundCount(), - Position: 0, - ActionList: dataFile[chosenIndex].Conversation, + // LastRound must start at the current round - the cleanup in + // getConversation treats rounds since LastRound as staleness, and a + // zero value makes a brand-new conversation look ancient and + // eligible for deletion before its first action fires. + LastRound: util.GetRoundCount(), + Position: 0, + ActionList: dataFile[chosenIndex].Conversation, } return conversationUniqueId diff --git a/internal/conversations/plugin_test.go b/internal/conversations/plugin_test.go index a74e1459f..a621cc163 100644 --- a/internal/conversations/plugin_test.go +++ b/internal/conversations/plugin_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/GoMudEngine/GoMud/internal/mudlog" + "github.com/GoMudEngine/GoMud/internal/util" ) func TestMain(m *testing.M) { @@ -119,3 +120,30 @@ func TestReadPluginConversationFile_KeyFormat(t *testing.T) { t.Fatalf("did not expect a file for mob 43") } } + +// TestAttemptConversation_FreshConversationNotStale guards against a fresh +// conversation being eligible for the random cleanup in getConversation. A +// zero LastRound made new conversations look ancient (the round counter +// starts above one million), so any lookup that won the 2% maintenance roll +// deleted a conversation that was just created. +func TestAttemptConversation_FreshConversationNotStale(t *testing.T) { + resetPluginState() + defer resetPluginState() + + RegisterFS(newFakeFS(map[string][]byte{ + `conversations/testzone/9001.yaml`: []byte(sampleConversation), + })) + + convId := AttemptConversation(9001, 1, "goblin", 2, "rat", "TestZone") + if convId == 0 { + t.Fatalf("expected a non-zero conversation id from plugin file") + } + + c := conversations[convId] + if c == nil { + t.Fatalf("expected conversation to be stored") + } + if c.LastRound != util.GetRoundCount() { + t.Fatalf("expected fresh conversation LastRound to equal the current round %d, got %d", util.GetRoundCount(), c.LastRound) + } +}