The userBinary and minidata maps with their locks were spread across
Server as raw fields with manual lock management. Cross-channel session
searches also required acquiring nested locks (server lock + binary
lock). Encapsulating in dedicated types eliminates the nested locking
and reduces Server's field count by 4.
The quest cache fields (lock, data map, expiry map) were spread across
Server with manual lock management. The old read path also missed the
RLock entirely, creating a data race. Encapsulating in a dedicated type
fixes the race and reduces Server's field count by 2.
All guild subsystem tables are now migrated into repo_guild.go.
21 repository files cover all major subsystems. Only 5 inline SQL
queries remain across 3 handler files. Updated summary table and
refactoring priority list to mark completed items.
A failed Begin() returned a nil tx that would panic on the next
tx.Exec() call. Now logs the error, closes rows, and returns an
empty response gracefully.
Move remaining raw s.server.db.* queries from handler files into
dedicated repository structs, completing the repository extraction
effort. Also adds SaveCharacterData and SaveHouseData to
CharacterRepository.
Fixes guild_hunts query to select both cats_used and start columns
to match the existing two-column Scan call. Adds slot index
validation in GoocooRepository to prevent SQL injection via
fmt.Sprintf.
The config package used `package _config` with a leading underscore,
which is unconventional in Go. Rename to `package config` (matching the
directory name) and use `cfg` as the standard import alias across all
93 importing files.
Move 22 raw SQL queries from 4 handler files into dedicated repository
structs, continuing the repository extraction pattern. Achievement
insert uses ON CONFLICT DO NOTHING to eliminate check-then-insert race,
and IncrementScore validates the column index to prevent SQL injection.
RolloverDailyRP now locks the guild row with SELECT FOR UPDATE and
re-checks rp_reset_at inside the transaction, so concurrent callers
cannot double-rollover and zero out freshly donated RP.
Gacha stepup entry-type check is now skipped when the row was already
deleted as stale, avoiding a redundant DELETE on step 0.
Gacha stepup progress now resets when queried after the most recent
noon boundary, using a new created_at column on gacha_stepup.
Guild member rp_today rolls into rp_yesterday lazily when members are
enumerated after noon, using a new rp_reset_at column on guilds.
Both follow the established lazy-reset pattern from the cafe handler.
The handler was a stub that discarded pkt.NumUsers. Now it
looks up the player's guild and atomically accumulates the
count via a new weekly_bonus_users column on the guilds table.
sqlx.Open was called with no pool configuration, risking PostgreSQL
connection exhaustion under load. Set max open/idle conns and lifetimes.
CreatePost INSERT + soft-delete UPDATE were two separate queries with
no transaction, risking inconsistent state on partial failure.
CollectAdventure used SELECT then UPDATE without a lock, allowing
concurrent guild members to double-collect. Now uses SELECT FOR UPDATE
within a transaction.
- testhelpers_db: retry truncateAllTables up to 3 times on deadlock,
which occurs when previous tests' goroutines still hold DB connections
- handlers_rengoku_integration_test: restore rengoku_score table after
TestRengokuData_SaveOnDBError drops it, preventing cascading failures
in all subsequent rengoku tests
- client_connection_simulation_test: fix TestClientConnection_PacketDuringLogout
to accept both race outcomes (save-wins or logout-wins) since both
handlers independently load from DB and last-writer-wins is valid
Eliminate 18 direct s.server.db calls from handlers_items.go,
handlers_distitem.go, and handlers_session.go by moving queries into
dedicated repository types.
New repositories:
- StampRepository (7 methods, stamps table)
- DistributionRepository (4 methods, distribution/distribution_items)
- SessionRepository (4 methods, sign_sessions/servers)
Also adds ClearTreasureHunt and InsertKillLog to GuildRepository,
which already owns those tables for read operations.
- Use users.frontier_points instead of characters.frontier_points
(column moved in 9.2 schema migration) across savedata and session
lifecycle tests
- Use BYTEA column (otomoairou) instead of INTEGER column
(kouryou_point) in repo_character Load/SaveColumn tests
- Build blocked CSV from actual auto-incremented character IDs instead
of hardcoded IDs in ListMember integration test
- Fix nil charRepo panic in CompleteSaveLoadCycle by using SetTestDB()
Cover all 14 handler functions in handlers_house.go with 25 new tests:
- 7 unit tests for guard paths (payload size limits, box index
bounds, no-op handlers) that run without a database
- 18 integration tests against real PostgreSQL covering interior
updates, house state/password, house enumeration by char ID and
name, house loading with access control, mission data CRUD,
title acquisition with dedup, warehouse operations (box names,
usage limits, rename guards), item storage round-trips, and
deco myset defaults
Introduces readAck() helper to parse MsgSysAck wire format from
the sendPackets channel, and setupHouseTest() for DB + session
scaffolding with user_binary row initialization.
Three fixes for the channelserver test suite:
- Add net.ErrClosed check in acceptClients() so a closed listener
breaks the loop immediately instead of spinning and logging warnings.
This is correct production behavior too.
- Remove -c flag from pg_restore that conflicted with CleanTestDB's
prior DROP, causing "gook DROP CONSTRAINT" errors. Clean the schema
with DROP SCHEMA CASCADE instead of per-table drops.
- Use sync.Once to apply the test schema once per binary run, then
TRUNCATE tables for isolation. Reduces ~60 pg_restore + 29-patch
cycles to a single setup pass.
Move all direct DB access from handlers_rengoku.go (11 calls) and
handlers_mail.go (10 calls) into dedicated repository types, continuing
the established extraction pattern.
RengokuRepository provides UpsertScore and GetRanking, replacing a
3-call check/insert/update sequence and an 8-case switch of nearly
identical queries respectively.
MailRepository provides SendMail, SendMailTx, GetListForCharacter,
GetByID, MarkRead, MarkDeleted, SetLocked, and MarkItemReceived.
The old Mail.Send(), Mail.MarkRead(), GetMailListForCharacter, and
GetMailByID free functions are removed. Guild handlers that sent mail
via Mail.Send(s, ...) now call mailRepo directly.
The stages map was protected by two incompatible locks: the embedded
Server.Mutex and Server.stagesLock (RWMutex). Since these are separate
mutexes they don't exclude each other, and many handlers accessed the
map with no lock at all.
Route all stages map access through stagesLock: read-only lookups use
RLock, writes (create/delete) use Lock. Per-stage field mutations
continue to use each stage's own RWMutex. Restructure
handleMsgSysUnlockStage to avoid holding stagesLock nested inside a
stage RLock, preventing potential deadlock with destructEmptyStages.
Move all direct DB calls from handlers_festa.go (23 calls across 8
tables) and handlers_tower.go (16 calls across 4 tables) into
dedicated repository structs following the established pattern.
FestaRepository (14 methods): lifecycle cleanup, event management,
team souls, trial stats/rankings, user state, voting, registration,
soul submission, prize claiming/enumeration.
TowerRepository (12 methods): personal tower data (skills, progress,
gems), guild tenrouirai progress/scores/page advancement, tower RP.
Also fix pre-existing nil pointer panics in integration tests by
adding SetTestDB helper that initializes both the DB connection and
all repositories, and wire the done channel in createTestServerWithDB
to prevent Shutdown panics.
The game client sometimes writes -1 (0xFF bytes) into the house_tier
field during save, which causes the house theme to vanish on next
login. Snapshot the house tier before applying the save delta and
restore it if the incoming value is corrupted.
Add 34 tests covering all previously-untested GuildRepository methods:
invitations/scouts, guild posts, alliances, adventures, treasure
hunts, meals, kill tracking, and edge cases like disband with
alliance cleanup.
Fix test schema setup to apply the 9.2 update schema after init.sql,
which bridges v9.1.0 to v9.2.0 and resolves 9 pre-existing test
failures caused by missing columns (rp_today, created_at, etc.).
Make patch schemas 14 and 19 idempotent so they no longer fail when
applied against a schema that already has the target state (e.g.
festival_color type already renamed, legacy column names).
Centralizes all gacha_shop/gacha_entries/gacha_items/gacha_stepup/gacha_box
table access into GachaRepository (15 methods) and all user_binary house
columns, warehouse, and titles table access into HouseRepository (17 methods).
Eliminates all direct DB calls from handlers_gacha.go and handlers_house.go.
Centralizes all 31 direct users-table SQL queries from 11 handler
files into a single UserRepository, following the same pattern as
CharacterRepository and GuildRepository. The only excluded query is
the sign_sessions JOIN in handleMsgSysLogin which spans multiple
tables.
Move ~39 inline SQL queries from six handler files into repo_guild.go,
consolidating all guild-related DB access (posts, alliances, adventures,
treasure hunts, meals, kill logs, scouts) behind GuildRepository methods.
Handler files now contain only packet serialization, business logic, and
ACK responses with no direct database calls.
Per anti-patterns.md item #9, guild-related SQL was scattered across
~15 handler files with no repository abstraction. Following the same
pattern established by CharacterRepository, this centralizes all
guilds, guild_characters, and guild_applications table access into a
single GuildRepository (~30 methods).
guild_model.go and handlers_guild_member.go are trimmed to types and
pure business logic only. All handler files (guild_*, festa, mail,
house, mercenary, rengoku) now call s.server.guildRepo methods
instead of direct DB queries or methods on domain objects.
Add 18 new typed methods to CharacterRepository (ReadTime, SaveTime,
SaveInt, SaveBool, SaveString, ReadBool, ReadString, LoadColumnWithDefault,
SetDeleted, UpdateDailyCafe, ResetDailyQuests, ReadEtcPoints, ResetCafeTime,
UpdateGuildPostChecked, ReadGuildPostChecked, SaveMercenary, UpdateGCPAndPact,
FindByRastaID) and migrate ~56 inline SQL queries across 13 handler files.
Pure refactor — zero behavior change. Each handler produces identical SQL
with identical parameters. Cross-table JOINs and bulk CharacterSaveData
operations are intentionally left out of scope.
Centralizes all characters table SQL behind a CharacterRepository struct
in repo_character.go. The 4 existing helpers (loadCharacterData,
saveCharacterData, readCharacterInt, adjustCharacterInt) now delegate to
the repository, keeping identical signatures so all ~70 callsites remain
unchanged. Direct queries in handlers_session.go, sys_channel_server.go
(DisconnectUser), and handlers_mail.go are also migrated.
Pure refactor with zero behavior change — first step toward eliminating
the ~130 scattered character queries identified in anti-patterns #9.
golangci-lint's errcheck rule requires explicit handling of error
return values from Close, Write, and Logout calls. Use blank
identifier assignment for cleanup paths where errors are
intentionally discarded.
Add readCharacterInt/adjustCharacterInt helpers for single-column
integer operations on the characters table. Eliminates fmt.Sprintf
SQL construction in handlers_misc.go and replaces inline queries
across cafe, kouryou, and mercenary handlers.
Second round of protocol constant extraction: adds constants_time.go
(secsPerDay, secsPerWeek), constants_raviente.go (register IDs,
semaphore constants), and named constants across 14 handler files
replacing raw hex/numeric literals. Updates anti-patterns doc to
mark #4 (magic numbers) as substantially fixed.
- Add lint to build job's needs so lint failures prevent artifacts
- Combine race detector + coverage into a single test run
- Pin golangci-lint to v2.1.6 to avoid surprise breakage
- Rename workflow from go-improved.yml to go.yml
Silently ignored DB errors in handlers could cause data loss (frontier
point transactions completing without DB writes), reward duplication
(stamp exchange granting items on failed UPDATE), and crashes (tower
mission page=0 causing index-out-of-bounds). House access state
defaulting to 0 on DB failure also bypassed all access controls.
HIGH risk fixes:
- frontier point buy/sell now fails with ACK on DB error
- stamp exchange/stampcard abort on failed UPDATE
- guild meal INSERT returns fail ACK instead of orphaned ID 0
- mercenary/airou creation aborts on failed sequence nextval
MEDIUM risk fixes:
- tower mission page clamped to >= 1 preventing array underflow
- tower RP donation returns early on failed guild state read
- house state defaults to 2 (password-protected) on DB failure
- playtime read failure logged instead of silently resetting RP
Also cache userID on Session at login time, eliminating ~25 redundant
subqueries of the form WHERE u.id=(SELECT c.user_id FROM characters
c WHERE c.id=$1) across shop, gacha, command, and distitem handlers.
Binary I/O (#5): all 12 remaining encoding/binary calls are
legitimate (zero-alloc spot-reads, random-access into game blobs).
Copy-paste handlers (#8): loadCharacterData/saveCharacterData helpers
now cover standard blob patterns.
Also upgrades saveCharacterData to send doAckSimpleFail on oversize
payloads and DB errors, and migrates handleMsgMhfSaveScenarioData
to the improved helper.
handleMsgSysOperateRegister returned without sending an ACK when the
payload was empty, causing the client to softlock waiting for a response.
Send doAckBufSucceed with nil data on the early-return path to match
the success-path ACK type.
Also update tests to expect error returns instead of panics from
unimplemented Build/Parse stubs (matching prior panic→error refactor),
and mark resolved anti-patterns in docs.
ByteFrame previously panicked on out-of-bounds reads, which crashed
the server when parsing malformed client packets. Now sets a sticky
error (checked via Err()) and returns zero values, matching the
encoding/binary scanner pattern. The session recv loop checks Err()
after parsing to reject malformed packets gracefully.
Also replaces remaining panic("Not implemented") stubs in network
packet Build/Parse methods with proper error returns.
Extract numeric literals into named constants across quest handling,
save data parsing, rengoku skill layout, diva event timing, guild info,
achievement trophies, RP accrual rates, and semaphore IDs. Adds
constants_quest.go for quest-related constants shared across functions.
Pure rename/extract with zero behavior change.
Handlers that log errors and return without sending a MsgSysAck leave
the client waiting indefinitely. Add doAckSimpleFail/doAckBufFail to
14 error paths across 4 files, matching the pattern already used in
~70 other error paths across the codebase.
Affected handlers:
- handleMsgMhfGetCafeDuration (1 path)
- handleMsgMhfSavedata (1 path)
- handleMsgMhfArrangeGuildMember (3 paths)
- handleMsgMhfEnumerateGuildMember (5 paths)
- handleMsgSysLogin (4 paths)
- handleMsgSysIssueLogkey (1 path)
Replace all fmt.Printf/Println and log.Printf/Fatal with structured
zap.Logger calls to eliminate inconsistent logging (anti-pattern #12).
- network/crypt_conn: inject logger via NewCryptConn, replace 6 fmt calls
- signserver/session: use existing s.logger for debug packet dumps
- entranceserver: use s.logger for inbound/outbound debug logging
- api/utils: accept logger param in verifyPath, replace fmt.Println
- api/endpoints: use s.logger for screenshot path diagnostics
- config: replace log.Fatal with error return in getOutboundIP4
- deltacomp: replace log.Printf with zap.L() global logger
The handler table was a package-level global populated by init(), making
registration implicit and untestable. Move it to buildHandlerTable()
which returns the map, store it as a Server struct field initialized in
NewServer(), and add a missing-handler guard in handlePacketGroup to log
a warning instead of panicking on unknown opcodes.
Covers 13 identified anti-patterns across error handling, architecture,
concurrency, and code organization with severity ratings and
recommended refactoring priorities.
Replace vague "Alpelo object system backport" references in CHANGELOG
and AUTHORS with accurate descriptions of the per-session object ID
allocation rework. Remove stale test comment referencing the deleted
NextObjectID method.
Replace the mutable global `_config.ErupeConfig` with dependency
injection across 79 files. Config is now threaded through existing
paths: `ClientContext.RealClientMode` for packet encoding, `s.server.
erupeConfig` for channel handlers, and explicit parameters for utility
functions. This removes hidden coupling, enables test parallelism
without global save/restore, and prevents low-level packages from
reaching up to the config layer.
Key changes:
- Enrich ClientContext with RealClientMode for packet files
- Add mode parameter to CryptConn, mhfitem, mhfcourse functions
- Convert handlers_commands init() to lazy sync.Once initialization
- Delete global var, init(), and helper functions from config.go
- Update all tests to pass config explicitly
Summary
- Each channel server now runs as a fully independent instance with its own listener, goroutines, and
state — one channel crashing or shutting down no longer affects the others
- Introduces a ChannelRegistry interface for cross-channel operations (find session, disconnect user,
worldcast) replacing direct iteration over a shared []*Server slice
- Adds cmd/protbot, a headless MHF protocol bot that exercises the full sign → entrance → channel
flow for automated testing
- Fixes several data races and panics found by the race detector during isolation testing
Changes
Channel server isolation (server/channelserver/)
- ChannelRegistry interface + LocalChannelRegistry implementation for cross-channel lookups
- done channel for clean goroutine shutdown signaling, idempotent Shutdown()
- Race-free acceptClients/manageSessions using select on done instead of closing acceptConns
- invalidateSessions rewritten with proper locking (snapshot under lock, process outside)
- logoutPlayer guards nil DB and logs errors instead of panicking
- Session loops use per-server erupeConfig instead of global _config.ErupeConfig
- Per-channel Enabled flag in config for selectively disabling channels
Protocol bot (cmd/protbot/)
- Standalone Blowfish connection package (no dependency on server config)
- Sign, entrance, and channel protocol implementations
- 5 scenario actions: login, lobby, session, chat, quests
- 19 unit tests covering packet building, parsing, and connection handling
Bug fixes
- Nil decompSave panic on disconnect before character data loads
- Docker Postgres 18 volume mount path (/var/lib/postgresql/ not /data/)
Test plan
- go test -race ./... passes (27 packages, 0 races)
- 5 channel isolation tests verify: independent shutdown, listener failure recovery, session panic
containment, cross-channel registry after shutdown, stage isolation
- Protbot live-tested against Docker stack (all 5 actions)
- Existing config.json files work unchanged (Enabled defaults to false but config.example.json sets
it explicitly)
