MP fence clear and sychronisation fixed

Author: gcclinux

docs/test-fence-sync.md

@@ -0,0 +1,96 @@
+player1# Fence Synchronization Test
+
+## Issues Fixed
+
+### Issue 1: Fence Creation Not Synchronized
+Players in multiplayer mode could not see each other's fences.
+
+**Root Cause**: The local Player object was not getting its player ID set when connecting to multiplayer, causing all fence placement messages to use "local_player" as the owner ID instead of the actual client ID.
+
+**Fix Applied**: Modified `GameMessageHandler.java` to set the player ID on the local Player object when receiving the connection success message.
+
+### Issue 2: Fence Deletion Not Synchronized  
+Players could see each other create fences, but fence deletions were not synchronized between players.
+
+**Root Cause**: When removing fences, the client was generating a new random UUID for the fence ID instead of using the actual fence ID of the piece being removed. This caused the server to reject the removal request because it couldn't find a fence with that random ID.
+
+**Fix Applied**: 
+1. Added fence ID support to `FencePiece` class
+2. Modified `FencePieceFactory` to accept fence IDs
+3. Updated `FenceStructureManager` to store and use fence IDs
+4. Fixed `FenceBuildingManager` to use the correct fence ID when removing pieces
+5. Updated client-side fence processing to handle fence IDs properly
+
+### Issue 3: Clear All Fences (C Key) Not Synchronized
+Players could see individual fence deletions, but when a player pressed "C" to clear an entire enclosure, other players could not see the mass deletion.
+
+**Root Cause**: The `clearNearestEnclosure()` method was only removing fences locally without sending network messages to other players. It bypassed the normal removal flow that sends `FenceRemoveMessage` to the server.
+
+**Fix Applied**: 
+1. Modified `clearNearestEnclosure()` method in `FenceBuildingManager` to send individual `FenceRemoveMessage` for each fence piece being removed
+2. Fixed `replacePieceType()` method in `FenceStructureManager` to preserve fence IDs when updating adjacent pieces
+3. Enhanced server validation in `ClientConnection` to allow fence removal with fallback IDs when the player owns the fence, preventing rejection of legitimate removal requests during enclosure clearing
+
+### Issue 4: Clear All Fences (C Key) Not Respecting Ownership
+Players could clear fence enclosures created by other players using the "C" key, which is a security issue.
+
+**Root Cause**: The `clearNearestEnclosure()` method was removing ALL connected fence pieces without checking ownership, allowing players to delete fences they didn't create.
+
+**Fix Applied**: Added ownership validation to `clearNearestEnclosure()` method:
+1. Check if the closest fence piece is owned by the current player before proceeding
+2. Filter connected pieces to only include those owned by the current player
+3. Only remove fence pieces that belong to the requesting player
+4. Provide clear feedback when no owned pieces are found in the enclosure
+
+## Changes Made
+
+### Core Classes Modified:
+- `FencePiece.java` - Added fenceId field and methods
+- `FencePieceFactory.java` - Added createPiece method with fence ID parameter
+- `FenceStructureManager.java` - Added fence ID support to addFencePiece method
+- `FenceBuildingManager.java` - Fixed fence ID handling for both creation and removal
+- `MyGdxGame.java` - Updated fence processing methods to use fence IDs
+- `GameMessageHandler.java` - Added player ID assignment on connection
+
+### Key Changes:
+1. **Fence Creation**: Generate fence ID once and use it for both local fence piece and network message
+2. **Fence Removal**: Use the actual fence ID from the removed piece instead of generating a new one
+3. **Network Sync**: Pass fence IDs through all fence synchronization messages
+4. **Player ID**: Set correct player ID on connection to avoid "local_player" fallback
+5. **Fence ID Preservation**: Maintain fence IDs when updating adjacent piece types
+6. **Fallback ID Handling**: Allow server to accept fence removal with fallback IDs for legitimate owners
+
+## Test Steps
+1. Start Player1 as host server
+2. Connect Player2 to localhost:25565
+3. Both players activate Free World mode
+4. **Fence Creation Test**:
+   - Player1 builds a fence → Player2 should see it immediately
+   - Player2 builds a fence → Player1 should see it immediately
+5. **Individual Fence Deletion Test**:
+   - Player1 removes a fence (right-click) → Player2 should see it disappear immediately
+   - Player2 removes a fence (right-click) → Player1 should see it disappear immediately
+6. **Enclosure Clear Test**:
+   - Player1 builds a complete fence enclosure
+   - Player2 should see the entire enclosure
+   - Player1 presses "C" to clear the enclosure → Player2 should see all fences disappear immediately
+   - Player2 builds an enclosure and presses "C" → Player1 should see it clear immediately
+7. **Ownership Validation Test**:
+   - Player1 builds a fence enclosure
+   - Player2 tries to press "C" near Player1's enclosure → Should NOT clear Player1's fences
+   - Player2 builds their own enclosure next to Player1's
+   - Player2 presses "C" → Should only clear Player2's fences, leaving Player1's intact
+   - Player1 presses "C" → Should only clear Player1's remaining fences
+
+## Expected Behavior
+- ✅ Each player sees other players' fence creations in real-time
+- ✅ Each player sees other players' individual fence deletions in real-time
+- ✅ Each player sees other players' enclosure clearing (C key) in real-time
+- ✅ Fence placement messages use correct client IDs
+- ✅ Fence removal messages use correct fence IDs
+- ✅ No more "local_player" fallback in multiplayer mode
+- ✅ Server properly validates fence ownership before allowing removal
+- ✅ Mass fence clearing sends individual removal messages for each piece
+- ✅ Players can only clear their own fence enclosures (ownership validation)
+- ✅ Mixed-ownership enclosures only clear pieces owned by the requesting player
+- ✅ Clear feedback when attempting to clear unowned fences

src/main/java/wagemaker/uk/fence/FenceBuildingManager.java

@@ -9,6 +9,7 @@
 import com.badlogic.gdx.math.Vector2;
 import com.badlogic.gdx.math.Vector3;
 import java.awt.Point;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -414,11 +415,30 @@ public boolean placeFenceSegment(int gridX, int gridY) {
 
         try {
             System.out.println("[FenceBuildingManager] Attempting to place fence at grid (" + gridPos.x + ", " + gridPos.y + ") with material " + selectedMaterialType + " for owner " + playerId);
+            
+            // Generate fence ID for both local and network use
+            String fenceId = java.util.UUID.randomUUID().toString();
+            
             // Attempt to place the fence piece with automatic piece type selection
-            FencePiece placedPiece = structureManager.addFencePiece(gridPos, selectedMaterialType, playerId);
+            FencePiece placedPiece = structureManager.addFencePiece(gridPos, selectedMaterialType, playerId, fenceId);
 
             if (placedPiece != null) {
-                System.out.println("[FenceBuildingManager] Fence piece created: " + placedPiece.getType() + " at world (" + placedPiece.getX() + ", " + placedPiece.getY() + ")");
+                System.out.println("[FenceBuildingManager] Fence piece created: " + placedPiece.getType() + " at world (" + placedPiece.getX() + ", " + placedPiece.getY() + ") with ID " + fenceId);
+                
+                // Send fence place message to server in multiplayer mode
+                wagemaker.uk.network.GameClient gameClient = player.getGameClient();
+                System.out.println("DEBUG: Sending fence place message - gameClient=" + (gameClient != null ? "exists" : "null") + 
+                                 ", connected=" + (gameClient != null ? gameClient.isConnected() : "N/A"));
+                if (gameClient != null && gameClient.isConnected()) {
+                    wagemaker.uk.network.FencePlaceMessage fenceMsg = new wagemaker.uk.network.FencePlaceMessage(
+                        playerId, fenceId, gridX, gridY, placedPiece.getType(), selectedMaterialType, playerId
+                    );
+                    gameClient.sendMessage(fenceMsg);
+                    System.out.println("DEBUG: Sent FencePlaceMessage to server: " + fenceMsg);
+                } else {
+                    System.out.println("DEBUG: NOT sending fence message - gameClient null or not connected");
+                }
+                
                 // Consume materials from inventory
                 FenceMaterialProvider materialProvider = validator.getMaterialProvider();
                 if (materialProvider != null) {
@@ -555,6 +575,28 @@ public boolean removeFenceSegment(int gridX, int gridY) {
             FencePiece removedPiece = structureManager.removeFencePiece(gridPos);
 
             if (removedPiece != null) {
+                // Get the fence ID from the removed piece
+                String fenceId = removedPiece.getFenceId();
+                if (fenceId == null) {
+                    // Fallback for pieces without IDs (shouldn't happen in multiplayer)
+                    fenceId = "unknown-" + System.currentTimeMillis();
+                    System.out.println("WARNING: Removed fence piece had no ID, using fallback: " + fenceId);
+                }
+                
+                // Send fence remove message to server in multiplayer mode
+                wagemaker.uk.network.GameClient gameClient = player.getGameClient();
+                System.out.println("DEBUG: Sending fence remove message - gameClient=" + (gameClient != null ? "exists" : "null") + 
+                                 ", connected=" + (gameClient != null ? gameClient.isConnected() : "N/A"));
+                if (gameClient != null && gameClient.isConnected()) {
+                    wagemaker.uk.network.FenceRemoveMessage fenceMsg = new wagemaker.uk.network.FenceRemoveMessage(
+                        playerId, fenceId, gridX, gridY, materialToReturn, playerId
+                    );
+                    gameClient.sendMessage(fenceMsg);
+                    System.out.println("DEBUG: Sent FenceRemoveMessage to server with fence ID: " + fenceId);
+                } else {
+                    System.out.println("DEBUG: NOT sending fence remove message - gameClient null or not connected");
+                }
+                
                 // Return materials to inventory
                 FenceMaterialProvider materialProvider = validator.getMaterialProvider();
                 if (materialProvider != null) {
@@ -922,20 +964,65 @@ public void clearNearestEnclosure() {
         }
 
         if (closestPos != null) {
+            // Get the current player's ID for ownership validation
+            String playerId = player.getPlayerId();
+            if (playerId == null) {
+                playerId = "local_player";
+            }
+            
+            // Check if the closest piece is owned by the current player
+            FencePiece closestPiece = structureManager.getFencePiece(closestPos);
+            if (closestPiece == null || !playerId.equals(closestPiece.getOwnerId())) {
+                System.out.println("[FenceBuildingManager] Cannot clear enclosure - closest fence piece is not owned by current player");
+                return;
+            }
+            
             // Get connected pieces (the enclosure)
             Set<Point> connectedPoints = structureManager.findConnectedPieces(closestPos);
 
             if (!connectedPoints.isEmpty()) {
-                System.out.println("[FenceBuildingManager] Clearing enclosure of " + connectedPoints.size() + " pieces nearest to " + closestPos);
+                // Filter to only include pieces owned by the current player
+                Set<Point> ownedPieces = new HashSet<>();
+                for (Point pos : connectedPoints) {
+                    FencePiece piece = structureManager.getFencePiece(pos);
+                    if (piece != null && playerId.equals(piece.getOwnerId())) {
+                        ownedPieces.add(pos);
+                    }
+                }
+                
+                if (ownedPieces.isEmpty()) {
+                    System.out.println("[FenceBuildingManager] Cannot clear enclosure - no pieces owned by current player");
+                    return;
+                }
+                
+                System.out.println("[FenceBuildingManager] Clearing " + ownedPieces.size() + " owned pieces out of " + connectedPoints.size() + " total pieces nearest to " + closestPos);
 
                 int removedCount = 0;
 
-                // Remove each piece
-                for (Point pos : connectedPoints) {
+                // Remove each owned piece and send network messages
+                for (Point pos : ownedPieces) {
                     FencePiece removedPiece = structureManager.removeFencePiece(pos);
                     if (removedPiece != null) {
                         updateCollisionBoundaries(pos, false);
                         removedCount++;
+                        
+                        // Send fence remove message to server in multiplayer mode
+                        
+                        wagemaker.uk.network.GameClient gameClient = player.getGameClient();
+                        if (gameClient != null && gameClient.isConnected()) {
+                            String fenceId = removedPiece.getFenceId();
+                            if (fenceId == null) {
+                                fenceId = "unknown-" + System.currentTimeMillis() + "-" + pos.x + "-" + pos.y;
+                                System.out.println("WARNING: Removed fence piece had no ID, using fallback: " + fenceId);
+                            }
+                            
+                            wagemaker.uk.network.FenceRemoveMessage fenceMsg = new wagemaker.uk.network.FenceRemoveMessage(
+                                playerId, fenceId, pos.x, pos.y, selectedMaterialType, playerId
+                            );
+                            gameClient.sendMessage(fenceMsg);
+                            System.out.println("DEBUG: Sent FenceRemoveMessage for enclosure clear - fence ID: " + fenceId + " at (" + pos.x + ", " + pos.y + ")");
+                        }
+                        
                         // Trigger visual effect for each piece
                         if (visualEffectsManager != null) {
                             visualEffectsManager.triggerRemovalAnimation(pos, selectedMaterialType);

src/main/java/wagemaker/uk/fence/FencePiece.java

@@ -19,6 +19,7 @@ public abstract class FencePiece {
     protected static final String TEXTURE_PATH = "textures/fense.png";
 
     protected String ownerId;
+    protected String fenceId;
 
     /**
      * Creates a new fence piece at the specified position.
@@ -27,7 +28,7 @@ public abstract class FencePiece {
      * @param type The type of fence piece
      */
     public FencePiece(float x, float y, FencePieceType type) {
-        this(x, y, type, null);
+        this(x, y, type, null, null);
     }
 
     /**
@@ -38,10 +39,23 @@ public FencePiece(float x, float y, FencePieceType type) {
      * @param ownerId The ID of the owner of this fence piece
      */
     public FencePiece(float x, float y, FencePieceType type, String ownerId) {
+        this(x, y, type, ownerId, null);
+    }
+
+    /**
+     * Creates a new fence piece at the specified position with ownership and fence ID.
+     * @param x World X coordinate
+     * @param y World Y coordinate
+     * @param type The type of fence piece
+     * @param ownerId The ID of the owner of this fence piece
+     * @param fenceId The unique ID of this fence piece
+     */
+    public FencePiece(float x, float y, FencePieceType type, String ownerId, String fenceId) {
         this.x = x;
         this.y = y;
         this.type = type;
         this.ownerId = ownerId;
+        this.fenceId = fenceId;
         loadTexture();
     }
 
@@ -172,6 +186,22 @@ public void setOwnerId(String ownerId) {
         this.ownerId = ownerId;
     }
 
+    /**
+     * Gets the fence ID of this fence piece.
+     * @return Fence ID, or null if not set
+     */
+    public String getFenceId() {
+        return fenceId;
+    }
+
+    /**
+     * Sets the fence ID of this fence piece.
+     * @param fenceId New fence ID
+     */
+    public void setFenceId(String fenceId) {
+        this.fenceId = fenceId;
+    }
+
     /**
      * Disposes of the texture resources used by this fence piece.
      * Should be called when the fence piece is no longer needed.

src/main/java/wagemaker/uk/fence/FencePieceFactory.java

@@ -56,6 +56,22 @@ public static FencePiece createPiece(FencePieceType type, float x, float y, Stri
         piece.setOwnerId(ownerId);
         return piece;
     }
+
+    /**
+     * Creates a fence piece of the specified type at the given position with ownership and fence ID.
+     * @param type The type of fence piece to create
+     * @param x World X coordinate
+     * @param y World Y coordinate
+     * @param ownerId The ID of the owner
+     * @param fenceId The unique ID of the fence piece
+     * @return A new FencePiece instance of the specified type
+     */
+    public static FencePiece createPiece(FencePieceType type, float x, float y, String ownerId, String fenceId) {
+        FencePiece piece = createPiece(type, x, y);
+        piece.setOwnerId(ownerId);
+        piece.setFenceId(fenceId);
+        return piece;
+    }
 
     /**
      * Gets the sequence of fence piece types needed for a rectangular enclosure.

src/main/java/wagemaker/uk/fence/FenceStructureManager.java

@@ -90,6 +90,22 @@ public FencePiece addFencePiece(Point gridPos, FenceMaterialType materialType) {
      * @throws IllegalArgumentException if the position is already occupied
      */
     public FencePiece addFencePiece(Point gridPos, FenceMaterialType materialType, String ownerId) {
+        return addFencePiece(gridPos, materialType, ownerId, null);
+    }
+
+    /**
+     * Adds a fence piece at the specified grid position with ownership and fence ID.
+     * Automatically determines the correct piece type based on adjacent pieces.
+     * Updates connections with adjacent pieces.
+     * 
+     * @param gridPos Grid position where to place the fence piece
+     * @param materialType Type of material to use for the fence piece
+     * @param ownerId ID of the player who owns this fence piece
+     * @param fenceId Unique ID of the fence piece
+     * @return The created FencePiece, or null if placement failed
+     * @throws IllegalArgumentException if the position is already occupied
+     */
+    public FencePiece addFencePiece(Point gridPos, FenceMaterialType materialType, String ownerId, String fenceId) {
         if (placedFences.containsKey(gridPos)) {
             throw new IllegalArgumentException("Position " + gridPos + " is already occupied");
         }
@@ -106,7 +122,7 @@ public FencePiece addFencePiece(Point gridPos, FenceMaterialType materialType, S
         com.badlogic.gdx.math.Vector2 worldPos = grid.gridToWorld(gridPos);
 
         // Create the fence piece
-        FencePiece piece = FencePieceFactory.createPiece(pieceType, worldPos.x, worldPos.y, ownerId);
+        FencePiece piece = FencePieceFactory.createPiece(pieceType, worldPos.x, worldPos.y, ownerId, fenceId);
 
         // Add to our data structures
         placedFences.put(new Point(gridPos.x, gridPos.y), piece);
@@ -316,9 +332,9 @@ private void replacePieceType(Point gridPos, FencePieceType newType) {
             return; // Nothing to replace
         }
 
-        // Create new piece with the same position and owner
+        // Create new piece with the same position, owner, and fence ID
         com.badlogic.gdx.math.Vector2 worldPos = grid.gridToWorld(gridPos);
-        FencePiece newPiece = FencePieceFactory.createPiece(newType, worldPos.x, worldPos.y, oldPiece.getOwnerId());
+        FencePiece newPiece = FencePieceFactory.createPiece(newType, worldPos.x, worldPos.y, oldPiece.getOwnerId(), oldPiece.getFenceId());
 
         // Replace in the map
         placedFences.put(gridPos, newPiece);