mirror of
https://github.com/nolen777/eagle0.git
synced 2026-07-28 21:35:42 +00:00
Complete deproto migration: delete plan and update linter (#5529)
The library/ code is now fully protoless (zero Scala proto dependencies). Transitive deps through C++/Go build tools (map generation) are expected. Changes: - Delete docs/DEPROTO_PLAN.md - migration complete - Delete scripts/build_deps_baseline.txt - no longer needed - Update check_build_deps.sh to enforce zero Scala proto deps in library/ (C++/Go proto deps are allowed as they're build-time tools) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,170 +0,0 @@
|
||||
# Deproto Migration Plan
|
||||
|
||||
## Vision
|
||||
|
||||
**Protocol buffers should only be used at the edges** — for network serialization (gRPC) and disk persistence. Inside the Eagle game engine, all logic should operate on native Scala models.
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────────┐
|
||||
│ GRPC BOUNDARY │
|
||||
│ EagleServiceImpl.scala ←→ Proto Messages ←→ Unity Client │
|
||||
└─────────────────────────────────────────────────────────────────────┘
|
||||
↓
|
||||
GameStateConverter
|
||||
↓
|
||||
┌─────────────────────────────────────────────────────────────────────┐
|
||||
│ SCALA ENGINE │
|
||||
│ │
|
||||
│ GameStateC ───→ Actions ───→ ActionResultT ───→ New GameStateC │
|
||||
│ ↑ │ │
|
||||
│ │ (Pure Scala models) │ │
|
||||
│ └───────────────────────────────────────────────────┘ │
|
||||
│ │
|
||||
│ HeroC, FactionC, ProvinceC, BattalionC, ArmyC, etc. │
|
||||
└─────────────────────────────────────────────────────────────────────┘
|
||||
↓
|
||||
GameStateConverter
|
||||
↓
|
||||
┌─────────────────────────────────────────────────────────────────────┐
|
||||
│ PERSISTENCE BOUNDARY │
|
||||
│ GameHistory.scala ←→ Proto Messages ←→ File/Database │
|
||||
└─────────────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Current State (January 2026)
|
||||
|
||||
| Area | Status |
|
||||
|------|--------|
|
||||
| Production code (`src/main/scala/.../library/`) | ✅ **Complete** - zero proto imports |
|
||||
| Test code (`src/test/scala/.../library/`) | ✅ **Complete** - 24 migrated, 0 deferred, 4 deleted |
|
||||
| Hostility proto migration | ✅ **Complete** - all 4 files migrated to Scala enum |
|
||||
|
||||
**Note:** The `bazel query` for proto dependencies still shows ~26 targets, but these are transitive build-graph dependencies from map data generation (C++/Go tools), not Scala code imports.
|
||||
|
||||
---
|
||||
|
||||
## Phase 10: Library Test Migration ✅ Complete
|
||||
|
||||
### Overview
|
||||
|
||||
27 test files in `src/test/scala/.../library/` were reviewed for proto type usage. 20 files were successfully migrated to use Scala model types exclusively.
|
||||
|
||||
### Migration Results
|
||||
|
||||
#### Successfully Migrated (20 files)
|
||||
|
||||
| File | PR | Notes |
|
||||
|------|-----|-------|
|
||||
| `QuestFulfillmentUtilsTest.scala` | Merged | Light migration |
|
||||
| `FeastCommandTest.scala` | Merged | Light migration |
|
||||
| `FriendlyMoveActionTest.scala` | Merged | Light migration |
|
||||
| `DeclineQuestCommandTest.scala` | Merged | Light migration |
|
||||
| `PerformHostileArmySetupActionTest.scala` | Merged | Medium migration |
|
||||
| `NewYearActionTest.scala` | Merged | Medium migration |
|
||||
| `ProvinceHeldActionTest.scala` | Merged | Medium migration |
|
||||
| `AvailableMarchCommandFactoryTest.scala` | Merged | Medium migration |
|
||||
| `AvailableCommandsFactoryTest.scala` | Merged | Heavy migration |
|
||||
| `AvailablePleaseRecruitMeCommandFactoryTest.scala` | Merged | Heavy migration |
|
||||
| `PerformReconResolutionActionTest.scala` | Merged | Heavy migration |
|
||||
| `AttackDecisionCommandChooserTest.scala` | Merged | Heavy migration |
|
||||
| `PerformProvinceEventsActionTest.scala` | #5499 | Heavy migration |
|
||||
| `PerformFoodConsumptionPhaseActionTest.scala` | #5500 | Heavy migration |
|
||||
| `AvailableDiplomacyCommandsFactoryTest.scala` | #5502 | Heavy migration |
|
||||
| `EndPlayerCommandsPhaseActionTest.scala` | #5504 | Heavy migration |
|
||||
| `ProvinceConqueredActionTest.scala` | #5505 | Heavy migration |
|
||||
| `NewRoundActionTest.scala` | #5506 | Heavy migration (22 imports) |
|
||||
| `EngineImplTest.scala` | #5488 | Added InMemoryHistory.fromScalaResults |
|
||||
| `GameStateViewFilterTest.scala` | #5510 | Heavy migration with view types |
|
||||
|
||||
#### Additional Migrations (4 files)
|
||||
|
||||
| File | PR | Notes |
|
||||
|------|-----|-------|
|
||||
| `BattalionTypesTestData.scala` | hostility-deproto | Migrated to Scala BattalionType |
|
||||
| `NewRoundActionTest.scala` | hostility-deproto | Heavy migration (FactionRelationship, MovingArmy) |
|
||||
| `EndBattleAftermathPhaseActionTest.scala` | hostility-deproto | Complete rewrite to Scala types |
|
||||
| `ResolveBattleActionTest.scala` | hostility-deproto | Minimal migration (removed IDable usage) |
|
||||
|
||||
#### Deleted (4 files)
|
||||
|
||||
| File | Reason |
|
||||
|------|--------|
|
||||
| `RansomCommandTest.scala` | Orphaned test with no corresponding implementation |
|
||||
| `IDable.scala` | Unused - BUILD deps existed but no code actually imported it |
|
||||
| `impl/package.scala` | Defined HeroMap/BattalionMap but never used |
|
||||
| `availability/package.scala` | Defined HeroMap/BattalionMap but never used |
|
||||
|
||||
### Key Migration Patterns Used
|
||||
|
||||
**Import aliasing to avoid naming conflicts:**
|
||||
```scala
|
||||
import net.eagle0.eagle.model.action_result.types.ActionResultType.{
|
||||
NewRoundAction as NewRoundActionType,
|
||||
NewYearAction as NewYearActionType
|
||||
}
|
||||
```
|
||||
|
||||
**Using `inside()` for concrete type access:**
|
||||
```scala
|
||||
inside(result.changedProvinces.head) { case cp: ChangedProvinceC =>
|
||||
cp.foodDelta shouldBe Some(-258)
|
||||
}
|
||||
```
|
||||
|
||||
**Scala model construction:**
|
||||
```scala
|
||||
HeroC(id = 9, vigor = 50, factionId = Some(actingFactionId))
|
||||
ProvinceC(id = 7, rulingFactionId = Some(5), food = 2987)
|
||||
FactionC(id = 5, factionHeadId = 9, name = "Test Faction")
|
||||
```
|
||||
|
||||
### Key Conversions Reference
|
||||
|
||||
| Proto Type | Scala Equivalent |
|
||||
|------------|------------------|
|
||||
| `GameState` | `GameState` (model/state/game_state) |
|
||||
| `Province` | `ProvinceC` |
|
||||
| `Hero` | `HeroC` |
|
||||
| `Faction` | `FactionC` |
|
||||
| `Battalion` | `BattalionC` |
|
||||
| `Army` | `Army` (already Scala) |
|
||||
| `Date` | `Date` (model/state/date) |
|
||||
| `ChangedHero` | `ChangedHeroC` |
|
||||
| `ChangedProvince` | `ChangedProvinceC` |
|
||||
| `ActionResultType.*` | `ActionResultType.*` (Scala enum) |
|
||||
|
||||
### Success Criteria
|
||||
|
||||
- [x] 24 test files fully migrated to Scala types
|
||||
- [x] All tests pass
|
||||
- [x] Deferred files documented with clear rationale
|
||||
- [x] Zero proto imports in `src/test/scala/.../library/` ✅
|
||||
|
||||
---
|
||||
|
||||
## Architecture Summary
|
||||
|
||||
### Fully Protoless ✅
|
||||
|
||||
- **Production library code** (`src/main/scala/.../library/`)
|
||||
- **AI layer** (`/ai/`)
|
||||
- **Actions** (`/library/actions/impl/action/`)
|
||||
- **Commands** (`/library/actions/impl/command/`)
|
||||
- **Availability** (`/library/actions/availability/`)
|
||||
- **Command helpers** (`/library/util/command_choice_helpers/`)
|
||||
- **View filters** (`/library/util/view_filters/`)
|
||||
- **LLM generators** (`/library/actions/llm_prompt_generators/`)
|
||||
- **All library tests** (`src/test/scala/.../library/`) - 24 files migrated
|
||||
|
||||
### Proto at Boundaries (Expected) ✅
|
||||
|
||||
- `EagleServiceImpl.scala` - gRPC boundary
|
||||
- `GameController.scala` - Client communication
|
||||
- `*Converter.scala` - Explicit conversion utilities
|
||||
- `PersistedHistory.scala` - Disk persistence
|
||||
|
||||
### Deferred (Documented) 📋
|
||||
|
||||
- None! All library test utilities migrated or deleted.
|
||||
@@ -1,3 +0,0 @@
|
||||
# BUILD dependency baseline - updated Tue Jan 20 21:22:24 PST 2026
|
||||
# Do not increase these numbers - only decrease!
|
||||
library_proto_deps=26
|
||||
+27
-80
@@ -6,16 +6,12 @@
|
||||
# ./scripts/check_build_deps.sh # Check all rules
|
||||
# ./scripts/check_build_deps.sh --ci # CI mode (fail on any violation)
|
||||
# ./scripts/check_build_deps.sh --count # Just count current violations (for tracking progress)
|
||||
# ./scripts/check_build_deps.sh --strict # Fail if proto deps in library/ exceed baseline
|
||||
#
|
||||
# The baseline for proto deps is stored in scripts/build_deps_baseline.txt
|
||||
# Update it with: ./scripts/check_build_deps.sh --update-baseline
|
||||
# ./scripts/check_build_deps.sh --strict # Same as --ci (strict enforcement)
|
||||
|
||||
set -e
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
|
||||
BASELINE_FILE="$SCRIPT_DIR/build_deps_baseline.txt"
|
||||
cd "$REPO_ROOT"
|
||||
|
||||
RED='\033[0;31m'
|
||||
@@ -26,15 +22,6 @@ NC='\033[0m' # No Color
|
||||
MODE="${1:-check}"
|
||||
EXIT_CODE=0
|
||||
|
||||
# Get baseline proto count (default to a high number if no baseline exists)
|
||||
get_baseline() {
|
||||
if [ -f "$BASELINE_FILE" ]; then
|
||||
grep "^library_proto_deps=" "$BASELINE_FILE" | cut -d= -f2
|
||||
else
|
||||
echo "999999" # No baseline = don't fail
|
||||
fi
|
||||
}
|
||||
|
||||
# Rule 1: src/main should not depend on src/test
|
||||
check_main_depends_on_test() {
|
||||
echo -e "${YELLOW}Checking: src/main should not depend on src/test...${NC}"
|
||||
@@ -51,26 +38,24 @@ check_main_depends_on_test() {
|
||||
fi
|
||||
}
|
||||
|
||||
# Rule 2: library/ should not depend on src/main/protobuf
|
||||
# Note: This rule is aspirational - there are currently many violations as part of ongoing deproto effort
|
||||
check_library_depends_on_proto() {
|
||||
echo -e "${YELLOW}Checking: library/ should not depend on src/main/protobuf...${NC}"
|
||||
# Rule 2: library/ should not depend on Scala proto types
|
||||
# C++/Go proto deps are allowed (they're build-time deps for map generation tools)
|
||||
check_library_depends_on_scala_proto() {
|
||||
echo -e "${YELLOW}Checking: library/ should not depend on Scala proto types...${NC}"
|
||||
|
||||
violations=$(bazel query 'deps(//src/main/scala/net/eagle0/eagle/library/...) intersect //src/main/protobuf/...' 2>/dev/null || true)
|
||||
count=$(echo "$violations" | grep -c "^//" || echo "0")
|
||||
violations=$(bazel query 'deps(//src/main/scala/net/eagle0/eagle/library/...) intersect //src/main/protobuf/...' 2>/dev/null | grep "_scala_proto" || true)
|
||||
if [ -z "$violations" ]; then
|
||||
count=0
|
||||
else
|
||||
count=$(echo "$violations" | grep -c "^//" || true)
|
||||
fi
|
||||
|
||||
if [ "$count" -gt 0 ]; then
|
||||
echo -e "${YELLOW}Found $count proto dependencies in library/ (deproto in progress)${NC}"
|
||||
if [ "$MODE" == "--count" ]; then
|
||||
echo "$violations" | head -20
|
||||
if [ "$count" -gt 20 ]; then
|
||||
echo "... and $((count - 20)) more"
|
||||
fi
|
||||
fi
|
||||
# Don't fail on this rule yet - it's aspirational
|
||||
return 0
|
||||
echo -e "${RED}VIOLATION: Found $count Scala proto dependencies in library/:${NC}"
|
||||
echo "$violations"
|
||||
return 1
|
||||
else
|
||||
echo -e "${GREEN}✓ No proto dependencies in library/${NC}"
|
||||
echo -e "${GREEN}✓ No Scala proto dependencies in library/${NC}"
|
||||
return 0
|
||||
fi
|
||||
}
|
||||
@@ -96,51 +81,21 @@ check_library_depends_on_proto_converters() {
|
||||
fi
|
||||
}
|
||||
|
||||
# Count proto deps for tracking deproto progress
|
||||
# Count proto deps for informational purposes
|
||||
count_proto_deps() {
|
||||
echo -e "${YELLOW}=== Proto dependency counts ===${NC}"
|
||||
|
||||
library_count=$(bazel query 'deps(//src/main/scala/net/eagle0/eagle/library/...) intersect //src/main/protobuf/...' 2>/dev/null | grep -c "^//" || echo "0")
|
||||
echo "library/: $library_count proto dependencies"
|
||||
|
||||
baseline=$(get_baseline)
|
||||
if [ "$baseline" != "999999" ]; then
|
||||
echo "baseline: $baseline"
|
||||
if [ "$library_count" -lt "$baseline" ]; then
|
||||
echo -e "${GREEN}↓ Reduced by $((baseline - library_count)) from baseline${NC}"
|
||||
elif [ "$library_count" -gt "$baseline" ]; then
|
||||
echo -e "${RED}↑ Increased by $((library_count - baseline)) from baseline${NC}"
|
||||
fi
|
||||
fi
|
||||
}
|
||||
|
||||
# Check if proto deps exceed baseline (for strict mode)
|
||||
check_proto_baseline() {
|
||||
library_count=$(bazel query 'deps(//src/main/scala/net/eagle0/eagle/library/...) intersect //src/main/protobuf/...' 2>/dev/null | grep -c "^//" || echo "0")
|
||||
baseline=$(get_baseline)
|
||||
|
||||
echo -e "${YELLOW}Checking: proto deps in library/ should not exceed baseline...${NC}"
|
||||
echo "Current: $library_count, Baseline: $baseline"
|
||||
|
||||
if [ "$library_count" -gt "$baseline" ]; then
|
||||
echo -e "${RED}VIOLATION: Proto dependencies increased from $baseline to $library_count${NC}"
|
||||
echo "Run './scripts/check_build_deps.sh --count' to see which protos are used"
|
||||
return 1
|
||||
scala_proto_results=$(bazel query 'deps(//src/main/scala/net/eagle0/eagle/library/...) intersect //src/main/protobuf/...' 2>/dev/null | grep "_scala_proto" || true)
|
||||
if [ -z "$scala_proto_results" ]; then
|
||||
scala_proto_count=0
|
||||
else
|
||||
echo -e "${GREEN}✓ Proto deps at or below baseline${NC}"
|
||||
return 0
|
||||
scala_proto_count=$(echo "$scala_proto_results" | wc -l | tr -d ' ')
|
||||
fi
|
||||
}
|
||||
echo "library/ Scala proto deps: $scala_proto_count"
|
||||
|
||||
# Update baseline file
|
||||
update_baseline() {
|
||||
library_count=$(bazel query 'deps(//src/main/scala/net/eagle0/eagle/library/...) intersect //src/main/protobuf/...' 2>/dev/null | grep -c "^//" || echo "0")
|
||||
|
||||
echo "# BUILD dependency baseline - updated $(date)" > "$BASELINE_FILE"
|
||||
echo "# Do not increase these numbers - only decrease!" >> "$BASELINE_FILE"
|
||||
echo "library_proto_deps=$library_count" >> "$BASELINE_FILE"
|
||||
|
||||
echo -e "${GREEN}Updated baseline: library_proto_deps=$library_count${NC}"
|
||||
# C++/Go proto deps are expected (map generation tools)
|
||||
all_proto_count=$(bazel query 'deps(//src/main/scala/net/eagle0/eagle/library/...) intersect //src/main/protobuf/...' 2>/dev/null | wc -l | tr -d ' ')
|
||||
echo "library/ all proto deps (includes C++/Go build tools): $all_proto_count"
|
||||
}
|
||||
|
||||
echo "=== BUILD.bazel Dependency Check ==="
|
||||
@@ -150,22 +105,14 @@ case "$MODE" in
|
||||
--count)
|
||||
count_proto_deps
|
||||
;;
|
||||
--ci)
|
||||
--ci|--strict)
|
||||
check_main_depends_on_test || EXIT_CODE=1
|
||||
check_library_depends_on_proto || EXIT_CODE=1
|
||||
check_library_depends_on_scala_proto || EXIT_CODE=1
|
||||
check_library_depends_on_proto_converters || EXIT_CODE=1
|
||||
;;
|
||||
--strict)
|
||||
check_main_depends_on_test || EXIT_CODE=1
|
||||
check_proto_baseline || EXIT_CODE=1
|
||||
check_library_depends_on_proto_converters || EXIT_CODE=1
|
||||
;;
|
||||
--update-baseline)
|
||||
update_baseline
|
||||
;;
|
||||
*)
|
||||
check_main_depends_on_test || EXIT_CODE=1
|
||||
check_library_depends_on_proto
|
||||
check_library_depends_on_scala_proto || EXIT_CODE=1
|
||||
check_library_depends_on_proto_converters || EXIT_CODE=1
|
||||
echo ""
|
||||
count_proto_deps
|
||||
|
||||
Reference in New Issue
Block a user