feat/EntityModule/RandomAnimalSizes #356
Conversation
github-actions
bot
changed the title
…tion Signed-off-by: Illyrius <fitimq@live.nl>
Signed-off-by: Illyrius <fitimq@live.nl>
Signed-off-by: Illyrius <fitimq@live.nl>
Signed-off-by: Illyrius <fitimq@live.nl>
…nsistency Signed-off-by: Illyrius <fitimq@live.nl>
There was a problem hiding this comment.
Pull request overview
This PR introduces a feature to randomize the scale/size of spawning entities (animals, monsters, and villagers) based on configurable ranges. The implementation adds a reusable Range data class and integrates with the existing EntityModule configuration system.
Key changes:
- Adds a serializable Range data class with min/max validation
- Implements random entity scaling via EntitySpawnEvent handler
- Adds three configurable scale ranges for different entity types
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/main/kotlin/org/xodium/vanillaplus/utils/Utils.kt | Adds a serializable Range data class with validation to represent min/max numerical ranges |
| src/main/kotlin/org/xodium/vanillaplus/modules/EntityModule.kt | Implements entity scale randomization, adds event handler for EntitySpawnEvent, updates config with scale ranges, and changes egg drop chance default |
| .idea/dictionaries/project.xml | Adds "slenderman" to the project dictionary |
Files not reviewed (1)
- .idea/dictionaries/project.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private fun randomizeEntityScale(entity: Entity) { | ||
| when (entity) { | ||
| is Animals -> { | ||
| val range = config.entityModule.animalScaleRange ?: return | ||
| entity.getAttribute(Attribute.SCALE)?.baseValue = Random.nextDouble(range.min, range.max) | ||
| } | ||
|
|
||
| is Monster -> { | ||
| val range = config.entityModule.monsterScaleRange ?: return | ||
| entity.getAttribute(Attribute.SCALE)?.baseValue = Random.nextDouble(range.min, range.max) | ||
| } | ||
|
|
||
| is Villager -> { | ||
| val range = config.entityModule.villagerScaleRange ?: return | ||
| entity.getAttribute(Attribute.SCALE)?.baseValue = Random.nextDouble(range.min, range.max) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The randomizeEntityScale function should check if the module is enabled before applying changes. Other event handlers in the codebase follow the pattern of early returns based on config flags. Consider adding a check for the module's enabled state or individual config flags for each entity type's scale randomization.
| var disableGhastGrief: Boolean = true, | ||
| var disableWitherGrief: Boolean = true, | ||
| var entityEggDropChance: Double = 0.1, | ||
| var entityEggDropChance: Double = 0.001, |
There was a problem hiding this comment.
The entityEggDropChance has been changed from 0.1 (10%) to 0.001 (0.1%), which is a significant behavioral change unrelated to the random animal sizes feature. This change should either be reverted or explained in a separate commit/PR focused on egg drop mechanics.
| var entityEggDropChance: Double = 0.001, | |
| var entityEggDropChance: Double = 0.1, |
| <w>searchinv</w> | ||
| <w>shulker</w> | ||
| <w>signedit</w> | ||
| <w>slenderman</w> |
There was a problem hiding this comment.
The addition of "slenderman" to the dictionary appears unrelated to the random animal sizes feature introduced in this PR. This dictionary entry should be removed or explained if it relates to a different change.
| <w>slenderman</w> |
| @EventHandler | ||
| fun on(event: EntitySpawnEvent) = randomizeEntityScale(event.entity) |
There was a problem hiding this comment.
The EntitySpawnEvent fires for every entity spawn in the game, including projectiles, items, experience orbs, etc. The current implementation already handles this with early returns when the entity type doesn't match (Animals, Monster, or Villager), but consider adding documentation or a comment explaining this to clarify the performance characteristics of this event handler.
| val max: Double, | ||
| ) { | ||
| init { | ||
| require(min <= max) { "min must be less than or equal to max" } |
There was a problem hiding this comment.
The Range validation should also check for non-finite values (NaN and infinity). The current check only validates that min is less than or equal to max, but does not prevent invalid Double values such as Double.NaN or Double.POSITIVE_INFINITY, which could cause issues when used with Random.nextDouble.
| require(min <= max) { "min must be less than or equal to max" } | |
| require(min.isFinite() && max.isFinite() && min <= max) { | |
| "min and max must be finite and min must be less than or equal to max" | |
| } |
| var disableWitherGrief: Boolean = true, | ||
| var entityEggDropChance: Double = 0.1, | ||
| var entityEggDropChance: Double = 0.001, | ||
| var animalScaleRange: Utils.Range? = Utils.Range(0.8, 1.2), |
There was a problem hiding this comment.
The monsterScaleRange default of 0.9 to 1.9 allows monsters to spawn up to 1.9x their normal size, which could create significant gameplay impact (hitboxes, visibility, balance). Consider documenting this range or adding validation to ensure reasonable limits, or provide a comment explaining the intended gameplay impact of these values.
| var animalScaleRange: Utils.Range? = Utils.Range(0.8, 1.2), | |
| var animalScaleRange: Utils.Range? = Utils.Range(0.8, 1.2), | |
| /** | |
| * Random scale range for hostile mobs. | |
| * | |
| * The default range of 0.9–1.9 intentionally allows monsters to spawn | |
| * noticeably smaller or larger than normal (up to just under 2x size), | |
| * which affects hitboxes, visibility, and combat difficulty. Values | |
| * significantly higher than ~2.0 are not recommended as they may | |
| * lead to extreme hitboxes or balance issues depending on the mob. | |
| */ |
Description
How Has This Been Tested?