Merge pull request #8602 from MSedgeMC/fix/fireball

🤖 Agent: desktop (NixOS, -j 8) on head + main: fmt, clippy -D warnings, horizon-client-lib + horizon-client tests (1589 lib + 812 game passed, GPU tests skipped). Queued.

crates/horizon-client/src/entity_nbt.rs

@@ -2498,19 +2498,10 @@ pub fn fireball_to_nbt(
     if let Some(power) = power {
         put(&mut tag, "ExplosionPower", Tag::Byte(power as i8));
     }
-    // Fireball.addAdditionalSaveData: the item it is drawn as, one of the loaded
-    // item or else a fire charge (`setItem` refuses an empty stack and keeps one).
-    if matches!(
-        p.kind,
-        ProjectileKind::LargeFireball { .. } | ProjectileKind::SmallFireball
-    ) {
-        let mut item = tag
-            .get("Item")
-            .and_then(item_from_nbt)
-            .filter(|i| i.count > 0 && &*i.item != "minecraft:air")
-            .unwrap_or_else(|| crate::items::ItemStack::new("minecraft:fire_charge", 1));
-        item.count = 1;
-        put(&mut tag, "Item", item_to_nbt(&item));
+    // Fireball.addAdditionalSaveData: the item `getItem` returns, a fire
+    // charge unless `setItem` was given another (kept as a single one).
+    if let Some(item) = &p.item {
+        put(&mut tag, "Item", item_to_nbt(item));
     }
     Some(tag)
 }
@@ -2722,6 +2713,10 @@ pub fn owner_uuid(tag: &Tag) -> Option<[i32; 4]> {
     uuid_from_tag(tag.get("Owner"))
 }
 
+/// `Fireball.readAdditionalSaveData` (and, around it, `Projectile`'s Owner and
+/// LeftOwner and `AbstractHurtingProjectile`'s acceleration power): a fireball
+/// also reads the item it is drawn as through `setItem`, whose default is the
+/// fire charge; a wind charge is no `Fireball` and has none.
 pub fn fireball_from_nbt(tag: &Tag, player: Option<[i32; 4]>) -> Option<Projectile> {
     let kind = match kind(tag)?.as_str() {
         "minecraft:fireball" => ProjectileKind::LargeFireball {
@@ -2742,6 +2737,9 @@ pub fn fireball_from_nbt(tag: &Tag, player: Option<[i32; 4]>) -> Option<Projecti
     p.owner_uuid = owner;
     p.no_gravity = flag(tag, "NoGravity");
     p.acceleration = float(tag.get("acceleration_power")).unwrap_or(0.1);
+    if let Some(item) = tag.get("Item").and_then(item_from_nbt) {
+        p.set_item(item);
+    }
     Some(p)
 }
 
@@ -5689,7 +5687,7 @@ mod fireball_nbt_tests {
                 ("minecraft:fire_charge".to_owned(), 1),
                 "{id}"
             );
-            // A loaded item stays, as a single one.
+            // A loaded item stays, as a single one, on the entity itself.
             let mut loaded = plain.clone();
             put(
                 &mut loaded,
@@ -5697,7 +5695,18 @@ mod fireball_nbt_tests {
                 item_to_nbt(&crate::items::ItemStack::new("minecraft:diamond", 5)),
             );
             let p = fireball_from_nbt(&loaded, None).unwrap();
-            let back = fireball_to_nbt(&p, [1, 2, 3, 4], None, Some(&loaded)).unwrap();
+            let held = p.item.as_ref().unwrap();
+            assert_eq!((&*held.item, held.count), ("minecraft:diamond", 1), "{id}");
+            // `addAdditionalSaveData` writes `getItem()`: the item in the
+            // compound it was loaded from does not replace the one it holds,
+            // which is what `/data get` and the chunk save then report.
+            let mut stale = plain.clone();
+            put(
+                &mut stale,
+                "Item",
+                item_to_nbt(&crate::items::ItemStack::new("minecraft:stone", 8)),
+            );
+            let back = fireball_to_nbt(&p, [1, 2, 3, 4], None, Some(&stale)).unwrap();
             assert_eq!(item_of(&back), ("minecraft:diamond".to_owned(), 1), "{id}");
             // An empty one falls back to the fire charge.
             let mut empty = plain.clone();

crates/horizon-client/src/game/creatures.rs

@@ -7234,6 +7234,19 @@ impl Game<'_> {
                 let size = a.kind.size() as f32;
                 flame_geometry(atlas, c, size, size, self.yaw, light, out);
             }
+            // `ThrownItemRenderer` for a fireball: the item `Fireball.getItem`
+            // returns (a fire charge until `setItem` gave it another), its model
+            // at the renderer's scale for the kind. `packed` is already 15, the
+            // renderer's full bright.
+            if let Some(stack) = a.item.as_ref() {
+                let owner = super::item_draw::owner_at(at, 0.0, a.kind.id());
+                let state = self.resolve_item(stack, DisplayContext::Ground, Some(&owner), 0);
+                let pose = glam::Mat4::from_translation(c)
+                    * glam::Mat4::from_quat(self.camera_orientation())
+                    * glam::Mat4::from_scale(glam::Vec3::splat(a.sprite_scale()));
+                self.submit_item_world(&state, pose, packed, out);
+                continue;
+            }
             let Some(name) = a.item_sprite() else {
                 continue;
             };
@@ -7294,15 +7307,12 @@ fn thrown_item_quad(
 }
 /// Whether vanilla draws the kind as `ThrownItemRenderer`'s item billboard, the
 /// only ones that take that lift. Llama spit shares this quad but
-/// `LlamaSpitRenderer` is a model, with its own offset.
+/// `LlamaSpitRenderer` is a model, with its own offset, and a fireball's item
+/// is drawn as the item model that carries the lift itself.
 fn is_item_billboard(kind: &ProjectileKind) -> bool {
     matches!(
         kind,
-        ProjectileKind::Snowball
-            | ProjectileKind::Egg
-            | ProjectileKind::EnderPearl
-            | ProjectileKind::SmallFireball
-            | ProjectileKind::LargeFireball { .. }
+        ProjectileKind::Snowball | ProjectileKind::Egg | ProjectileKind::EnderPearl
     )
 }
 /// `Bucketable.getBucketItemStack` and `getPickupSound` of a mob that a water
@@ -7548,13 +7558,15 @@ mod tests {
 
     #[test]
     fn thrown_item_billboard_lifts_by_the_generated_ground_transform() {
-        // The kinds vanilla draws with `ThrownItemRenderer`: the only lifted ones.
+        // The kinds vanilla draws with `ThrownItemRenderer` as a billboard: the
+        // only lifted ones. A fireball hands `getItem` to the item renderer
+        // instead, which carries the ground transform itself.
         assert!(is_item_billboard(&ProjectileKind::Snowball));
         assert!(is_item_billboard(&ProjectileKind::Egg));
         assert!(is_item_billboard(&ProjectileKind::EnderPearl));
-        assert!(is_item_billboard(&ProjectileKind::SmallFireball));
+        assert!(!is_item_billboard(&ProjectileKind::SmallFireball));
         let large = ProjectileKind::LargeFireball { power: 1 };
-        assert!(is_item_billboard(&large));
+        assert!(!is_item_billboard(&large));
         // A projectile at y = 100 with right = +X, up = +Y: the sprite sits
         // 0.125 blocks above the entity (2 of `item/generated`'s 16 units) and
         // stays 0.5 blocks wide, so the quad's y runs 99.875 .. 100.375.
@@ -7564,13 +7576,6 @@ mod tests {
         let (lo, hi) = bounds(&thrown_item_quad(at, right, up, 1.0, lifted));
         assert_eq!((lo.y, hi.y), (99.875, 100.375));
         assert_eq!((lo.x, hi.x), (-0.25, 0.25));
-        // A small fireball's 0.75 scale: the same quad scaled down, lifted by
-        // 0.09375, half extent 0.1875.
-        let (lo, hi) = bounds(&thrown_item_quad(at, right, up, 0.75, lifted));
-        assert_eq!((lo.y, hi.y), (99.90625, 100.28125));
-        // A large fireball's 3.0: lift 0.375, half extent 0.75.
-        let (lo, hi) = bounds(&thrown_item_quad(at, right, up, 3.0, lifted));
-        assert_eq!((lo.y, hi.y), (99.625, 101.125));
         // Llama spit shares the quad here but is a model in vanilla
         // (`LlamaSpitRenderer`): no lift at all.
         let lifted = is_item_billboard(&ProjectileKind::LlamaSpit);

crates/horizon-client/src/game/lan_entities.rs

@@ -647,6 +647,17 @@ impl Game<'_> {
                         Value::Item(hub.stack(&Some((**stack).clone()))),
                     ));
                 }
+                // Fireball.DATA_ITEM_STACK: the item it is drawn as, which a
+                // guest has no other way of learning. A plain fire charge is
+                // the client's default and is not sent.
+                K::SmallFireball | K::LargeFireball { .. }
+                    if p.item.as_ref().is_some_and(|i| {
+                        &*i.item != "minecraft:fire_charge" || i.count != 1 || i.extra.is_some()
+                    }) =>
+                {
+                    e.meta
+                        .push((8, serializer::ITEM_STACK, Value::Item(hub.stack(&p.item))));
+                }
                 _ => {}
             }
             // ChunkMap.addEntity (#7461): a fresh arrow joined before its first tick

crates/horizon-client/src/game/remote_projectiles.rs

@@ -179,6 +179,18 @@ fn shulker_bullet_trail_at(at: Vec3, random: &mut impl RandomSource) -> [f64; 3]
     ]
 }
 
+/// `ClientPacketListener.handleSetEntityData`'s `Fireball.DATA_ITEM_STACK`:
+/// the item the guest draws the fireball as, through `Fireball.setItem`;
+/// a kind that is no `Fireball` has none.
+pub(super) fn remote_projectile_item(projectiles: &mut [Projectile], id: u32, stack: ItemStack) {
+    if let Some(p) = projectiles
+        .iter_mut()
+        .find(|p| p.id == id && p.kind.is_item_fireball())
+    {
+        p.set_item(stack);
+    }
+}
+
 /// `ClientPacketListener.handleProjectilePowerPacket`: the power a hurting
 /// projectile (`AbstractHurtingProjectile`) accelerates by, found by id;
 /// anything else is ignored.
@@ -503,6 +515,17 @@ impl Game<'_> {
             }
             return;
         }
+        // Fireball.DATA_ITEM_STACK: the item a fireball is drawn as.
+        if let Some(stack) = values
+            .iter()
+            .find_map(|(index, _, value)| match (index, value) {
+                (8, Value::Item(stack)) => Some(stack),
+                _ => None,
+            })
+            && let Some(stack) = self.to_stack(stack)
+        {
+            remote_projectile_item(&mut self.creatures.projectiles, id, stack);
+        }
         for (index, _, value) in values {
             if let Some(t) = self.tnts.iter_mut().find(|t| t.id == id) {
                 // PrimedTnt.DATA_FUSE_ID.
@@ -1086,4 +1109,31 @@ mod tests {
             [0.1; 3]
         );
     }
+
+    #[test]
+    fn a_guest_draws_a_fireball_as_the_item_the_server_sends() {
+        // `ClientPacketListener.handleSetEntityData` of
+        // Fireball.DATA_ITEM_STACK: the item `Fireball.getItem` returns, which
+        // is the fire charge the default was until the server says otherwise.
+        let mut list = vec![
+            tracked(ProjectileKind::LargeFireball { power: 1 }, 12),
+            tracked(ProjectileKind::SmallFireball, 13),
+            tracked(ProjectileKind::Snowball, 14),
+        ];
+        remote_projectile_item(&mut list, 12, ItemStack::new("minecraft:diamond", 4));
+        assert_eq!(
+            list[0].item.as_ref().map(|i| (&*i.item, i.count)),
+            Some(("minecraft:diamond", 1)),
+            "one of it, as `setItem` keeps"
+        );
+        // The default fire charge goes back when the server sends an empty
+        // stack, and a snowball is no `Fireball`: it has no item to set.
+        remote_projectile_item(&mut list, 13, ItemStack::new("minecraft:air", 1));
+        assert_eq!(
+            list[1].item.as_ref().map(|i| (&*i.item, i.count)),
+            Some(("minecraft:fire_charge", 1))
+        );
+        remote_projectile_item(&mut list, 14, ItemStack::new("minecraft:diamond", 1));
+        assert_eq!(list[2].item, None);
+    }
 }

crates/horizon-client/src/projectiles.rs

@@ -133,6 +133,13 @@ impl ProjectileKind {
     pub fn ignores_blast(&self) -> bool {
         matches!(self, Self::WindCharge | Self::BreezeWindCharge)
     }
+    /// Whether the kind is a `Fireball` proper (`LargeFireball`,
+    /// `SmallFireball`): the projectiles that carry an `Item`
+    /// (`Fireball.DATA_ITEM_STACK`) and are drawn as it. A `DragonFireball`
+    /// and a `WitherSkull` extend the hurting projectile only.
+    pub fn is_item_fireball(&self) -> bool {
+        matches!(self, Self::SmallFireball | Self::LargeFireball { .. })
+    }
     /// `EntityType.Builder.eyeHeight`: 0.13 for arrows and tridents, 0 for
     /// wind charges, else the default 0.85 of the box height.
     pub fn eye_height(&self) -> f64 {
@@ -240,6 +247,10 @@ pub struct Projectile {
     /// `Entity.remainingFireTicks`: a thrown projectile that flew through a
     /// fire block burns for 160 ticks, with the flame drawn over it.
     pub fire_ticks: i32,
+    /// `Fireball.DATA_ITEM_STACK`: the item a fireball is drawn as, a fire
+    /// charge until [`Projectile::set_item`] gives it another. None for the
+    /// kinds that are no `Fireball` (a wither skull, a wind charge).
+    pub item: Option<crate::items::ItemStack>,
 }
 /// `ThrownEnderpearl.isAllowedToTeleportOwner(owner, level)` for an owner in
 /// the pearl's own dimension: vanilla reaches
@@ -367,6 +378,10 @@ impl Projectile {
     }
     pub fn new(kind: ProjectileKind, position: Vec3, owner: Option<Who>) -> Self {
         let kind_is_arrow = matches!(kind, ProjectileKind::Arrow { .. } | ProjectileKind::Trident);
+        // `Fireball.defineSynchedData`: DATA_ITEM_STACK starts as a fire charge.
+        let item = kind
+            .is_item_fireball()
+            .then(|| crate::items::ItemStack::new("minecraft:fire_charge", 1));
         // `AbstractWindCharge`'s constructors set it to 0.
         let acceleration = if matches!(
             kind,
@@ -401,6 +416,7 @@ impl Projectile {
             touching_water: false,
             no_gravity: false,
             fire_ticks: 0,
+            item,
             arrow: kind_is_arrow.then(|| {
                 // Entity.random: seeded unpredictably, the position stands in.
                 Box::new(crate::arrows::ArrowState::new(
@@ -706,6 +722,21 @@ impl Projectile {
         p.update_rotation();
         p
     }
+    /// `Fireball.setItem`: the item a fireball is drawn as. An empty stack
+    /// puts the default fire charge back, anything else is kept as a single
+    /// one (`copyWithCount(1)`); a kind that is no `Fireball` has no item.
+    pub fn set_item(&mut self, stack: crate::items::ItemStack) {
+        if !self.kind.is_item_fireball() {
+            return;
+        }
+        let mut stack = if stack.count == 0 || &*stack.item == "minecraft:air" {
+            crate::items::ItemStack::new("minecraft:fire_charge", 1)
+        } else {
+            stack
+        };
+        stack.count = 1;
+        self.item = Some(stack);
+    }
     fn update_rotation(&mut self) {
         let v = self.velocity;
         let sd = (v.x * v.x + v.z * v.z).sqrt();
@@ -1220,7 +1251,8 @@ impl Projectile {
             }
         })
     }
-    /// Render scale of the item billboard (fireball renderers).
+    /// `ThrownItemRenderer`'s scale for the kind: 3 for a large fireball,
+    /// 0.75 for a small one, 1 for the rest.
     pub fn sprite_scale(&self) -> f32 {
         match self.kind {
             ProjectileKind::LargeFireball { .. } => 3.0,
@@ -1696,6 +1728,66 @@ mod tests {
         assert!((p.velocity.x - 0.19).abs() < 1e-9);
     }
 
+    /// `Fireball.setItem` with `Fireball.defineSynchedData`'s default and
+    /// `getItem`: a fireball is drawn as a fire charge until it is given
+    /// another item, and it keeps a single one of it. A kind that is no
+    /// `Fireball` (a wither skull, a snowball) has no item to set.
+    #[test]
+    fn a_fireball_holds_one_of_the_item_it_is_given() {
+        let held = |p: &Projectile| {
+            p.item
+                .as_ref()
+                .map(|i| (&*i.item, i.count))
+                .map(|(i, n)| (i.to_string(), n))
+        };
+        for kind in [
+            ProjectileKind::LargeFireball { power: 1 },
+            ProjectileKind::SmallFireball,
+        ] {
+            let mut p = Projectile::new(kind, Vec3::new(0.0, 100.0, 0.0), None);
+            assert_eq!(
+                held(&p),
+                Some(("minecraft:fire_charge".to_owned(), 1)),
+                "the default"
+            );
+            p.set_item(crate::items::ItemStack::new("minecraft:diamond", 5));
+            assert_eq!(
+                held(&p),
+                Some(("minecraft:diamond".to_owned(), 1)),
+                "copyWithCount(1)"
+            );
+            // `setItem` refuses an empty stack, either way of being empty, and
+            // puts the default fire charge back.
+            for empty in [
+                crate::items::ItemStack::new("minecraft:air", 1),
+                crate::items::ItemStack::new("minecraft:diamond", 0),
+            ] {
+                p.set_item(empty);
+                assert_eq!(held(&p), Some(("minecraft:fire_charge".to_owned(), 1)));
+            }
+        }
+        for kind in [
+            ProjectileKind::WitherSkull { dangerous: false },
+            ProjectileKind::Snowball,
+        ] {
+            let mut p = Projectile::new(kind, Vec3::new(0.0, 100.0, 0.0), None);
+            assert_eq!(p.item, None);
+            p.set_item(crate::items::ItemStack::new("minecraft:diamond", 1));
+            assert_eq!(p.item, None, "no Fireball, no item");
+        }
+    }
+    /// `ThrownItemRenderer`'s scale per entity type, which the item model a
+    /// fireball is drawn as is scaled by: 3 for a fireball, 0.75 for a small
+    /// one, 1 for the other kinds and their own renderers.
+    #[test]
+    fn a_fireball_draws_at_thrice_or_three_quarters() {
+        let scale = |kind: ProjectileKind| {
+            Projectile::new(kind, Vec3::new(0.0, 100.0, 0.0), None).sprite_scale()
+        };
+        assert_eq!(scale(ProjectileKind::LargeFireball { power: 1 }), 3.0);
+        assert_eq!(scale(ProjectileKind::SmallFireball), 0.75);
+        assert_eq!(scale(ProjectileKind::Snowball), 1.0);
+    }
     #[test]
     fn fireball_inertia_preserves_custom_acceleration() {
         let mut p = Projectile::fireball(

tools/port_map/classes-01.tsv

@@ -79,9 +79,9 @@ PlayerSkin.with	missing		minor: skin patches (body, cape, elytra and model overr
 PlayerSkin.Patch.create	skip:java		record factory for the patch; the patch itself is missing (see PlayerSkin.with)
 SpectralArrow.doPostHurtEffects	port	crates/horizon-client/src/game/potions.rs::Game::arrow_post_hurt_effects	200-tick Glowing on any target a spectral arrow lands on (creatures.rs:3589); a bow, a crossbow and a dispenser also store the effect on the arrow
 SpectralArrow.tick	port	crates/horizon-client/src/game/remote_projectiles.rs::Game::arrow_effect_particles	client tick: an effect sparkle at the arrow's position while it is not stuck; the tipped arrow's colour swirls share the fn
-Fireball.getSlot	missing		minor: the fireball's item slot (its displayed fire charge) is not modelled
-Fireball.readAdditionalSaveData	missing		`fireball_from_nbt` reads no `Item` key, so a saved fireball is drawn as the default fire charge
-Fireball.setItem	missing		minor: the displayed item of a fireball is not set
+Fireball.getSlot	missing		minor: entity slot access (`/loot insert` on an entity) is not modelled, so nothing here can set the slot the fireball's item sits in
+Fireball.readAdditionalSaveData	port	crates/horizon-client/src/entity_nbt.rs::fireball_from_nbt	the `Item` it is drawn as goes through `setItem`, which refuses an empty stack and keeps a single one
+Fireball.setItem	port	crates/horizon-client/src/projectiles.rs::Projectile::set_item	an empty stack puts the default fire charge back, anything else is kept as one (`copyWithCount(1)`); a kind that is no `Fireball` has no item
 Fireball.shouldRenderAtSqrDistance	skip:ui		render distance rule for the first two ticks; Horizon draws the projectile as its own pass
 LargeFireball.onHit	port	crates/horizon-client/src/game/creatures.rs::Game::projectile_hit	LargeFireball arm: explosion of the given power, then the projectile is gone
 LargeFireball.onHitEntity	port	crates/horizon-client/src/game/creatures.rs::Game::projectile_hit	LargeFireball arm: 6 damage to the entity

tools/port_map/client-004.tsv

@@ -129,8 +129,8 @@ StriderRenderer.isShaking	port	crates/horizon-client/src/mobs/render.rs::shaking
 SulfurCubeRenderer.submit	port	crates/horizon-client/src/mobs/sulfur_cube.rs::body	normal or small model
 SulfurCubeRenderer.extractRenderState	port	crates/horizon-client/src/game/sulfur_cubes.rs::Game::sulfur_cube_body_geometry	fuse flash, contained block and worn head
 SulfurCubeRenderer.applySizeAndSquish	port	crates/horizon-client/src/mobs/sulfur_cube.rs::scale	size and squish scale
-ThrownItemRenderer.submit	port	crates/horizon-client/src/game/creatures.rs::Game::mob_geometry	thrown item sprite at its scale
-ThrownItemRenderer.extractRenderState	inline	crates/horizon-client/src/game/creatures.rs::Game::mob_geometry	item sprite read there
+ThrownItemRenderer.submit	port	crates/horizon-client/src/game/creatures.rs::Game::mob_geometry	thrown item sprite at its scale; a fireball's stack is drawn as the item model (`DisplayContext.Ground`) at 3 or 0.75, full bright
+ThrownItemRenderer.extractRenderState	inline	crates/horizon-client/src/game/creatures.rs::Game::mob_geometry	item sprite read there; a fireball's stack is `Projectile.item` (`Fireball.getItem`)
 ThrownTridentRenderer.submit	port	crates/horizon-client/src/game/trident.rs::Game::trident_geometry	gap: minor: foil glint is not drawn on a thrown trident
 ThrownTridentRenderer.extractRenderState	inline	crates/horizon-client/src/game/trident.rs::Game::trident_geometry	yaw, pitch and foil read there
 TippableArrowRenderer.extractRenderState	port	crates/horizon-client/src/projectiles.rs::Projectile::arrow_effect_color	brewing::potion_color gives a tipped arrow its potion colour