diff --git a/CHANGELOG.md b/CHANGELOG.md index 787624144..fe0936a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed lobby search returning all reserved players instead of only quest-bound players — `QuestReserved` field now counts only clients in "Qs" (quest) stages, matching retail behavior ([#167](https://github.com/Mezeporta/Erupe/issues/167)) +- Documented log key off-by-one (RE'd from mhfo-hd.dll ZZ): `putRecord_log`/`putTerminal_log` don't use the key in ZZ, so the server value is unused beyond issuance ([#167](https://github.com/Mezeporta/Erupe/issues/167)) +- Documented user search padding version boundary (8 vs 40 bytes) with RE findings from ZZ DLL; G2 analysis inconclusive ([#167](https://github.com/Mezeporta/Erupe/issues/167)) - Fixed bookshelf save data pointer being off by 14810 bytes for G1–Z2, F4–F5, and S6 game versions — corrected offsets to 103928, 71928, and 23928 respectively ([#164](https://github.com/Mezeporta/Erupe/issues/164)) - Fixed guild alliance application toggle being hardcoded to always-open — now persisted in DB and togglable by the parent guild leader via `OperateJoint` Allow/Deny actions ([#166](https://github.com/Mezeporta/Erupe/issues/166)) - Fixed gacha shop not working on G1–GG clients due to protocol differences in `handleMsgMhfEnumerateShop` when `ShopType` is 1 or 2 — thanks @Sin365 (#150) diff --git a/docs/technical-debt.md b/docs/technical-debt.md index 653306672..3544b56d6 100644 --- a/docs/technical-debt.md +++ b/docs/technical-debt.md @@ -29,9 +29,9 @@ These TODOs represent features that are visibly broken for players. | ~~`model_character.go:88,101,113`~~ | ~~`TODO: fix bookshelf data pointer` for G10-ZZ, F4-F5, and S6 versions~~ | ~~Wrong pointer corrupts character save reads for three game versions.~~ **Fixed.** Corrected offsets to 103928 (G1–Z2), 71928 (F4–F5), 23928 (S6) — validated via inter-version delta analysis and Ghidra decompilation of `snj_db_get_housedata` in the ZZ DLL. | [#164](https://github.com/Mezeporta/Erupe/issues/164) | | `handlers_achievement.go:117` | `TODO: Notify on rank increase` — always returns `false` | Achievement rank-up notifications are silently suppressed. Requires understanding what `MhfDisplayedAchievement` (currently an empty handler) sends to track "last displayed" state. | [#165](https://github.com/Mezeporta/Erupe/issues/165) | | ~~`handlers_guild_info.go:443`~~ | ~~`TODO: Enable GuildAlliance applications` — hardcoded `true`~~ | ~~Guild alliance applications are always open regardless of setting.~~ **Fixed.** Added `recruiting` column to `guild_alliances`, wired `OperateJoint` actions `0x06`/`0x07`, reads from DB. | [#166](https://github.com/Mezeporta/Erupe/issues/166) | -| `handlers_session.go:410` | `TODO(Andoryuuta): log key index off-by-one` | Known off-by-one in log key indexing is unresolved | [#167](https://github.com/Mezeporta/Erupe/issues/167) | -| `handlers_session.go:551` | `TODO: This case might be <=G2` | Uncertain version detection in switch case | [#167](https://github.com/Mezeporta/Erupe/issues/167) | -| `handlers_session.go:714` | `TODO: Retail returned the number of clients in quests` | Player count reported to clients does not match retail behavior | [#167](https://github.com/Mezeporta/Erupe/issues/167) | +| ~~`handlers_session.go:410`~~ | ~~`TODO(Andoryuuta): log key index off-by-one`~~ | ~~Known off-by-one in log key indexing is unresolved~~ **Documented.** RE'd from ZZ DLL: `putRecord_log`/`putTerminal_log` don't embed the key (size 0), so the off-by-one only matters in pre-ZZ clients and is benign server-side. | [#167](https://github.com/Mezeporta/Erupe/issues/167) | +| ~~`handlers_session.go:551`~~ | ~~`TODO: This case might be <=G2`~~ | ~~Uncertain version detection in switch case~~ **Documented.** RE'd ZZ per-entry parser (FUN_115868a0) confirms 40-byte padding. G2 DLL analysis inconclusive (stripped, no shared struct sizes). Kept <=G1 boundary with RE documentation. | [#167](https://github.com/Mezeporta/Erupe/issues/167) | +| ~~`handlers_session.go:714`~~ | ~~`TODO: Retail returned the number of clients in quests`~~ | ~~Player count reported to clients does not match retail behavior~~ **Fixed.** Added `QuestReserved` field to `StageSnapshot` that counts only clients in "Qs" stages, pre-collected under server lock to respect lock ordering. | [#167](https://github.com/Mezeporta/Erupe/issues/167) | | `msg_mhf_add_ud_point.go:28` | `TODO: Parse is a stub` — field meanings unknown | UD point packet fields unnamed, `Build` not implemented | [#168](https://github.com/Mezeporta/Erupe/issues/168) | ### 2. Test gaps on critical paths diff --git a/server/channelserver/channel_registry.go b/server/channelserver/channel_registry.go index e034250e8..c63bfb164 100644 --- a/server/channelserver/channel_registry.go +++ b/server/channelserver/channel_registry.go @@ -46,13 +46,14 @@ type SessionSnapshot struct { // StageSnapshot is an immutable copy of stage data taken under lock. type StageSnapshot struct { - ServerIP net.IP - ServerPort uint16 - StageID string - ClientCount int - Reserved int - MaxPlayers uint16 - RawBinData0 []byte - RawBinData1 []byte - RawBinData3 []byte + ServerIP net.IP + ServerPort uint16 + StageID string + ClientCount int + Reserved int + QuestReserved int // Players who left to enter quest stages ("Qs" prefix) + MaxPlayers uint16 + RawBinData0 []byte + RawBinData1 []byte + RawBinData3 []byte } diff --git a/server/channelserver/channel_registry_local.go b/server/channelserver/channel_registry_local.go index 15985fb88..286c81777 100644 --- a/server/channelserver/channel_registry_local.go +++ b/server/channelserver/channel_registry_local.go @@ -107,6 +107,19 @@ func (r *LocalChannelRegistry) SearchStages(stagePrefix string, max int) []Stage if len(results) >= max { break } + + // Pre-collect which charIDs are in quest stages under server lock, + // so we can count quest-reserved players without lock ordering issues + // (Server.Mutex must be acquired before Stage.RWMutex). + c.Lock() + inQuest := make(map[uint32]bool) + for _, sess := range c.sessions { + if sess.stage != nil && len(sess.stage.id) > 4 && sess.stage.id[3:5] == "Qs" { + inQuest[sess.charID] = true + } + } + c.Unlock() + cIP := net.ParseIP(c.IP).To4() cPort := c.Port c.stages.Range(func(_ string, stage *Stage) bool { @@ -127,16 +140,24 @@ func (r *LocalChannelRegistry) SearchStages(stagePrefix string, max int) []Stage bin3Copy := make([]byte, len(bin3)) copy(bin3Copy, bin3) + questReserved := 0 + for charID := range stage.reservedClientSlots { + if inQuest[charID] { + questReserved++ + } + } + results = append(results, StageSnapshot{ - ServerIP: cIP, - ServerPort: cPort, - StageID: stage.id, - ClientCount: len(stage.clients) + len(stage.reservedClientSlots), - Reserved: len(stage.reservedClientSlots), - MaxPlayers: stage.maxPlayers, - RawBinData0: bin0Copy, - RawBinData1: bin1Copy, - RawBinData3: bin3Copy, + ServerIP: cIP, + ServerPort: cPort, + StageID: stage.id, + ClientCount: len(stage.clients) + len(stage.reservedClientSlots), + Reserved: len(stage.reservedClientSlots), + QuestReserved: questReserved, + MaxPlayers: stage.maxPlayers, + RawBinData0: bin0Copy, + RawBinData1: bin1Copy, + RawBinData3: bin3Copy, }) stage.RUnlock() return true diff --git a/server/channelserver/handlers_session.go b/server/channelserver/handlers_session.go index 1395bc7e9..847478bcb 100644 --- a/server/channelserver/handlers_session.go +++ b/server/channelserver/handlers_session.go @@ -407,8 +407,13 @@ func handleMsgSysIssueLogkey(s *Session, p mhfpacket.MHFPacket) { return } - // TODO(Andoryuuta): In the official client, the log key index is off by one, - // cutting off the last byte in _most uses_. Find and document these accordingly. + // Client log key off-by-one (RE'd from mhfo-hd.dll ZZ): + // putIssue_logkey (0x1D) requests and stores all 16 bytes correctly. + // putRecord_log (0x1E) and putTerminal_log (0x13) do NOT embed the log key + // in their packets — they pass size 0 to the packet builder for the key field. + // The original off-by-one note (Andoryuuta) may apply to pre-ZZ clients where + // these functions did use the key. In ZZ the key is stored but never sent back, + // so the server value is effectively unused beyond issuance. s.Lock() s.logKey = logKey s.Unlock() @@ -548,7 +553,11 @@ func handleMsgMhfTransitMessage(s *Session, p mhfpacket.MHFPacket) { resp.WriteUint8(uint8(len(sjisName) + 1)) resp.WriteUint16(uint16(len(snap.UserBinary3))) - // TODO: This case might be <=G2 + // User search response padding block (RE'd from mhfo-hd.dll ZZ): + // ZZ per-entry parser (FUN_115868a0) reads 0x28 (40) bytes at offset +8 + // via memcpy into the result struct. G1 and earlier use 8 bytes. + // G2 DLL analysis was inconclusive (stripped binary, no shared struct + // sizes with ZZ) — the boundary may be <=G2 rather than <=G1. if s.server.erupeConfig.RealClientMode <= cfg.G1 { resp.WriteBytes(make([]byte, 8)) } else { @@ -711,8 +720,10 @@ func handleMsgMhfTransitMessage(s *Session, p mhfpacket.MHFPacket) { resp.WriteUint16(0) // Unk, [0 1 2] resp.WriteUint16(uint16(sr.ClientCount)) resp.WriteUint16(sr.MaxPlayers) - // TODO: Retail returned the number of clients in quests, not workshop/my series - resp.WriteUint16(uint16(sr.Reserved)) + // Retail returned only clients in quest stages ("Qs" prefix), + // not workshop/my series. RE'd from FUN_11586690 in mhfo-hd.dll ZZ: + // field at entry offset 0x08-0x09 → struct offset 0x1C (param_1[0xe]). + resp.WriteUint16(uint16(sr.QuestReserved)) resp.WriteUint8(0) // Static? resp.WriteUint8(uint8(sr.MaxPlayers))