Conversation
Added PowerUpContainer to store components within an entity without activating their logic. Implemented PowerUpFactory to create power-up entities in the game. This enhances the game's functionality by allowing dynamic power-up management.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors entity compatibility checking, adds new power-up infrastructure, and improves code organization. Key changes include simplifying the compatibility logic, making undoable behaviors more flexible, and restructuring game system initialization.
- Simplified the
throwIfNotCompatiblemethod logic by inverting condition checks for better readability - Added
PowerUpFactoryandPowerUpContainerclasses to support power-up entities in the game - Enhanced
UndoableBehaviorwith configurable removal behavior and proper cleanup hooks
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ForEntity.java | Updated documentation with clarified examples for entity compatibility annotations |
| PowerUpFactory.java | New factory class for creating power-up entities with physics components |
| PaddleFactory.java | Removed ScaleChange component and updated TODO comment |
| BrickFactory.java | Removed unused PaddleShooting component from brick creation |
| InputSystem.java | Made class final to prevent inheritance |
| GameSystem.java | Refactored apply() method by extracting spawn logic into separate methods and removed unused variable |
| PowerUpContainer.java | New component for storing and applying components from power-up entities |
| UndoableBehavior.java | Added configurable removal behavior, cleanup hook, and constructors |
| ScaleChange.java | Added constructor to support new UndoableBehavior requirements |
| Utilities.java | Simplified compatibility checking logic by inverting conditional flow |
| build.gradle | Removed unused auto-service dependencies and fixed duplicate guava declaration |
Comments suppressed due to low confidence (1)
src/main/java/com/github/codestorm/bounceverse/core/systems/GameSystem.java:48
- Variable 'GameWorld gameWorld' is never read.
final var gameWorld = FXGL.getGameWorld();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @Override | ||
| public void apply() { | ||
| final var gameWorld = FXGL.getGameWorld(); |
There was a problem hiding this comment.
The variable gameWorld is declared but never used. It should be removed or utilized if it was intended for the addFactory method.
| final var gameWorld = FXGL.getGameWorld(); |
| public UndoableBehavior(Duration duration) { | ||
| this.duration = duration; | ||
| } | ||
|
|
||
| public UndoableBehavior(Duration duration, boolean removeWhenUndo) { | ||
| this(duration); | ||
| this.removeWhenUndo = removeWhenUndo; | ||
| } |
There was a problem hiding this comment.
Adding required constructors to the abstract class UndoableBehavior is a breaking change that requires all subclasses to define explicit constructors. Consider providing a default no-arg constructor or documenting this breaking change.
… constructor Removed unused import in GameSystem and added a default constructor to UndoableBehavior for better flexibility.
No description provided.