chore: remove stale TODO, update codecov-action to v5, refresh tech debt doc

The guild daily RP rollover TODO in handlers_guild_ops.go was stale —
the feature was already implemented via lazy rollover in
handlers_guild.go. Several other items in technical-debt.md were also
resolved in prior commits (typos, db guard investigation). Updated
the doc to reflect current state and bumped codecov-action to v5.
This commit is contained in:
Houmgaor
2026-02-22 18:38:10 +01:00
parent de00e41830
commit bcb5086dbb
3 changed files with 13 additions and 22 deletions

View File

@@ -44,7 +44,7 @@ jobs:
run: go test -race -coverprofile=coverage.out ./... -timeout=10m run: go test -race -coverprofile=coverage.out ./... -timeout=10m
- name: Upload Coverage to Codecov - name: Upload Coverage to Codecov
uses: codecov/codecov-action@v4 uses: codecov/codecov-action@v5
with: with:
files: ./coverage.out files: ./coverage.out
flags: unittests flags: unittests

View File

@@ -11,9 +11,8 @@ This document tracks actionable technical debt items discovered during a codebas
- [2. Test gaps on critical paths](#2-test-gaps-on-critical-paths) - [2. Test gaps on critical paths](#2-test-gaps-on-critical-paths)
- [Medium Priority](#medium-priority) - [Medium Priority](#medium-priority)
- [3. Logging anti-patterns](#3-logging-anti-patterns) - [3. Logging anti-patterns](#3-logging-anti-patterns)
- [4. Typos and stale comments](#4-typos-and-stale-comments)
- [Low Priority](#low-priority) - [Low Priority](#low-priority)
- [5. CI updates](#5-ci-updates) - [4. CI updates](#4-ci-updates)
- [Completed Items](#completed-items) - [Completed Items](#completed-items)
- [Suggested Execution Order](#suggested-execution-order) - [Suggested Execution Order](#suggested-execution-order)
@@ -27,10 +26,9 @@ These TODOs represent features that are visibly broken for players.
| Location | Issue | Impact | | Location | Issue | Impact |
|----------|-------|--------| |----------|-------|--------|
| `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 | | `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. |
| `handlers_guild_ops.go:148` | `TODO: Move this value onto rp_yesterday and reset to 0... daily?` | Guild daily RP rollover logic is missing entirely | | `handlers_achievement.go:125` | `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. |
| `handlers_achievement.go:125` | `TODO: Notify on rank increase` — always returns `false` | Achievement rank-up notifications are silently suppressed | | `handlers_guild_info.go:443` | `TODO: Enable GuildAlliance applications` — hardcoded `true` | Guild alliance applications are always open regardless of setting. Needs research into where the toggle originates. |
| `handlers_guild_info.go:443` | `TODO: Enable GuildAlliance applications` — hardcoded `true` | Guild alliance applications are always open regardless of setting |
| `handlers_session.go:394` | `TODO(Andoryuuta): log key index off-by-one` | Known off-by-one in log key indexing is unresolved | | `handlers_session.go:394` | `TODO(Andoryuuta): log key index off-by-one` | Known off-by-one in log key indexing is unresolved |
| `handlers_session.go:535` | `TODO: This case might be <=G2` | Uncertain version detection in switch case | | `handlers_session.go:535` | `TODO: This case might be <=G2` | Uncertain version detection in switch case |
| `handlers_session.go:698` | `TODO: Retail returned the number of clients in quests` | Player count reported to clients does not match retail behavior | | `handlers_session.go:698` | `TODO: Retail returned the number of clients in quests` | Player count reported to clients does not match retail behavior |
@@ -65,21 +63,13 @@ These are validated indirectly through mock-based handler tests but have no SQL-
~~**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. ~~**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.
### 4. Typos and stale comments
| Location | Issue |
|----------|-------|
| `sys_session.go:73` | Comment says "For Debuging" — typo ("Debugging"), and the field is used in production logging, not just debugging |
| `handlers_session.go:394` | "offical" should be "official" |
| `handlers_session.go:322` | `if s.server.db != nil` guard wraps repo calls — leaky abstraction from the pre-repository refactor |
--- ---
## Low Priority ## Low Priority
### 5. CI updates ### 4. CI updates
- `codecov-action@v4` could be updated to `v5` (current stable) - ~~`codecov-action@v4` could be updated to `v5` (current stable)~~ **Fixed.** Updated to `codecov-action@v5`.
- No coverage threshold is enforced — coverage is uploaded but regressions aren't caught - No coverage threshold is enforced — coverage is uploaded but regressions aren't caught
--- ---
@@ -97,6 +87,9 @@ Items resolved since the original audit:
| — | **Monthly guild item claim** (`handlers_guild.go:389`) | Now tracks per-character per-type monthly claims via `stamps` table. | | — | **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 (4 files)** | Tests added for `handlers_session.go`, `handlers_gacha.go`, `handlers_plate.go`, `handlers_shop.go`. |
| — | **Entrance server raw SQL** | Refactored to repository interfaces (`repo_interfaces.go`, `repo_session.go`, `repo_server.go`). | | — | **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. |
--- ---
@@ -105,7 +98,6 @@ Items resolved since the original audit:
Based on remaining impact: Based on remaining impact:
1. **Add tests for `handlers_commands.go`** — highest-risk remaining untested handler (admin commands) 1. **Add tests for `handlers_commands.go`** — highest-risk remaining untested handler (admin commands)
2. **Fix bookshelf data pointer** (`model_character.go`) — corrupts saves for three game versions 2. **Fix bookshelf data pointer** (`model_character.go`) — corrupts saves for three game versions (needs save data validation)
3. **Implement guild daily RP rollover** (`handlers_guild_ops.go:148`) — missing game feature 3. **Fix achievement rank-up notifications** (`handlers_achievement.go:125`) — needs protocol research on `MhfDisplayedAchievement`
4. **Fix typos** (`sys_session.go:73`, `handlers_session.go:394`) — quick cleanup 4. **Add coverage threshold** to CI — prevents regressions
5. **Update `codecov-action` to v5** and add coverage threshold — prevents regressions

View File

@@ -145,7 +145,6 @@ func handleMsgMhfOperateGuild(s *Session, p mhfpacket.MHFPacket) {
case mhfpacket.OperateGuildDonateEvent: case mhfpacket.OperateGuildDonateEvent:
quantity := uint16(pkt.Data1.ReadUint32()) quantity := uint16(pkt.Data1.ReadUint32())
bf.WriteBytes(handleDonateRP(s, quantity, guild, 1)) bf.WriteBytes(handleDonateRP(s, quantity, guild, 1))
// TODO: Move this value onto rp_yesterday and reset to 0... daily?
if err := s.server.guildRepo.AddMemberDailyRP(s.charID, quantity); err != nil { if err := s.server.guildRepo.AddMemberDailyRP(s.charID, quantity); err != nil {
s.logger.Error("Failed to update guild character daily RP", zap.Error(err)) s.logger.Error("Failed to update guild character daily RP", zap.Error(err))
} }