From 982393daf497b18cc78bf897989d80889f8d09df Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Fri, 30 Jan 2026 01:01:21 +0100 Subject: [PATCH] test: add unit tests for cherry-pick impacted handlers Add comprehensive tests documenting current behavior before applying fixes from main branch. Tests cover: - Cafe item PointCost parsing (uint32 vs uint16 for different client modes) - Guild poogie outfit unlock calculation bug (math.Pow issue) - Guild manage right nil pointer condition (&& vs || logic) - InfoGuild applicant GR field size for adds 9 to pugi_outfits +// +// EXPECTED BEHAVIOR (after fix commit 7459ded): +// +// pugi_outfits = outfitID +// Example: outfitID=3 -> sets pugi_outfits to 3 +// +// The pugi_outfits field is a bitmask where each bit represents an unlocked outfit. +// The current math.Pow calculation is completely wrong for a bitmask. +func TestPoogiOutfitUnlockCalculation(t *testing.T) { + tests := []struct { + name string + outfitID uint32 + currentBuggy int // What the current buggy code produces + expectedFixed uint32 // What the fix should produce + }{ + { + name: "outfit 0", + outfitID: 0, + currentBuggy: int(math.Pow(float64(0), 2)), // 0 + expectedFixed: 0, + }, + { + name: "outfit 1", + outfitID: 1, + currentBuggy: int(math.Pow(float64(1), 2)), // 1 + expectedFixed: 1, + }, + { + name: "outfit 2", + outfitID: 2, + currentBuggy: int(math.Pow(float64(2), 2)), // 4 (WRONG!) + expectedFixed: 2, + }, + { + name: "outfit 3", + outfitID: 3, + currentBuggy: int(math.Pow(float64(3), 2)), // 9 (WRONG!) + expectedFixed: 3, + }, + { + name: "outfit 10", + outfitID: 10, + currentBuggy: int(math.Pow(float64(10), 2)), // 100 (WRONG!) + expectedFixed: 10, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Verify our understanding of the current buggy behavior + buggyResult := int(math.Pow(float64(tt.outfitID), 2)) + if buggyResult != tt.currentBuggy { + t.Errorf("buggy calculation = %d, expected %d", buggyResult, tt.currentBuggy) + } + + // Document that the fix should just use the outfitID directly + fixedResult := tt.outfitID + if fixedResult != tt.expectedFixed { + t.Errorf("fixed calculation = %d, expected %d", fixedResult, tt.expectedFixed) + } + + // Show the difference + if tt.outfitID > 1 { + if buggyResult == int(tt.expectedFixed) { + t.Logf("outfit %d: buggy and fixed results match (this is coincidental)", tt.outfitID) + } else { + t.Logf("outfit %d: buggy=%d, fixed=%d (BUG DOCUMENTED)", tt.outfitID, buggyResult, tt.expectedFixed) + } + } + }) + } +} + +// TestGuildManageRightNilPointerCondition documents the nil pointer bug in handleMsgMhfGetGuildManageRight. +// +// CURRENT BEHAVIOR (BUG - commit 5028355): +// +// if guild == nil && s.prevGuildID != 0 { +// This means: only try prevGuildID lookup if guild is nil AND prevGuildID is set +// +// EXPECTED BEHAVIOR (after fix): +// +// if guild == nil || s.prevGuildID != 0 { +// This means: try prevGuildID lookup if guild is nil OR prevGuildID is set +// +// The bug causes incorrect behavior when: +// - guild is NOT nil (player has a guild) +// - BUT s.prevGuildID is also set (player recently left a guild) +// In this case, the old code would NOT use prevGuildID, but the new code would. +func TestGuildManageRightNilPointerCondition(t *testing.T) { + tests := []struct { + name string + guildIsNil bool + prevGuildID uint32 + shouldUsePrevGuild bool // What the condition evaluates to + buggyBehavior bool // Current buggy && condition + fixedBehavior bool // Fixed || condition + }{ + { + name: "no guild, no prevGuildID", + guildIsNil: true, + prevGuildID: 0, + buggyBehavior: false, // true && false = false + fixedBehavior: true, // true || false = true + shouldUsePrevGuild: false, + }, + { + name: "no guild, has prevGuildID", + guildIsNil: true, + prevGuildID: 42, + buggyBehavior: true, // true && true = true + fixedBehavior: true, // true || true = true + shouldUsePrevGuild: true, + }, + { + name: "has guild, no prevGuildID", + guildIsNil: false, + prevGuildID: 0, + buggyBehavior: false, // false && false = false + fixedBehavior: false, // false || false = false + shouldUsePrevGuild: false, + }, + { + name: "has guild, has prevGuildID - THE BUG CASE", + guildIsNil: false, + prevGuildID: 42, + buggyBehavior: false, // false && true = false (WRONG!) + fixedBehavior: true, // false || true = true (CORRECT) + shouldUsePrevGuild: true, // Should use prevGuildID + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Simulate the condition evaluation + buggyCondition := tt.guildIsNil && (tt.prevGuildID != 0) + fixedCondition := tt.guildIsNil || (tt.prevGuildID != 0) + + if buggyCondition != tt.buggyBehavior { + t.Errorf("buggy condition = %v, expected %v", buggyCondition, tt.buggyBehavior) + } + if fixedCondition != tt.fixedBehavior { + t.Errorf("fixed condition = %v, expected %v", fixedCondition, tt.fixedBehavior) + } + + // Document when the bug manifests + if buggyCondition != fixedCondition { + t.Logf("BUG: %s - buggy=%v, fixed=%v", tt.name, buggyCondition, fixedCondition) + } + }) + } +} + +// TestOperateGuildConstants verifies guild operation constants are defined correctly. +func TestOperateGuildConstants(t *testing.T) { + // Test that the unlock outfit constant exists and has expected value + if mhfpacket.OPERATE_GUILD_UNLOCK_OUTFIT != 0x12 { + t.Errorf("OPERATE_GUILD_UNLOCK_OUTFIT = 0x%X, want 0x12", mhfpacket.OPERATE_GUILD_UNLOCK_OUTFIT) + } +} + +// TestGuildMemberInfo tests the GuildMember struct behavior. +func TestGuildMemberInfo(t *testing.T) { + member := &GuildMember{ + CharID: 12345, + GuildID: 100, + Name: "TestHunter", + Recruiter: true, + HRP: 500, + GR: 50, + } + + if member.CharID != 12345 { + t.Errorf("CharID = %d, want 12345", member.CharID) + } + if !member.Recruiter { + t.Error("Recruiter should be true") + } + if member.HRP != 500 { + t.Errorf("HRP = %d, want 500", member.HRP) + } + if member.GR != 50 { + t.Errorf("GR = %d, want 50", member.GR) + } +} + +// TestInfoGuildApplicantGRFieldSize documents the client mode difference for = maxIterations { + return iterations // Safety break for test + } + // In real code: time.Sleep(1 * time.Second) + } + } + + // Binary never arrives + neverReturns := func() bool { return false } + result := simulateCurrentBehavior(neverReturns) + + if result < maxIterations { + t.Errorf("expected loop to hit safety limit (%d), got %d", maxIterations, result) + } + t.Logf("Current behavior would loop forever (hit safety limit at %d iterations)", result) + }) + + t.Run("fixed behavior has timeout", func(t *testing.T) { + // Simulate fixed behavior with timeout + const maxTimeout = 10 // Maximum iterations before timeout + + simulateFixedBehavior := func(getBinary func() bool) (int, bool) { + for i := 0; i < maxTimeout; i++ { + if getBinary() { + return i, true // Found binary + } + // In real code: time.Sleep(1 * time.Second) + } + return maxTimeout, false // Timeout + } + + // Binary never arrives + neverReturns := func() bool { return false } + iterations, found := simulateFixedBehavior(neverReturns) + + if found { + t.Error("expected timeout (not found)") + } + if iterations != maxTimeout { + t.Errorf("expected %d iterations before timeout, got %d", maxTimeout, iterations) + } + t.Logf("Fixed behavior times out after %d iterations", iterations) + }) + + t.Run("fixed behavior returns quickly when binary exists", func(t *testing.T) { + const maxTimeout = 10 + + // Simulate binary arriving on 3rd check + checkCount := 0 + arrivesOnThird := func() bool { + checkCount++ + return checkCount >= 3 + } + + simulateFixedBehavior := func(getBinary func() bool) (int, bool) { + for i := 0; i < maxTimeout; i++ { + if getBinary() { + return i + 1, true + } + } + return maxTimeout, false + } + + iterations, found := simulateFixedBehavior(arrivesOnThird) + + if !found { + t.Error("expected to find binary") + } + if iterations != 3 { + t.Errorf("expected 3 iterations, got %d", iterations) + } + }) +} + +// TestStageBinaryKeyAccess tests the stageBinaryKey struct used for indexing. +func TestStageBinaryKeyAccess(t *testing.T) { + // Create a stage with binary data + stage := NewStage("test_stage") + + key := stageBinaryKey{id0: 1, id1: 2} + testData := []byte{0xDE, 0xAD, 0xBE, 0xEF} + + // Store binary data + stage.Lock() + stage.rawBinaryData[key] = testData + stage.Unlock() + + // Retrieve binary data + stage.Lock() + data, exists := stage.rawBinaryData[key] + stage.Unlock() + + if !exists { + t.Error("expected binary data to exist") + } + if len(data) != 4 { + t.Errorf("data length = %d, want 4", len(data)) + } + if data[0] != 0xDE { + t.Errorf("data[0] = 0x%X, want 0xDE", data[0]) + } +} + +// TestStageBinaryKeyUniqueness verifies different keys map to different data. +func TestStageBinaryKeyUniqueness(t *testing.T) { + stage := NewStage("test_stage") + + key1 := stageBinaryKey{id0: 1, id1: 2} + key2 := stageBinaryKey{id0: 1, id1: 3} + key3 := stageBinaryKey{id0: 2, id1: 2} + + data1 := []byte{0x01} + data2 := []byte{0x02} + data3 := []byte{0x03} + + stage.Lock() + stage.rawBinaryData[key1] = data1 + stage.rawBinaryData[key2] = data2 + stage.rawBinaryData[key3] = data3 + stage.Unlock() + + stage.Lock() + defer stage.Unlock() + + if d, ok := stage.rawBinaryData[key1]; !ok || d[0] != 0x01 { + t.Error("key1 data mismatch") + } + if d, ok := stage.rawBinaryData[key2]; !ok || d[0] != 0x02 { + t.Error("key2 data mismatch") + } + if d, ok := stage.rawBinaryData[key3]; !ok || d[0] != 0x03 { + t.Error("key3 data mismatch") + } +} + +// TestStageBinarySpecialCase tests the special case for id0=1, id1=12. +// This case returns immediately with a fixed response instead of waiting. +func TestStageBinarySpecialCase(t *testing.T) { + // The special case is: + // if pkt.id0 == 1 && pkt.id1 == 12 { + // doAckBufSucceed(s, pkt.AckHandle, []byte{0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}) + // return + // } + + expectedResponse := []byte{0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00} + + if len(expectedResponse) != 8 { + t.Errorf("special case response length = %d, want 8", len(expectedResponse)) + } + if expectedResponse[0] != 0x04 { + t.Errorf("special case response[0] = 0x%X, want 0x04", expectedResponse[0]) + } +} + +// TestStageLockingDuringBinaryAccess verifies proper locking during binary data access. +func TestStageLockingDuringBinaryAccess(t *testing.T) { + stage := NewStage("concurrent_test") + key := stageBinaryKey{id0: 1, id1: 1} + + var wg sync.WaitGroup + errors := make(chan error, 100) + + // Concurrent writers + for i := 0; i < 10; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + for j := 0; j < 10; j++ { + stage.Lock() + stage.rawBinaryData[key] = []byte{byte(id), byte(j)} + stage.Unlock() + } + }(i) + } + + // Concurrent readers + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 10; j++ { + stage.Lock() + _ = stage.rawBinaryData[key] + stage.Unlock() + } + }() + } + + wg.Wait() + close(errors) + + for err := range errors { + t.Error(err) + } +} + +// TestWaitStageBinaryTimeoutDuration documents the expected timeout duration. +// After the fix, the handler should wait at most 10 seconds (10 iterations * 1 second sleep). +func TestWaitStageBinaryTimeoutDuration(t *testing.T) { + const ( + sleepDuration = 1 * time.Second + maxIterations = 10 + expectedTimeout = sleepDuration * maxIterations + ) + + if expectedTimeout != 10*time.Second { + t.Errorf("expected timeout = %v, want 10s", expectedTimeout) + } + + t.Logf("After fix, WaitStageBinary will timeout after %v (%d iterations * %v sleep)", + expectedTimeout, maxIterations, sleepDuration) +} diff --git a/server/entranceserver/make_resp_test.go b/server/entranceserver/make_resp_test.go index 4e5f9c843..2bf2de2ef 100644 --- a/server/entranceserver/make_resp_test.go +++ b/server/entranceserver/make_resp_test.go @@ -137,3 +137,69 @@ func TestMakeHeaderConsistency(t *testing.T) { t.Error("makeHeader() with same input should produce same output") } } + +// TestEncodeServerInfoClanMemberLimit documents the hardcoded clan member limit. +// +// CURRENT BEHAVIOR: +// +// bf.WriteUint32(0x0000003C) // Hardcoded to 60 +// +// EXPECTED BEHAVIOR (after fix commit 7d760bd): +// +// bf.WriteUint32(uint32(s.erupeConfig.GameplayOptions.ClanMemberLimits[len(s.erupeConfig.GameplayOptions.ClanMemberLimits)-1][1])) +// This reads the maximum clan member limit from the last entry of ClanMemberLimits config. +// +// Note: The ClanMemberLimits config field doesn't exist in this branch yet. +// This test documents the expected value (60 = 0x3C) that will be read from config. +func TestEncodeServerInfoClanMemberLimit(t *testing.T) { + // The hardcoded value is 60 (0x3C) + hardcodedLimit := uint32(0x0000003C) + + if hardcodedLimit != 60 { + t.Errorf("hardcoded clan member limit = %d, expected 60", hardcodedLimit) + } + + t.Logf("Current implementation uses hardcoded clan member limit: %d", hardcodedLimit) + t.Logf("After fix, this will be read from config.GameplayOptions.ClanMemberLimits") +} + +// TestMakeHeaderDataIntegrity verifies that data passed to makeHeader is preserved +// through encryption/decryption. +func TestMakeHeaderDataIntegrity(t *testing.T) { + testCases := []struct { + name string + data []byte + respType string + count uint16 + key byte + }{ + {"empty SV2", []byte{}, "SV2", 0, 0x00}, + {"simple SVR", []byte{0x01, 0x02}, "SVR", 1, 0x00}, + {"with key", []byte{0xDE, 0xAD, 0xBE, 0xEF}, "SV2", 2, 0x42}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := makeHeader(tc.data, tc.respType, tc.count, tc.key) + + // Result should have key as first byte + if len(result) == 0 { + t.Fatal("makeHeader returned empty result") + } + if result[0] != tc.key { + t.Errorf("first byte = 0x%X, want 0x%X (key)", result[0], tc.key) + } + + // Decrypt and verify response type + if len(result) > 1 { + decrypted := DecryptBin8(result[1:], tc.key) + if len(decrypted) >= 3 { + gotType := string(decrypted[:3]) + if gotType != tc.respType { + t.Errorf("decrypted respType = %s, want %s", gotType, tc.respType) + } + } + } + }) + } +}