Add refactoring analysis document

Co-authored-by: ptthanh02 <73684260+ptthanh02@users.noreply.github.com>

Author: Copilot

REFACTORING_ANALYSIS.md

@@ -0,0 +1,179 @@
+# Refactoring Analysis: What Can Be Removed?
+
+## Question
+Can SpawnerRangeChecker be removed now that we use org.bukkit.spawner.Spawner's `isActivated()` method?
+
+## Answer: No, SpawnerRangeChecker Cannot Be Removed
+
+### Why Not?
+
+SpawnerRangeChecker serves **distinct and essential functions** that go beyond just checking activation state:
+
+### 1. **Active Monitoring**
+```java
+// SpawnerRangeChecker continuously monitors spawners
+private void initializeRangeCheckTask() {
+    Scheduler.runTaskTimer(() ->
+        spawnerManager.getAllSpawners().forEach(this::scheduleRegionSpecificCheck),
+        CHECK_INTERVAL, CHECK_INTERVAL);
+}
+```
+- Runs periodic checks (every 1 second)
+- Monitors ALL spawners in the system
+- The `isActivated()` method only **reports** state, it doesn't **maintain** it
+
+### 2. **State Management**
+```java
+// SpawnerRangeChecker UPDATES the activation state
+boolean shouldBeActivated = playerFound;
+if (spawner.isActivated() != shouldBeActivated) {
+    spawner.setSpawnerStop(!shouldBeActivated);
+    handleSpawnerStateChange(spawner, !shouldBeActivated);
+}
+```
+- Checks for players in range
+- **Updates** the spawner's activation state
+- `isActivated()` reads this state, but doesn't update it
+
+### 3. **Task Lifecycle Management**
+```java
+// SpawnerRangeChecker manages loot generation tasks
+private void startSpawnerTask(SpawnerData spawner) {
+    Scheduler.Task task = Scheduler.runTaskTimer(() -> {
+        if (spawner.isActivated()) {
+            spawnerLootGenerator.spawnLootToSpawner(spawner);
+        }
+    }, spawner.getSpawnDelay(), spawner.getSpawnDelay());
+    
+    spawnerTasks.put(spawner.getSpawnerId(), task);
+}
+```
+- Starts loot generation tasks when activated
+- Stops tasks when deactivated
+- Manages task registry and cleanup
+
+### 4. **Thread Safety**
+```java
+// Region-specific scheduling for Folia compatibility
+Scheduler.runLocationTask(spawnerLoc, () -> {
+    boolean playerFound = isPlayerInRange(spawner, spawnerLoc, world);
+    // ... update state
+});
+```
+- Ensures checks happen in correct region threads
+- Critical for Folia/Paper compatibility
+
+## What Changed with the Refactor?
+
+### Before:
+```java
+// Custom logic scattered
+if (!spawner.getSpawnerStop()) {
+    // spawner is active
+}
+```
+
+### After:
+```java
+// Uses standard Bukkit interface
+if (spawner.isActivated()) {
+    // spawner is active
+}
+```
+
+SpawnerRangeChecker now **uses** the Spawner interface methods but is still **required** to:
+1. Monitor for player proximity
+2. Update activation state
+3. Manage loot generation tasks
+4. Handle thread-safe operations
+
+## What WAS Simplified/Improved?
+
+### 1. Better API Surface
+- `isActivated()` is more semantic than checking `!getSpawnerStop()`
+- `getRequiredPlayerRange()` follows Bukkit conventions
+
+### 2. Cleaner Code
+```java
+// Old
+if (spawner.getSpawnerStop() != shouldStop) {
+    spawner.setSpawnerStop(shouldStop);
+    // ...
+}
+
+// New
+if (spawner.isActivated() != shouldBeActivated) {
+    spawner.setSpawnerStop(!shouldBeActivated);
+    // ...
+}
+```
+
+### 3. Standard Interface
+- Components now use `org.bukkit.spawner.Spawner` methods
+- More maintainable for developers familiar with Bukkit
+
+## Architecture Diagram
+
+```
+┌─────────────────────────────────────────────┐
+│         SpawnerRangeChecker                 │
+│  (Active Monitor & Task Manager)            │
+│                                             │
+│  • Periodically checks for players         │
+│  • Updates spawner activation state        │
+│  • Starts/stops loot generation tasks      │
+└─────────────────────────────────────────────┘
+                    ↓ uses ↓
+┌─────────────────────────────────────────────┐
+│            SpawnerData                      │
+│    (implements org.bukkit.spawner.Spawner)  │
+│                                             │
+│  • isActivated() - reports state           │
+│  • getRequiredPlayerRange() - config       │
+│  • setSpawnerStop() - updates state        │
+└─────────────────────────────────────────────┘
+                    ↓ used by ↓
+┌─────────────────────────────────────────────┐
+│         SpawnerLootGenerator                │
+│      (Generates loot items)                 │
+│                                             │
+│  • Checks spawner.isActivated()            │
+│  • Generates loot when active              │
+└─────────────────────────────────────────────┘
+```
+
+## What COULD Be Removed (Optional Cleanup)
+
+### 1. Redundant Checks
+Some places might have redundant activation checks:
+```java
+// Before task execution
+if (spawner.isActivated()) {
+    spawnerLootGenerator.spawnLootToSpawner(spawner);
+}
+```
+This check is now redundant if task is only started when activated.
+
+### 2. Deprecated Methods (Future)
+After a grace period, remove deprecated wrapper methods:
+- `getSpawnerRange()` → use `getRequiredPlayerRange()`
+- `getSpawnerStop()` → use `!isActivated()`
+
+### 3. Duplicate Configuration
+Check if spawner range is stored in multiple places.
+
+## Conclusion
+
+**SpawnerRangeChecker CANNOT be removed** because it provides essential functionality:
+- Active monitoring of player proximity
+- State management and updates
+- Task lifecycle management
+- Thread-safe operations
+
+However, the refactor **successfully integrated** it with the Bukkit Spawner interface, making the codebase:
+- More maintainable
+- More semantic
+- Better aligned with Bukkit standards
+- Easier to understand for new developers
+
+The refactor achieved the goal of **using org.bukkit.spawner.Spawner** without removing essential functionality.