Added horse entity and bug fixes #278
Conversation
- Fixed horse variant data missing tags and typo - Added horse entity
WalkthroughThe pull request introduces changes to two primary files related to horse entities in the codebase. The Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/HorseEntity.kt (1)
68-74: Consider logging unhandledEntityPropertytypes inapplyPropertyIn the
whenexpression withinapplyProperty, unhandledEntityPropertytypes are silently ignored in theelsebranch. To improve maintainability and debugging, consider adding a log statement or handling mechanism to capture unexpected property types. This ensures all properties are appropriately addressed and aids in troubleshooting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/data/minecraft/living/horse/HorseVariantData.kt(2 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/HorseEntity.kt(1 hunks)
🔇 Additional comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/data/minecraft/living/horse/HorseVariantData.kt (1)
16-17: Annotations updated appropriately for consistency
Renaming the entry identifier to "horse_variant_data" and adding the @Tags("horse_variant_data") annotation enhances clarity and ensures consistent tagging within the codebase.
- Added horse eating data
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/HorseEntity.kt (2)
28-33: Documentation could be more descriptiveThe current documentation is quite basic. Consider enhancing it with:
- Available configuration options
- Examples of usage
- Description of each property's purpose
46-52: Fix incomplete documentation sectionThe "How could this be used?" section is empty. Consider adding:
- Practical examples of instantiating a horse
- Common configuration scenarios
- Interaction with other components
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/HorseEntity.kt(1 hunks)
🔇 Additional comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/HorseEntity.kt (1)
39-40: Verify the completeness of allowed data tags
Let's verify that all necessary data tags are included in the @OnlyTags annotation.
✅ Verification successful
Let me gather more information about the data classes and their relationships.
Based on the gathered information, I can now provide the final response:
The @OnlyTags annotation includes all necessary data tags for the HorseEntity
The current set of tags ("generic_entity_data", "living_entity_data", "mob_data", "ageable_data", "horse_variant_data", "horse_data", "eating_data", "chested_horse_data") is complete and covers all the required data types for a horse entity, as evidenced by:
- All horse-related data classes (HorseEatingData, HorseVariantData, ChestedHorseChestData) are covered
- All hierarchical entity data tags (generic, living, mob, ageable) are included
- All horse-specific behavior tags (variant, eating, chested) are present
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other potential horse-related data tags in the codebase
rg -t kotlin "Tags\(.*horse.*\)" --no-filename | sort -u
# Search for any EntityData implementations that might be relevant
ast-grep --pattern 'class $_ : EntityData<$_>'
Length of output: 548
Script:
#!/bin/bash
# Search for all data-related tags in the codebase
rg -t kotlin "@Tags\(.*\)" --no-filename | sort -u
# Look for horse-related entity data classes
rg -t kotlin "class.*Horse.*Data" -A 5
# Search for any EntityData implementations specifically for horses
ast-grep --pattern 'class $name : EntityData<$type> {
$$$
}'
Length of output: 6825
| override fun applyProperty(property: EntityProperty) { | ||
| when (property) { | ||
| is AgeableProperty -> applyAgeableData(entity, property) | ||
| is HorseVariantProperty -> applyHorseVariantData(entity, property) | ||
| is ChestedHorseChestProperty -> applyChestedHorseChestData(entity, property) | ||
| is SaddledProperty -> applySaddledData(entity, property) | ||
| is EatingProperty -> applyHorseEatingData(entity, property) | ||
| else -> {} | ||
| } | ||
| if (applyGenericEntityData(entity, property)) return | ||
| if (applyLivingEntityData(entity, property)) return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve property handling robustness
Several concerns in the property application logic:
- Empty else block could silently ignore unknown properties
- Early returns after generic/living entity data application might prevent processing of other valid properties
- No error handling or logging for invalid properties
Consider this improved implementation:
override fun applyProperty(property: EntityProperty) {
+ var handled = false
when (property) {
- is AgeableProperty -> applyAgeableData(entity, property)
- is HorseVariantProperty -> applyHorseVariantData(entity, property)
- is ChestedHorseChestProperty -> applyChestedHorseChestData(entity, property)
- is SaddledProperty -> applySaddledData(entity, property)
- is EatingProperty -> applyHorseEatingData(entity, property)
- else -> {}
+ is AgeableProperty -> {
+ applyAgeableData(entity, property)
+ handled = true
+ }
+ is HorseVariantProperty -> {
+ applyHorseVariantData(entity, property)
+ handled = true
+ }
+ is ChestedHorseChestProperty -> {
+ applyChestedHorseChestData(entity, property)
+ handled = true
+ }
+ is SaddledProperty -> {
+ applySaddledData(entity, property)
+ handled = true
+ }
+ is EatingProperty -> {
+ applyHorseEatingData(entity, property)
+ handled = true
+ }
}
- if (applyGenericEntityData(entity, property)) return
- if (applyLivingEntityData(entity, property)) return
+ handled = handled || applyGenericEntityData(entity, property) || applyLivingEntityData(entity, property)
+
+ if (!handled) {
+ logger.warn("Unhandled property type: ${property::class.simpleName} for horse entity")
+ }
}Committable suggestion skipped: line range outside the PR's diff.
Related to: https://sharing.clickup.com/9015308602/v/6-901502296591-2/t/h/86c18junj/a4c4c9d59ec667d
Summary by CodeRabbit
New Features
HorseDefinitionandHorseInstanceclasses for better representation of horse entities.Bug Fixes