Feat/crystals+anchors#31
Open
HashimTheArab wants to merge 16 commits into
Open
Conversation
- Implement end crystals as behavior - Add EndCrystal to explosion config
Constraint: PR df-mc#1247 had merge conflicts against upstream/master in block registration, projectile behavior, and sound declarations. Rejected: Dropping upstream redstone/piercing changes | conflict resolution must preserve current base behavior. Confidence: high Scope-risk: moderate Directive: Keep projectile piercing collision tracking and non-living damageable entities in sync when editing ProjectileBehaviour. Tested: go test ./...; git diff --check Not-tested: In-game Bedrock client interaction.
End crystal placement and damage behavior needed source-backed parity before PR df-mc#1247 could be reviewed cleanly. This keeps the fixes isolated from the upstream merge already pushed to the contributor branch. Constraint: Primary source guidance came from ../AGENTS.md: Minecraft Wiki and mcsrc.dev first, with Dragonfly conventions preserved where code patterns already existed. Rejected: Keeping duplicate non-living damage helpers | player and projectile paths both need the same Behaviour-backed damage semantics. Confidence: high Scope-risk: moderate Directive: Keep End crystal zero-damage, explosion-damage, placement-fire, and continuous-End-fire behavior covered when changing entity or item placement paths. Tested: go test ./... Not-tested: Live Bedrock client interaction; dedicated regression tests per requester direction.
…fixes Fix source-backed crystal and anchor parity issues
Constraint: PR df-mc#1247 had already merged before this review cleanup, so this follow-up keeps the change limited to the offset declaration. Rejected: Keeping generated offset initialization | the explicit priority table is clearer and avoids an init-time helper for fixed vanilla order. Confidence: high Scope-risk: narrow Directive: Preserve the column layer-1/layer-2 priority before layer 3 when editing respawn anchor spawn offsets. Tested: go test ./...; git diff --check Not-tested: Live Bedrock client respawn flow.
Constraint: PR df-mc#1247 follow-up review requested documentation for helper tables and questioned whether a missing End crystal beam target should serialize as 0,0,0. Rejected: Sending a zero BlockTarget for crystals without a beam target | zero is a concrete position and can be interpreted as a beam target. Confidence: high Scope-risk: narrow Directive: Omit EntityDataKeyBlockTarget unless an End crystal has an explicit beam target. Tested: go test ./...; git diff --check Not-tested: Live Bedrock client metadata rendering.
Constraint: PR df-mc#1247 follow-up review requested documentation for helper tables and questioned whether a missing End crystal beam target should serialize as 0,0,0. Rejected: Sending a zero BlockTarget for crystals without a beam target | zero is a concrete position and can be interpreted as a beam target. Confidence: high Scope-risk: narrow Directive: Omit EntityDataKeyBlockTarget unless an End crystal has an explicit beam target. Tested: go test ./...; git diff --check Not-tested: Live Bedrock client metadata rendering.
Constraint: Follow-up PR review requested clarity on HurtEntity return values without changing behavior. Rejected: Adding explicit ProjectileDamageSource ExplodesEndCrystal true | End crystal explosion behavior is opt-out by default, so explicit true methods add noise. Confidence: high Scope-risk: narrow Directive: Keep HurtEntity return tuple semantics aligned with Living.Hurt plus the damageable ok flag. Tested: go test ./...; git diff --check Not-tested: Live Bedrock client interaction.
Constraint: Respawn anchors and beds share the same safe-spawn contract in player respawn logic. Rejected: Keeping the anonymous inline interface | It obscures the reusable local contract and makes the respawn path harder to scan. Confidence: high Scope-risk: narrow Directive: Keep this interface private unless a non-player package needs to refer to the contract directly. Tested: go test ./...; git diff --check Not-tested: Live Bedrock client respawn interaction.
Constraint: Bedrock lang sources keep tile.bed.notValid scoped to bed wording while respawn anchors have their own notValid key. Rejected: Java-style combined bed and anchor fallback | It does not match Dragonfly's original fallback or current Bedrock sample text. Confidence: high Scope-risk: narrow Directive: Keep chat translation fallbacks aligned with Bedrock language keys, not wiki prose when they differ. Tested: go test ./...; git diff --check Not-tested: Live Bedrock client locale rendering.
📝 WalkthroughWalkthroughThis PR adds End Crystals and Respawn Anchors as complete feature implementations, introduces Piercing enchantment with arrow configuration consolidation, refactors projectile piercing behavior, and provides supporting improvements including light emission levels, explosion Y-level clipping, absorption mechanics, and block registry optimizations. ChangesEnd Crystal Entity and Item
Respawn Anchor Block and Integration
Projectile Piercing Enchantment System
Entity Damage Mechanics and Abstraction
Supporting Mechanics and Enhancements
Sequence Diagram(s)sequenceDiagram
participant Player
participant EndCrystal as End Crystal
participant World
participant Explosion
Player->>EndCrystal: Attack / Damage
EndCrystal->>EndCrystal: Hurt (damage, source)
alt VoidDamageSource or ExplosionDamageSource
EndCrystal->>World: Close entity
else Other source (check endCrystalExploder)
EndCrystal->>Explosion: Trigger explosion (EndCrystal: true, size 6)
Explosion->>World: Clip entities below origin Y
Explosion->>World: Clip blocks below origin Y
end
Explosion->>World: Deal damage in radius
sequenceDiagram
participant Crossbow
participant Arrow as Projectile
participant Entity
participant World
Crossbow->>Arrow: Spawn with PiercingLevel
Note over Arrow: Store collidedEntities = []
Arrow->>World: Tick and move
World->>Entity: Check collision
Entity->>Arrow: Is damageable?
alt Yes and not in collidedEntities
Arrow->>Entity: HurtEntity (damage, source)
Entity->>Arrow: Record in collidedEntities
alt collidedEntities count > PiercingLevel
Arrow->>World: Close entity
else Continue
Arrow->>Arrow: Maintain velocity, continue
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6f5385b. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/entity/projectile.go`:
- Around line 179-183: The collision bookkeeping and hit handling are
incorrectly gated on lt.conf.Damage >= 0 which prevents non-damaging projectiles
from registering hits; remove the Damage >= 0 guard so that
lt.hitEntity(r.Entity(), e, vel) is called on entity collision for all
projectiles and ensure lt.collidedEntities = append(lt.collidedEntities,
r.Entity().H()) is executed when damageableEntity(r.Entity()) is true regardless
of lt.conf.Damage; apply the same change to the other similar blocks that
reference lt.conf.Damage, lt.hitEntity, lt.collidedEntities, and
damageableEntity so collisions are always recorded even for negative-damage
configs.
In `@server/entity/register.go`:
- Around line 52-56: The Arrow spawn function currently does an unchecked type
assertion tip := arrow.Tip.(potion.Potion) and directly calls arrow.Owner.H(),
which can panic if Tip is nil/unset or Owner is nil; update the Arrow function
to defensively check arrow.Tip's type (use a type assertion with ok or a nil
check) before assigning to tip and fall back to a safe default potion or leave
conf.Potion unset, and also validate arrow.Owner is non-nil before calling
arrow.Owner.H() (or use a safe owner handle getter); ensure you still set
conf.Damage, conf.KnockBackForceAddend, and other fields only after these checks
so spawn-time panics are prevented.
In `@server/player/player.go`:
- Around line 1793-1803: The branch for non-living entities returns before the
item's durability is updated; when entity.HurtEntity(e, i.AttackDamage(), ...)
succeeds you must invoke the same durability-wear logic used for living targets
before returning true. Modify the non-living branch (the block using
entity.HurtEntity, p.tx.PlaySound and p.Exhaust) to call the existing
item-durability handler/mutation that the living-target path uses (i.e., the
same code that updates the item stack and triggers break/consume) so durable
items are decremented on successful hits against non-living entities as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf686165-893e-4c43-90d8-aaab341ed2a3
📒 Files selected for processing (30)
server/block/blast_furnace.goserver/block/explosion.goserver/block/furnace.goserver/block/hash.goserver/block/register.goserver/block/respawn_anchor.goserver/block/smoker.goserver/entity/damage.goserver/entity/damageable.goserver/entity/end_crystal.goserver/entity/end_crystal_behaviour.goserver/entity/projectile.goserver/entity/register.goserver/item/bow.goserver/item/crossbow.goserver/item/enchantment/multishot.goserver/item/enchantment/piercing.goserver/item/enchantment/register.goserver/item/end_crystal.goserver/item/golden_apple.goserver/item/item.goserver/item/register.goserver/item/stack.goserver/player/chat/translate.goserver/player/player.goserver/session/entity_metadata.goserver/session/world.goserver/world/block_registry.goserver/world/entity.goserver/world/sound/block.go
Comment on lines
+179
to
+183
| if lt.conf.Damage >= 0 { | ||
| lt.hitEntity(r.Entity(), e, vel) | ||
| if damageableEntity(r.Entity()) { | ||
| lt.collidedEntities = append(lt.collidedEntities, r.Entity().H()) | ||
| } |
There was a problem hiding this comment.
Non-damaging projectiles no longer terminate on entity collision
Line 179 gates collision bookkeeping behind Damage >= 0, but Line 200 closes using collidedEntities length. For configs using negative damage, hits are never counted, so the projectile never closes after entity impact (and with Line 333 it can just stop moving in-place).
🔧 Proposed fix
switch r := result.(type) {
case trace.EntityResult:
- if lt.conf.Damage >= 0 {
- lt.hitEntity(r.Entity(), e, vel)
- if damageableEntity(r.Entity()) {
- lt.collidedEntities = append(lt.collidedEntities, r.Entity().H())
- }
- }
+ if damageableEntity(r.Entity()) {
+ if lt.conf.Damage >= 0 {
+ lt.hitEntity(r.Entity(), e, vel)
+ }
+ lt.collidedEntities = append(lt.collidedEntities, r.Entity().H())
+ }Also applies to: 200-202, 333-335
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/entity/projectile.go` around lines 179 - 183, The collision
bookkeeping and hit handling are incorrectly gated on lt.conf.Damage >= 0 which
prevents non-damaging projectiles from registering hits; remove the Damage >= 0
guard so that lt.hitEntity(r.Entity(), e, vel) is called on entity collision for
all projectiles and ensure lt.collidedEntities = append(lt.collidedEntities,
r.Entity().H()) is executed when damageableEntity(r.Entity()) is true regardless
of lt.conf.Damage; apply the same change to the other similar blocks that
reference lt.conf.Damage, lt.hitEntity, lt.collidedEntities, and
damageableEntity so collisions are always recorded even for negative-damage
configs.
Comment on lines
+52
to
+56
| Arrow: func(opts world.EntitySpawnOpts, arrow world.ArrowSpawnConfig) *world.EntityHandle { | ||
| tip := arrow.Tip.(potion.Potion) | ||
| conf := arrowConf | ||
| conf.Damage, conf.Potion, conf.Owner = damage, tip.(potion.Potion), owner.H() | ||
| conf.KnockBackForceAddend = float64(punchLevel) * enchantment.Punch.KnockBackMultiplier() | ||
| conf.DisablePickup = disallowPickup | ||
| if obtainArrowOnPickup { | ||
| conf.PickupItem = item.NewStack(item.Arrow{Tip: tip.(potion.Potion)}, 1) | ||
| conf.Damage, conf.Potion, conf.Owner = arrow.Damage, tip, arrow.Owner.H() | ||
| conf.KnockBackForceAddend = float64(arrow.PunchLevel) * enchantment.Punch.KnockBackMultiplier() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all ArrowSpawnConfig callsites and whether Tip/Owner are always set.
fd -e go | xargs rg -nP 'ArrowSpawnConfig\s*{' -C3
fd -e go | xargs rg -nP 'EntityRegistry\(\)\.Config\(\)\.Arrow\s*\(' -C3Repository: HashimTheArab/dragonfly
Length of output: 822
Defensively handle world.ArrowSpawnConfig fields to avoid spawn-time panics.
server/entity/register.go does tip := arrow.Tip.(potion.Potion), but server/item/crossbow.go constructs world.ArrowSpawnConfig{...} without setting Tip, so this can panic when spawning arrows. Guard the type assertion (and arrow.Owner.H() for safety).
💡 Suggested hardening
Arrow: func(opts world.EntitySpawnOpts, arrow world.ArrowSpawnConfig) *world.EntityHandle {
- tip := arrow.Tip.(potion.Potion)
+ tip, _ := arrow.Tip.(potion.Potion)
conf := arrowConf
- conf.Damage, conf.Potion, conf.Owner = arrow.Damage, tip, arrow.Owner.H()
+ conf.Damage, conf.Potion = arrow.Damage, tip
+ if arrow.Owner != nil {
+ conf.Owner = arrow.Owner.H()
+ }
conf.KnockBackForceAddend = float64(arrow.PunchLevel) * enchantment.Punch.KnockBackMultiplier()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Arrow: func(opts world.EntitySpawnOpts, arrow world.ArrowSpawnConfig) *world.EntityHandle { | |
| tip := arrow.Tip.(potion.Potion) | |
| conf := arrowConf | |
| conf.Damage, conf.Potion, conf.Owner = damage, tip.(potion.Potion), owner.H() | |
| conf.KnockBackForceAddend = float64(punchLevel) * enchantment.Punch.KnockBackMultiplier() | |
| conf.DisablePickup = disallowPickup | |
| if obtainArrowOnPickup { | |
| conf.PickupItem = item.NewStack(item.Arrow{Tip: tip.(potion.Potion)}, 1) | |
| conf.Damage, conf.Potion, conf.Owner = arrow.Damage, tip, arrow.Owner.H() | |
| conf.KnockBackForceAddend = float64(arrow.PunchLevel) * enchantment.Punch.KnockBackMultiplier() | |
| Arrow: func(opts world.EntitySpawnOpts, arrow world.ArrowSpawnConfig) *world.EntityHandle { | |
| tip, _ := arrow.Tip.(potion.Potion) | |
| conf := arrowConf | |
| conf.Damage, conf.Potion = arrow.Damage, tip | |
| if arrow.Owner != nil { | |
| conf.Owner = arrow.Owner.H() | |
| } | |
| conf.KnockBackForceAddend = float64(arrow.PunchLevel) * enchantment.Punch.KnockBackMultiplier() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/entity/register.go` around lines 52 - 56, The Arrow spawn function
currently does an unchecked type assertion tip := arrow.Tip.(potion.Potion) and
directly calls arrow.Owner.H(), which can panic if Tip is nil/unset or Owner is
nil; update the Arrow function to defensively check arrow.Tip's type (use a type
assertion with ok or a nil check) before assigning to tip and fall back to a
safe default potion or leave conf.Potion unset, and also validate arrow.Owner is
non-nil before calling arrow.Owner.H() (or use a safe owner handle getter);
ensure you still set conf.Damage, conf.KnockBackForceAddend, and other fields
only after these checks so spawn-time panics are prevented.
Comment on lines
1793
to
1803
| if !isLiving { | ||
| return false | ||
| n, vulnerable, ok := entity.HurtEntity(e, i.AttackDamage(), entity.AttackDamageSource{Attacker: p}) | ||
| if !ok { | ||
| return false | ||
| } | ||
| p.tx.PlaySound(entity.EyePosition(e), sound.Attack{Damage: !mgl64.FloatEqual(n, 0)}) | ||
| if vulnerable { | ||
| p.Exhaust(0.1) | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
Durability isn’t consumed when attacking non-living entities.
This new branch returns before the durability logic, so durable items never wear when damaging non-living targets.
Proposed fix
if !isLiving {
n, vulnerable, ok := entity.HurtEntity(e, i.AttackDamage(), entity.AttackDamageSource{Attacker: p})
if !ok {
return false
}
+ if durable, ok := i.Item().(item.Durable); ok {
+ _, left := p.HeldItems()
+ p.SetHeldItems(p.damageItem(i, durable.DurabilityInfo().AttackDurability), left)
+ }
p.tx.PlaySound(entity.EyePosition(e), sound.Attack{Damage: !mgl64.FloatEqual(n, 0)})
if vulnerable {
p.Exhaust(0.1)
}
return true
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/player/player.go` around lines 1793 - 1803, The branch for non-living
entities returns before the item's durability is updated; when
entity.HurtEntity(e, i.AttackDamage(), ...) succeeds you must invoke the same
durability-wear logic used for living targets before returning true. Modify the
non-living branch (the block using entity.HurtEntity, p.tx.PlaySound and
p.Exhaust) to call the existing item-durability handler/mutation that the
living-target path uses (i.e., the same code that updates the item stack and
triggers break/consume) so durable items are decremented on successful hits
against non-living entities as well.
Constraint: Respawn anchors must resolve in their saved dimension, End crystal placement checks the two-block column above the base, End crystal block clipping is source-backed only for supported crystals, and water must suppress End crystal entity impact. Rejected: Keeping position-only player spawn provider methods | Spawn position without dimension cannot model Bedrock's saved respawn target once anchors are supported. Confidence: medium Scope-risk: moderate Directive: Keep player spawn dimension persistence aligned with Bedrock player data and do not reapply End crystal Y clipping to entity exposure without a primary source. Tested: go test ./...; git diff --check Not-tested: Live Bedrock client respawn across dimensions; in-game End crystal water neutralization.
Fix respawn and End crystal parity gaps
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Medium Risk
Touches player respawn persistence, cross-dimension world resolution, and explosion/entity damage paths—important gameplay and data migration surface, though changes are localized to spawn and combat helpers.
Overview
Adds respawn anchors (glowstone charging, Nether spawn, safe spawn with charge depletion, overworld misuse explosion) and End crystals (placeable item, entity with beam/base metadata, End fire, vanilla-style explosion).
Player respawn is now dimension-aware:
PlayerSpawn/ provider persistence store position and dimension (SpawnDimensionin LevelDB); beds and anchors only update spawn when dimension matches; respawn resolves the correct world via optionalWorldByDimensionon players (wired from the server).Explosion gains End-crystal block clipping on obsidian/bedrock, optional entity-damage skip, and exposure divide-by-zero guard. Combat/projectiles use shared
HurtEntity/damageableEntityso non-Livingbehaviours (e.g. End crystals) can be hit; melee attacks non-living damageables too.Reviewed by Cursor Bugbot for commit 9bcc140. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Balance Changes
Bug Fixes