mirror of
https://github.com/Mezeporta/Erupe.git
synced 2026-03-22 07:32:32 +01:00
docs: add IMPROVEMENTS.md with 9.3.0 release milestones
Add comprehensive improvement recommendations and release milestones: - Security & stability improvements (token lifecycle, error handling) - Database performance (indexes, N+1 fixes, caching) - Feature completeness (guild, daily missions, tournament, tower) - Operational excellence (health checks, metrics, logging) - Discord bot enhancements - Multi-version support audit - Schema management infrastructure - High-value packet implementations Also add project roadmap section to CLAUDE.md referencing IMPROVEMENTS.md.
This commit is contained in:
Houmgaor
420
IMPROVEMENTS.md
Normal file
420
IMPROVEMENTS.md
Normal file
@@ -0,0 +1,420 @@
|
||||
# Erupe Improvement Recommendations
|
||||
|
||||
This document outlines prioritized improvements identified through codebase analysis.
|
||||
|
||||
## Critical Priority
|
||||
|
||||
### 1. Test Coverage
|
||||
|
||||
**Current state:** 7.5% coverage on core channelserver (12,351 lines of code)
|
||||
|
||||
**Recommendations:**
|
||||
- Add tests for packet handlers - 400+ handlers with minimal coverage
|
||||
- Focus on critical files:
|
||||
- `server/channelserver/handlers_quest.go`
|
||||
- `server/channelserver/handlers_guild.go`
|
||||
- `server/channelserver/sys_session.go`
|
||||
- `server/channelserver/sys_stage.go`
|
||||
- Create table-driven tests for the handler table
|
||||
- Add fuzzing tests for packet parsing in `common/byteframe/`
|
||||
- Target: 40%+ coverage on channelserver
|
||||
|
||||
### 2. Update Dependencies
|
||||
|
||||
Outdated packages in `go.mod` with potential security implications:
|
||||
|
||||
| Package | Current | Latest |
|
||||
|---------|---------|--------|
|
||||
| `go.uber.org/zap` | 1.18.1 | 1.27.0+ |
|
||||
| `github.com/spf13/viper` | 1.8.1 | 1.18.0+ |
|
||||
| `golang.org/x/crypto` | 0.1.0 | latest |
|
||||
| `github.com/lib/pq` | 1.10.4 | latest |
|
||||
|
||||
**Action:**
|
||||
```bash
|
||||
go get -u ./...
|
||||
go mod tidy
|
||||
go test -v ./...
|
||||
```
|
||||
|
||||
### 3. Add Context-Based Cancellation
|
||||
|
||||
`server/channelserver/sys_session.go` spawns goroutines without `context.Context`, preventing graceful shutdown and causing potential goroutine leaks.
|
||||
|
||||
**Changes needed:**
|
||||
- Add `context.Context` to `Session.Start()`
|
||||
- Pass context to `sendLoop()` and `recvLoop()`
|
||||
- Implement cancellation on session close
|
||||
- Add timeout contexts for database operations
|
||||
|
||||
---
|
||||
|
||||
## Important Priority
|
||||
|
||||
### 4. Fix Error Handling
|
||||
|
||||
**Issues found:**
|
||||
- 61 instances of `panic()` or `Fatal()` that crash the entire server
|
||||
- Ignored errors in `main.go` lines 29, 32, 195-196:
|
||||
```go
|
||||
_ = db.MustExec("DELETE FROM guild_characters") // Error ignored
|
||||
```
|
||||
- Typos in error messages (e.g., "netcate" instead of "netcafe" in `handlers_cafe.go`)
|
||||
|
||||
**Action:**
|
||||
```bash
|
||||
# Find all panics to review
|
||||
grep -rn "panic(" server/
|
||||
|
||||
# Find ignored errors
|
||||
grep -rn "_ = " server/ | grep -E "(Exec|Query)"
|
||||
```
|
||||
|
||||
### 5. Refactor Large Files
|
||||
|
||||
Files exceeding maintainability guidelines:
|
||||
|
||||
| File | Lines |
|
||||
|------|-------|
|
||||
| `handlers_guild.go` | 1,986 |
|
||||
| `handlers.go` | 1,835 |
|
||||
| `handlers_shop_gacha.go` | 679 |
|
||||
| `handlers_house.go` | 589 |
|
||||
|
||||
**Recommendations:**
|
||||
- Split large handler files by functionality
|
||||
- Move massive hex strings in `handlers_tactics.go` and `handlers_quest.go` to separate data files or compressed format
|
||||
- Extract repeated patterns into utility functions
|
||||
|
||||
### 6. Enhance CI/CD Pipeline
|
||||
|
||||
**Current gaps:**
|
||||
- No code coverage threshold enforcement
|
||||
- No security scanning
|
||||
- No database migration testing
|
||||
|
||||
**Add to `.github/workflows/`:**
|
||||
- Coverage threshold (fail build if coverage drops below 30%)
|
||||
- `gosec` for security scanning
|
||||
- Integration tests with test database
|
||||
- `go mod audit` for vulnerability scanning (Go 1.22+)
|
||||
|
||||
---
|
||||
|
||||
## Nice to Have
|
||||
|
||||
### 7. Logging Cleanup
|
||||
|
||||
**Issues:**
|
||||
- 17 remaining `fmt.Print`/`println` calls should use zap
|
||||
- `handlers_cast_binary.go` creates a new logger on every handler call (inefficient)
|
||||
|
||||
**Action:**
|
||||
```bash
|
||||
# Find printf calls that should use zap
|
||||
grep -rn "fmt.Print" server/
|
||||
grep -rn "println" server/
|
||||
```
|
||||
|
||||
### 8. Configuration Improvements
|
||||
|
||||
**Hardcoded values to extract:**
|
||||
|
||||
| Value | Location | Suggested Config Key |
|
||||
|-------|----------|---------------------|
|
||||
| `maxDecoMysets = 40` | `handlers_house.go` | `GameplayOptions.MaxDecoMysets` |
|
||||
| `decoMysetSize = 78` | `handlers_house.go` | `GameplayOptions.DecoMysetSize` |
|
||||
| Session timeout (30s) | `sys_session.go:132` | `Channel.SessionTimeout` |
|
||||
| Packet queue buffer (20) | `sys_session.go` | `Channel.PacketQueueSize` |
|
||||
|
||||
**Recommendation:** Create `config/constants.go` for game constants or add to `ErupeConfig`.
|
||||
|
||||
### 9. Resolve Technical Debt
|
||||
|
||||
14 TODO/FIXME comments in core code:
|
||||
|
||||
| File | Issue |
|
||||
|------|-------|
|
||||
| `signserver/session.go` | Token expiration not implemented |
|
||||
| `handlers.go` | Off-by-one error in log key index |
|
||||
| `handlers_guild.go` | Multiple incomplete features |
|
||||
| `handlers_stage.go` | Unknown packet behavior |
|
||||
| `crypto/crypto_test.go` | Failing test case needs debugging |
|
||||
|
||||
---
|
||||
|
||||
## Quick Wins
|
||||
|
||||
### Immediate Actions
|
||||
|
||||
```bash
|
||||
# 1. Update dependencies
|
||||
go get -u ./... && go mod tidy
|
||||
|
||||
# 2. Run security check
|
||||
go install github.com/securego/gosec/v2/cmd/gosec@latest
|
||||
gosec ./...
|
||||
|
||||
# 3. Find all panics
|
||||
grep -rn "panic(" server/ --include="*.go"
|
||||
|
||||
# 4. Find ignored errors
|
||||
grep -rn "_ = " server/ --include="*.go" | grep -v "_test.go"
|
||||
|
||||
# 5. Check for race conditions
|
||||
go test -race ./...
|
||||
|
||||
# 6. Generate coverage report
|
||||
go test ./... -coverprofile=coverage.out
|
||||
go tool cover -html=coverage.out -o coverage.html
|
||||
```
|
||||
|
||||
### Low-Effort High-Impact
|
||||
|
||||
1. Fix error message typo in `handlers_cafe.go` ("netcate" -> "netcafe")
|
||||
2. Add `defer rows.Close()` where missing after database queries
|
||||
3. Replace `fmt.Print` calls with zap logger
|
||||
4. Add missing error checks for `db.Exec()` calls
|
||||
|
||||
---
|
||||
|
||||
## Metrics to Track
|
||||
|
||||
| Metric | Current | Target |
|
||||
|--------|---------|--------|
|
||||
| Test coverage (channelserver) | 7.5% | 40%+ |
|
||||
| Test coverage (overall) | 21.4% | 50%+ |
|
||||
| Panic/Fatal calls | 61 | 0 (in handlers) |
|
||||
| Ignored errors | ~20 | 0 |
|
||||
| TODO/FIXME comments | 14 | 0 |
|
||||
| Outdated dependencies | 4+ | 0 |
|
||||
|
||||
---
|
||||
|
||||
## Implementation Order
|
||||
|
||||
1. **Week 1:** Update dependencies, fix critical error handling
|
||||
2. **Week 2:** Add context cancellation to session lifecycle
|
||||
3. **Week 3-4:** Expand test coverage for core handlers
|
||||
4. **Week 5:** Refactor large files, extract constants
|
||||
5. **Ongoing:** Resolve TODO comments, improve documentation
|
||||
|
||||
---
|
||||
|
||||
## Release Milestones (9.3.0)
|
||||
|
||||
The following milestones are organized for the upcoming 9.3.0 release.
|
||||
|
||||
### Milestone 1: Security & Stability
|
||||
|
||||
**Token Lifecycle Management**
|
||||
- [ ] Implement automatic token cleanup after inactivity (`signserver/session.go:133`)
|
||||
- [ ] Add configurable token expiration time
|
||||
- [ ] Add rate limiting on sign-in attempts
|
||||
- [ ] Document security implications of `DisableTokenCheck` option
|
||||
|
||||
**Graceful Error Handling**
|
||||
- [ ] Replace 9 `panic()` calls in `handlers_guild_scout.go` with proper error returns
|
||||
- [ ] Replace `panic()` in `handlers_tower.go:43` (GetOwnTowerLevelV3) with stub response
|
||||
- [ ] Convert fatal errors to recoverable errors where possible
|
||||
|
||||
**Database Connection Resilience**
|
||||
- [ ] Configure connection pooling in `main.go:182`:
|
||||
```go
|
||||
db.SetMaxOpenConns(25)
|
||||
db.SetMaxIdleConns(5)
|
||||
db.SetConnMaxLifetime(5 * time.Minute)
|
||||
db.SetConnMaxIdleTime(2 * time.Minute)
|
||||
```
|
||||
- [ ] Add connection health monitoring
|
||||
- [ ] Implement reconnection logic on connection loss
|
||||
|
||||
---
|
||||
|
||||
### Milestone 2: Database Performance
|
||||
|
||||
**Add Missing Indexes**
|
||||
- [ ] `CREATE INDEX idx_characters_user_id ON characters(user_id)`
|
||||
- [ ] `CREATE INDEX idx_guild_characters_guild_id ON guild_characters(guild_id)`
|
||||
- [ ] `CREATE INDEX idx_mail_sender_id ON mail(sender_id)`
|
||||
- [ ] `CREATE INDEX idx_user_binary_character_id ON user_binary(character_id)`
|
||||
- [ ] `CREATE INDEX idx_gacha_entries_gacha_id ON gacha_entries(gacha_id)`
|
||||
- [ ] `CREATE INDEX idx_distribution_items_dist_id ON distribution_items(distribution_id)`
|
||||
|
||||
**Fix N+1 Query Patterns**
|
||||
- [ ] `handlers_guild.go:1419-1444` - Batch alliance member queries into single UNION query
|
||||
- [ ] `signserver/dbutils.go:135-162` - Rewrite friend/guildmate queries as JOINs
|
||||
- [ ] `handlers_distitem.go:34-46` - Replace subquery with JOIN + GROUP BY
|
||||
- [ ] `handlers_cafe.go:29-88` - Combine 4 single-field queries into one
|
||||
|
||||
**Implement Caching Layer**
|
||||
- [ ] Create `server/channelserver/cache/cache.go` with `sync.RWMutex`-protected maps
|
||||
- [ ] Cache gacha shop data at server startup (`handlers_shop_gacha.go:112`)
|
||||
- [ ] Cache normal shop items
|
||||
- [ ] Add cache invalidation on admin updates
|
||||
- [ ] Cache guild information during session lifetime
|
||||
|
||||
---
|
||||
|
||||
### Milestone 3: Feature Completeness
|
||||
|
||||
**Guild System**
|
||||
- [ ] Implement daily RP reset (`handlers_guild.go:740`)
|
||||
- [ ] Enable guild alliance applications (`handlers_guild.go:1281`)
|
||||
- [ ] Add guild message board cleanup (`handlers_guild.go:1888`)
|
||||
- [ ] Record guild user counts to database (`handlers_guild.go:1946`)
|
||||
- [ ] Implement monthly reward tracker (`handlers_guild.go:1967`)
|
||||
- [ ] Handle alliance application deletion (`handlers_guild_alliance.go:154`)
|
||||
|
||||
**Daily/Recurring Systems**
|
||||
- [ ] Implement gacha daily reset at noon (`handlers_shop_gacha.go:513`)
|
||||
- [ ] Add achievement rank notifications (`handlers_achievement.go:122`)
|
||||
|
||||
**Daily Mission System** (currently empty handlers)
|
||||
- [ ] Implement `handleMsgMhfGetDailyMissionMaster()`
|
||||
- [ ] Implement `handleMsgMhfGetDailyMissionPersonal()`
|
||||
- [ ] Implement `handleMsgMhfSetDailyMissionPersonal()`
|
||||
|
||||
**Tournament System** (`handlers_tournament.go`)
|
||||
- [ ] Implement `handleMsgMhfEntryTournament()` (line 58)
|
||||
- [ ] Implement `handleMsgMhfAcquireTournament()` (line 60)
|
||||
- [ ] Complete tournament info handler with real data (line 14-25)
|
||||
|
||||
**Tower System** (`handlers_tower.go`)
|
||||
- [ ] Fix `GetOwnTowerLevelV3` panic (line 43)
|
||||
- [ ] Handle tenrou/irai hex decode errors gracefully (line 75)
|
||||
|
||||
**Seibattle System**
|
||||
- [ ] Implement `handleMsgMhfGetSeibattle()` (`handlers.go:1708-1711`)
|
||||
- [ ] Implement `handleMsgMhfPostSeibattle()`
|
||||
- [ ] Add configuration toggle for Seibattle feature
|
||||
|
||||
---
|
||||
|
||||
### Milestone 4: Operational Excellence
|
||||
|
||||
**Health Checks & Monitoring**
|
||||
- [ ] Add `/health` HTTP endpoint for container orchestration
|
||||
- [ ] Add `/ready` readiness probe
|
||||
- [ ] Add `/live` liveness probe
|
||||
- [ ] Implement basic Prometheus metrics:
|
||||
- `erupe_active_sessions` gauge
|
||||
- `erupe_active_stages` gauge
|
||||
- `erupe_packet_processed_total` counter
|
||||
- `erupe_db_query_duration_seconds` histogram
|
||||
|
||||
**Logging Improvements**
|
||||
- [ ] Replace all `fmt.Print`/`println` calls with zap (17 instances)
|
||||
- [ ] Fix logger creation in `handlers_cast_binary.go` (create once, reuse)
|
||||
- [ ] Add correlation IDs for request tracing
|
||||
- [ ] Add structured context fields (player ID, stage ID, guild ID)
|
||||
|
||||
**Configuration Management**
|
||||
- [ ] Create `config/constants.go` for game constants
|
||||
- [ ] Make session timeout configurable
|
||||
- [ ] Make packet queue buffer size configurable
|
||||
- [ ] Add feature flags for incomplete systems (Tournament, Seibattle)
|
||||
|
||||
---
|
||||
|
||||
### Milestone 5: Discord Bot Enhancements
|
||||
|
||||
**Current state:** Output-only with minimal features
|
||||
|
||||
**New Features**
|
||||
- [ ] Player login/logout notifications
|
||||
- [ ] Quest completion announcements
|
||||
- [ ] Achievement unlock notifications
|
||||
- [ ] Guild activity feed (joins, leaves, rank changes)
|
||||
- [ ] Administrative commands:
|
||||
- `/status` - Server status
|
||||
- `/players` - Online player count
|
||||
- `/kick` - Kick player (admin only)
|
||||
- `/announce` - Server-wide announcement
|
||||
- [ ] Two-way chat bridge (Discord ↔ in-game)
|
||||
|
||||
---
|
||||
|
||||
### Milestone 6: Multi-Version Support
|
||||
|
||||
**Client Version Handling**
|
||||
- [ ] Audit handlers for missing client version checks
|
||||
- [ ] Document version-specific packet format differences
|
||||
- [ ] Create version compatibility matrix
|
||||
- [ ] Add version-specific tower system handling
|
||||
- [ ] Test S6.0 through ZZ compatibility systematically
|
||||
|
||||
---
|
||||
|
||||
### Milestone 7: Schema Management
|
||||
|
||||
**Patch Schema Infrastructure**
|
||||
- [ ] Create numbered patch files in `patch-schema/`:
|
||||
- `01_add_indexes.sql` - Performance indexes
|
||||
- `02_token_expiry.sql` - Token cleanup support
|
||||
- `03_daily_mission.sql` - Daily mission tables
|
||||
- [ ] Add schema version tracking table
|
||||
- [ ] Create migration runner script
|
||||
- [ ] Document patch application process
|
||||
|
||||
**Schema Cleanup**
|
||||
- [ ] Add PRIMARY KEY to `shop_items_bought`
|
||||
- [ ] Add PRIMARY KEY to `cafe_accepted`
|
||||
- [ ] Add foreign key constraints to child tables
|
||||
- [ ] Remove or document unused tables (`achievement`, `titles`, `feature_weapon`)
|
||||
|
||||
---
|
||||
|
||||
### Milestone 8: Packet Implementation
|
||||
|
||||
**High-Value Packets** (393 files with "NOT IMPLEMENTED")
|
||||
|
||||
Priority implementations:
|
||||
- [ ] `msg_mhf_create_joint.go` - Joint quest creation
|
||||
- [ ] `msg_mhf_mercenary_huntdata.go` - Mercenary hunt data
|
||||
- [ ] `msg_mhf_save_deco_myset.go` - Decoration preset saving
|
||||
- [ ] `msg_mhf_get_ud_ranking.go` - User-defined quest rankings
|
||||
- [ ] `msg_mhf_load_hunter_navi.go` - Hunter Navi system
|
||||
- [ ] `msg_mhf_answer_guild_scout.go` - Guild scouting responses
|
||||
- [ ] `msg_mhf_acquire_guild_tresure.go` - Guild treasure acquisition
|
||||
- [ ] `msg_mhf_payment_achievement.go` - Payment achievements
|
||||
- [ ] `msg_mhf_stampcard_prize.go` - Stamp card prizes
|
||||
|
||||
---
|
||||
|
||||
## Release Checklist
|
||||
|
||||
Before 9.3.0 release:
|
||||
|
||||
- [ ] All Milestone 1 items completed (Security & Stability)
|
||||
- [ ] Critical database indexes added (Milestone 2)
|
||||
- [ ] N+1 queries fixed (Milestone 2)
|
||||
- [ ] Guild system TODOs resolved (Milestone 3)
|
||||
- [ ] Health check endpoints added (Milestone 4)
|
||||
- [ ] Schema patches created and tested (Milestone 7)
|
||||
- [ ] Test coverage increased to 30%+
|
||||
- [ ] All tests passing with race detector
|
||||
- [ ] Dependencies updated
|
||||
- [ ] CHANGELOG.md updated
|
||||
- [ ] Documentation reviewed
|
||||
|
||||
---
|
||||
|
||||
## Metrics to Track
|
||||
|
||||
| Metric | Current | 9.3.0 Target |
|
||||
|--------|---------|--------------|
|
||||
| Test coverage (channelserver) | 7.5% | 40%+ |
|
||||
| Test coverage (overall) | 21.4% | 50%+ |
|
||||
| Panic/Fatal calls | 61 | <10 (critical paths only) |
|
||||
| Ignored errors | ~20 | 0 |
|
||||
| TODO/FIXME comments | 18 | <5 |
|
||||
| Outdated dependencies | 4+ | 0 |
|
||||
| N+1 query patterns | 4 | 0 |
|
||||
| Missing critical indexes | 6 | 0 |
|
||||
| Unimplemented packets | 393 | 380 (13 high-value done) |
|
||||
|
||||
---
|
||||
|
||||
*Generated: 2026-02-01*
|
||||
*Updated: 2026-02-01 - Added release milestones*
|
||||
Reference in New Issue
Block a user