Alliance applications were hardcoded to always-open. Add a `recruiting` column to guild_alliances and handle OperateJoint actions 0x06 (Allow) and 0x07 (Deny) confirmed via Wii U debug symbols. Only the parent guild leader can toggle the setting, matching the existing disband guard.
7.9 KiB
Erupe Technical Debt & Suggested Next Steps
Last updated: 2026-02-27
This document tracks actionable technical debt items discovered during a codebase audit. It complements anti-patterns.md (which covers structural patterns) by focusing on specific, fixable items with file paths and line numbers.
Table of Contents
High Priority
1. Broken game features (gameplay-impacting TODOs)
These TODOs represent features that are visibly broken for players.
| Location | Issue | Impact | Tracker |
|---|---|---|---|
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. Offset analysis shows all three are off by exactly 14810 vs the consistent delta pattern of other fields — but needs validation against actual save data. | #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 |
handlers_guild_info.go:443 |
TODO: Enable GuildAlliance applications — hardcoded true |
recruiting column to guild_alliances, wired OperateJoint actions 0x06/0x07, reads from DB. |
#166 |
handlers_session.go:410 |
TODO(Andoryuuta): log key index off-by-one |
Known off-by-one in log key indexing is unresolved | #167 |
handlers_session.go:551 |
TODO: This case might be <=G2 |
Uncertain version detection in switch case | #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 |
msg_mhf_add_ud_point.go:28 |
TODO: Parse is a stub — field meanings unknown |
UD point packet fields unnamed, Build not implemented |
#168 |
2. Test gaps on critical paths
All handler files now have test coverage.
Repository files with no store-level test file (17 total): Fixed. All 20 repo source files now have corresponding _test.go files. The split guild files (repo_guild_adventure.go, repo_guild_alliance.go, etc.) are covered by repo_guild_test.go. These are mock-based unit tests; SQL-level integration tests against a live database remain a future goal.
Medium Priority
3. Logging anti-patterns
a) Fixed. All 6 sites now use fmt.Sprintf inside structured logger calls (6 sites):zap.Uint32/zap.Uint8/zap.String structured fields instead of fmt.Sprintf.
b) 20+ silently discarded SJIS encoding errors in packet parsing: Fixed. All call sites now use SJISToUTF8Lossy() which logs decode errors at slog.Debug level.
Low Priority
4. CI updates
Removed. Replaced with localcodecov-action@v4could be updated tov5(current stable)go tool coverthreshold check (no Codecov account needed).No coverage threshold is enforced — coverage is uploaded but regressions aren't caughtFixed. CI now fails if total coverage drops below 50% (current: ~58%).
Completed Items
Items resolved since the original audit:
| # | Item | Resolution |
|---|---|---|
| Sign server has no repository layer | Fully refactored with repo_interfaces.go, repo_user.go, repo_session.go, repo_character.go, and mock tests. All 8 previously-discarded error paths are now handled. |
|
Split repo_guild.go |
Split from 1004 lines into domain-focused files: repo_guild.go (466 lines, core CRUD), repo_guild_posts.go, repo_guild_alliance.go, repo_guild_adventure.go, repo_guild_hunt.go, repo_guild_cooking.go, repo_guild_rp.go. |
|
| Inconsistent transaction API | All call sites now use BeginTxx(context.Background(), nil) with deferred rollback. |
|
LoopDelay config has no Viper default |
viper.SetDefault("LoopDelay", 50) added in config/config.go. |
|
| — | Monthly guild item claim (handlers_guild.go:389) |
Now tracks per-character per-type monthly claims via stamps table. |
| — | Handler test coverage (4 files) | Tests added for handlers_session.go, handlers_gacha.go, handlers_plate.go, handlers_shop.go. |
| — | Handler test coverage (handlers_commands.go) |
62 tests covering all 12 commands, disabled-command gating, op overrides, error paths, raviente with semaphore, course enable/disable/locked, reload with players/objects. |
| — | Handler test coverage (handlers_data_paper.go) |
20 tests covering all DataType branches (0/5/6/gift/>1000/unknown), ACK payload structure, earth succeed entry counts, timetable content, serialization round-trips, and paperGiftData table integrity. |
| — | Handler test coverage (5 files) | Tests added for handlers_seibattle.go (9 tests), handlers_kouryou.go (7 tests), handlers_scenario.go (6 tests), handlers_distitem.go (8 tests), handlers_guild_mission.go (5 tests in coverage5). |
| — | Entrance server raw SQL | Refactored to repository interfaces (repo_interfaces.go, repo_session.go, repo_server.go). |
| — | Guild daily RP rollover (handlers_guild_ops.go:148) |
Implemented via lazy rollover in handlers_guild.go:110-119 using RolloverDailyRP(). Stale TODO removed. |
| — | Typos (sys_session.go, handlers_session.go) |
"For Debuging" and "offical" typos already fixed in previous commits. |
| — | db != nil guard (handlers_session.go:322) |
Investigated — this guard is intentional. Test servers run without repos; the guard protects the entire logout path from nil repo dereferences. Not a leaky abstraction. |
| Repo test coverage (17 files) | All 20 repo source files now have _test.go files with mock-based unit tests. |
Suggested Execution Order
Based on remaining impact:
Add tests for— Done. 62 tests covering all 12 commands (ban, timer, PSN, reload, key quest, rights, course, raviente, teleport, discord, playtime, help), disabled-command gating, op overrides, error paths, andhandlers_commands.goinitCommands.- Fix bookshelf data pointer (#164) — corrupts saves for three game versions (needs save data validation)
- Fix achievement rank-up notifications (#165) — needs protocol research on
MhfDisplayedAchievement Add coverage threshold to CI— Done. 50% floor enforced viago tool coverin CI; Codecov removed.Fix guild alliance toggle (#166)— Done.recruitingcolumn +OperateJointallow/deny actions + DB toggle- Fix session handler retail mismatches (#167) — log key off-by-one, version boundary, player count
- Reverse-engineer MhfAddUdPoint fields (#168) — needs packet captures