From de808e36129670562bdc55ecc0fe56108b4dedce Mon Sep 17 00:00:00 2001 From: Kacpherek Date: Mon, 26 Oct 2026 19:51:31 +0100 Subject: [PATCH 1/8] Empty a bundle by holding right click --- crates/horizon-client/src/bundle.rs | 4 +- crates/horizon-client/src/game.rs | 59 ++++++++++++++++++++++++ crates/horizon-client/src/game/remote.rs | 11 +++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/crates/horizon-client/src/bundle.rs b/crates/horizon-client/src/bundle.rs index 42473729f..6842d2584 100644 --- a/crates/horizon-client/src/bundle.rs +++ b/crates/horizon-client/src/bundle.rs @@ -49,10 +49,12 @@ impl Weight { /// The sounds of `BundleItem` (`Entity.playSound` at the player): insert /// and remove one at volume 0.8 and pitch `0.8 + random * 0.4`, a failed -/// insert at 1, 1. +/// insert at 1, 1; the drop of its contents is played at the player's block +/// like remove one. pub const INSERT: &str = "minecraft:item.bundle.insert"; pub const INSERT_FAIL: &str = "minecraft:item.bundle.insert_fail"; pub const REMOVE_ONE: &str = "minecraft:item.bundle.remove_one"; +pub const DROP_CONTENTS: &str = "minecraft:item.bundle.drop_contents"; fn max_stack(items: &Items, stack: &ItemStack) -> u32 { items.get(&stack.item).map_or(64, |i| i.max_stack).max(1) diff --git a/crates/horizon-client/src/game.rs b/crates/horizon-client/src/game.rs index cca1f1ee6..5e3b6864c 100644 --- a/crates/horizon-client/src/game.rs +++ b/crates/horizon-client/src/game.rs @@ -3741,6 +3741,14 @@ impl Game<'_> { self.use_spyglass(); return Ok(true); } + // BundleItem.use: startUsingItem, then onUseTick empties it. + if !on_block && info.bundle { + if self.using.is_none() { + self.using = Some((self.held_slot(), 0)); + } + self.swing_arm(); + return Ok(true); + } if !on_block && &*stack.item == "minecraft:bow" { if self.using.is_none() && (self.creative || self.projectile_slot("minecraft:bow").is_some()) @@ -4010,6 +4018,10 @@ impl Game<'_> { } return; } + if info.bundle { + self.tick_bundle(slot, ticks); + return; + } let Some(consumable) = info.consumable.as_ref() else { self.using = None; return; @@ -4031,6 +4043,53 @@ impl Game<'_> { self.using = Some((slot, ticks)); } } + /// `BundleItem.onUseTick` and `getUseDuration` (200): the first tick and + /// then every second tick from the 11th on drop one stack out of the + /// bundle in `slot`; the use ends after 200 ticks. + fn tick_bundle(&mut self, slot: usize, ticks: u32) { + const USE_DURATION: u32 = 200; + let remaining = USE_DURATION - ticks.min(USE_DURATION); + if ticks == 0 || (remaining < USE_DURATION - 10 && remaining.is_multiple_of(2)) { + self.drop_bundle_content(slot); + } + self.using = (ticks + 1 < USE_DURATION).then_some((slot, ticks + 1)); + } + /// `BundleItem.dropContent`: `removeOneItemFromBundle` and `Player.drop`, + /// then the drop sound and the used statistic. + fn drop_bundle_content(&mut self, slot: usize) { + let items = self.res.items(); + let Some(mut bundle) = self.stack_in(slot).cloned() else { + return; + }; + let Some(mut contents) = horizon_client::bundle::Contents::of(items, &bundle) else { + return; + }; + let Some(removed) = contents.remove_one(items) else { + return; + }; + contents.store(&mut bundle); + let item = Arc::clone(&bundle.item); + *self.stack_in_mut(slot) = Some(bundle); + let Some(position) = self.player.as_ref().map(|p| p.position) else { + return; + }; + let at = [position.x, position.y, position.z]; + // playRemoveOneSound, then Player.drop and playDropContentsSound at + // the player's block. + let pitch = 0.8 + self.world.random.next_float() * 0.4; + self.audio + .play(horizon_client::bundle::REMOVE_ONE, Some(at), 0.8, pitch); + self.player_drop(removed); + let block = BlockPos::containing(position); + let pitch = 0.8 + self.world.random.next_float() * 0.4; + self.audio.play( + horizon_client::bundle::DROP_CONTENTS, + Some([block.x, block.y, block.z].map(|v| f64::from(v) + 0.5)), + 0.8, + pitch, + ); + self.stats.add("used", &item, 1); + } /// `LivingEntity.releaseUsingItem` (`Item.releaseUsing`) for `stack`, /// used for `ticks`, its hand acting: a bow or a trident lets go (not /// when its hand changed), a spyglass stops scoping. diff --git a/crates/horizon-client/src/game/remote.rs b/crates/horizon-client/src/game/remote.rs index 3cfbd15e3..54c4aa5df 100644 --- a/crates/horizon-client/src/game/remote.rs +++ b/crates/horizon-client/src/game/remote.rs @@ -4318,7 +4318,10 @@ impl Game<'_> { if self.using.is_some() { return; } + let bundle = self.res.items().get(&held.item).is_some_and(|i| i.bundle); let start = match &*held.item { + // BundleItem.use: always starts; the server drops the contents. + _ if bundle => true, // BowItem.use: with an arrow, or in creative. "minecraft:bow" => self.creative || self.projectile_slot("minecraft:bow").is_some(), // CrossbowItem.use: charging when not charged and with ammunition; @@ -4383,6 +4386,14 @@ impl Game<'_> { // Another hotbar slot picked: LivingEntity.updatingUsingItem sees a // different stack in the hand. The off hand keeps its item. let in_hand = slot == self.selected || slot == super::shield::OFFHAND_SLOT; + // BundleItem.getUseDuration: 200 ticks, the server drops the contents. + if stack + .as_ref() + .is_some_and(|s| self.res.items().get(&s.item).is_some_and(|i| i.bundle)) + { + self.using = (in_hand && ticks + 1 < 200).then_some((slot, ticks + 1)); + return; + } // A drawn bow, a charging crossbow, a raised shield...: held until // the button lets go. if stack.as_ref().is_some_and(|s| self.held_until_released(s)) { From 6145077d7e094b19a0940fbf94bd13de10a03a37 Mon Sep 17 00:00:00 2001 From: Kacpherek Date: Mon, 26 Oct 2026 21:44:11 +0100 Subject: [PATCH 2/8] Make a tame parrot answer only to its owner --- crates/horizon-client/src/game/creatures.rs | 10 ++++++++-- crates/horizon-client/src/game/lan_mobs.rs | 15 ++++++++++++++- crates/horizon-client/src/mobs/tame.rs | 6 +++++- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/crates/horizon-client/src/game/creatures.rs b/crates/horizon-client/src/game/creatures.rs index 8eb4cc911..1b9a31679 100644 --- a/crates/horizon-client/src/game/creatures.rs +++ b/crates/horizon-client/src/game/creatures.rs @@ -2565,6 +2565,7 @@ impl Game<'_> { .item_tags .get("minecraft:parrot_poisonous_food") .is_some_and(|t| t.contains(&*item)); + let acting = self.acting_uuid(); let m = &mut self.mobs[index]; if !m.alive() { return Ok(false); @@ -2772,7 +2773,12 @@ impl Game<'_> { } // Parrot.mobInteract: the owner makes a tame parrot on the ground // sit or stand (poisonous food is handled apart). - Species::Parrot if m.extra.tame && m.on_ground && !parrot_poisonous => { + Species::Parrot + if m.extra.tame + && m.extra.owner == Some(acting) + && m.on_ground + && !parrot_poisonous => + { m.extra.sitting = !m.extra.sitting; self.world.modified.insert(( m.position.x.floor() as i32 >> 4, @@ -2841,7 +2847,7 @@ impl Game<'_> { let tamed = m.random.next_int_bound(10).unwrap_or(1) == 0; if tamed { m.extra.tame = true; - m.extra.owner = self.level.player_uuid.or(Some([0, 0, 0, 1])); + m.extra.owner = Some(acting); } let kind = if tamed { "heart" } else { "smoke" }; // TamableAnimal.spawnTamingParticles (entity event 7 or 6). diff --git a/crates/horizon-client/src/game/lan_mobs.rs b/crates/horizon-client/src/game/lan_mobs.rs index 4f53cec60..39bbd9794 100644 --- a/crates/horizon-client/src/game/lan_mobs.rs +++ b/crates/horizon-client/src/game/lan_mobs.rs @@ -5,7 +5,7 @@ //! (hits, pushes, fire, its projectiles) runs with that guest swapped in. use super::{Game, lan::LanHost}; use horizon_client::{ - mobs::{Mob, MobEvent, PlayerView, Who}, + mobs::{Mob, MobEvent, PlayerView, Species, Who}, projectiles::{Hit, Projectile}, }; use horizon_core::geometry::{Aabb, Vec3}; @@ -26,6 +26,19 @@ pub(super) fn player_for( guests: &GuestViews, bound: Option, ) -> Option { + // A tame parrot sees its owner (`TamableAnimal.getOwner`), while they + // are in play, whoever is nearer. + if m.species() == Species::Parrot + && m.extra.tame + && let Some(owner) = m.extra.owner + { + if host.is_some_and(|v| v.alive && v.uuid == owner) { + return None; + } + if let Some((conn, _)) = guests.iter().find(|(_, v)| v.alive && v.uuid == owner) { + return Some(*conn); + } + } let targeting = m.ai.target == Some(Who::Player) || m.ai.last_hurt_by == Some(Who::Player) && m.age as i32 - m.ai.last_hurt_by_time <= 100; choose(m.position, targeting, host, guests, bound) diff --git a/crates/horizon-client/src/mobs/tame.rs b/crates/horizon-client/src/mobs/tame.rs index 0043ff9ef..d757db0fe 100644 --- a/crates/horizon-client/src/mobs/tame.rs +++ b/crates/horizon-client/src/mobs/tame.rs @@ -135,12 +135,16 @@ pub fn set_tame(m: &mut Mob, tame: bool) { m.attrs.max_health = 8.0; } } -/// The owner (the local player) when tame. +/// The owner (the local player) when tame. A parrot's owner is the player +/// its saved UUID names (`TamableAnimal.getOwner`), not whoever is nearest. fn owner(m: &Mob, w: &MobWorld) -> Option<(Vec3, f64, bool)> { if !m.extra.tame { return None; } let p = w.player.as_ref()?; + if m.species() == Species::Parrot && m.extra.owner != Some(p.uuid) { + return None; + } Some((p.position, p.eye[1], p.spectator)) } /// `TamableAnimal.unableToMoveToOwner`. From 09142298005c5fe45b1d4883dc5809965d3446eb Mon Sep 17 00:00:00 2001 From: Ludovic Marie <136255825+ludovic111@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:07:40 +0100 Subject: [PATCH 3/8] A parrot saves its owner in main's Variant arm A second parrot arm was unreachable, so the variant was never saved. Co-Authored-By: Claude Opus 5.5 --- crates/horizon-client/src/entity_nbt.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/horizon-client/src/entity_nbt.rs b/crates/horizon-client/src/entity_nbt.rs index f7cc059aa..72b2136fc 100644 --- a/crates/horizon-client/src/entity_nbt.rs +++ b/crates/horizon-client/src/entity_nbt.rs @@ -330,14 +330,6 @@ pub fn mob_to_nbt(m: &Mob, uuid: [i32; 4], raw: Option<&Tag>) -> Tag { map.insert("Sitting".into(), Tag::Byte(i8::from(m.extra.sitting))); map.insert("CollarColor".into(), Tag::Byte(m.extra.color as i8)); } - "minecraft:parrot" => { - if let Some(owner) = m.extra.owner { - map.insert("Owner".into(), Tag::IntArray(owner.to_vec())); - } else { - map.remove(&"Owner".into()); - } - map.insert("Sitting".into(), Tag::Byte(i8::from(m.extra.sitting))); - } "minecraft:wolf" => { if let Some(owner) = m.extra.owner { map.insert("Owner".into(), Tag::IntArray(owner.to_vec())); @@ -511,6 +503,12 @@ pub fn mob_to_nbt(m: &Mob, uuid: [i32; 4], raw: Option<&Tag>) -> Tag { "Variant".into(), Tag::Int(i32::from(m.extra.variant.min(4))), ); + if let Some(owner) = m.extra.owner { + map.insert("Owner".into(), Tag::IntArray(owner.to_vec())); + } else { + map.remove(&"Owner".into()); + } + map.insert("Sitting".into(), Tag::Byte(i8::from(m.extra.sitting))); } "minecraft:salmon" => { let size = SALMON_SIZES[usize::from(m.extra.variant).min(2)]; From 21ac444f28e2dbc08936d3d0f7bfc75a274feba5 Mon Sep 17 00:00:00 2001 From: Ludovic Marie <136255825+ludovic111@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:16:27 +0100 Subject: [PATCH 4/8] refactor: block entity data moves down beside the chunk Step A of the horizon-client crate split. `Chunk.block_entities` held `containers::BlockEntity`, whose payloads were defined across containers, brewing, signs, sculk, mobs::spawner, mobs::trial_spawner, vault, beehive and command_block, so `chunk` reached most of the gameplay code. The enum and every payload type now live in `chunk/block_entity.rs` as plain data (definitions and derives). Every impl block stays in its feature module, and each module re-exports its types under the old path, so callers don't change. Also: - `mobs::Who` moves to `horizon_client_core::who` (the vibration `Info` names it); mobs re-exports it. - `skins::NameOrId` moves beside `SkullSkin`, which holds it; skins re-exports it. - `Spawner.next`, the `Next` enum and `Selector.current` become `pub(crate)`, since their struct now lives in another module. - The vault, spawner and trial_spawner harnesses record `chunk/block_entity.rs`. No behaviour change. Co-Authored-By: Claude Opus 5.5 --- crates/horizon-client-core/src/lib.rs | 1 + crates/horizon-client-core/src/who.rs | 8 + crates/horizon-client/src/beehive.rs | 15 +- crates/horizon-client/src/block_entities.rs | 2 +- crates/horizon-client/src/brewing.rs | 15 +- crates/horizon-client/src/chunk.rs | 4 +- .../horizon-client/src/chunk/block_entity.rs | 409 ++++++++++++++++++ crates/horizon-client/src/command_block.rs | 17 +- crates/horizon-client/src/containers.rs | 174 +------- crates/horizon-client/src/mobs.rs | 7 +- crates/horizon-client/src/mobs/spawner.rs | 44 +- .../horizon-client/src/mobs/trial_spawner.rs | 48 +- crates/horizon-client/src/sculk.rs | 43 +- crates/horizon-client/src/signs.rs | 17 +- crates/horizon-client/src/skins.rs | 8 +- crates/horizon-client/src/vault.rs | 34 +- tools/test_spawner_parity.py | 1 + tools/test_trial_spawner_parity.py | 1 + tools/test_vault_parity.py | 3 +- 19 files changed, 442 insertions(+), 409 deletions(-) create mode 100644 crates/horizon-client-core/src/who.rs create mode 100644 crates/horizon-client/src/chunk/block_entity.rs diff --git a/crates/horizon-client-core/src/lib.rs b/crates/horizon-client-core/src/lib.rs index d839feddf..4c4231359 100644 --- a/crates/horizon-client-core/src/lib.rs +++ b/crates/horizon-client-core/src/lib.rs @@ -24,4 +24,5 @@ pub mod snbt; pub mod sound_source; pub mod text; pub mod uuid; +pub mod who; pub type Result = std::result::Result>; diff --git a/crates/horizon-client-core/src/who.rs b/crates/horizon-client-core/src/who.rs new file mode 100644 index 000000000..63101e879 --- /dev/null +++ b/crates/horizon-client-core/src/who.rs @@ -0,0 +1,8 @@ +//! Who an action came from or is aimed at: the player or a mob by id. + +/// Who caused damage or is being targeted. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum Who { + Player, + Mob(u32), +} diff --git a/crates/horizon-client/src/beehive.rs b/crates/horizon-client/src/beehive.rs index 26aa87cfc..d0fa10c18 100644 --- a/crates/horizon-client/src/beehive.rs +++ b/crates/horizon-client/src/beehive.rs @@ -3,6 +3,7 @@ //! the rain, nor through a blocked front, unless the hive is emptied in an //! emergency), honey for every bee coming home with nectar. The host (the game //! or the mob AI probe) loads and adds the bees through [`HiveHost`]. +pub use crate::chunk::block_entity::{Beehive, Occupant}; use crate::mobs::{Mob, Species, bee}; use horizon_core::random::{LegacyRandom, RandomSource}; use horizon_world::nbt::{Compound, Tag}; @@ -38,20 +39,6 @@ const IGNORED_BEE_TAGS: [&str; 25] = [ "UUID", ]; -/// `BeehiveBlockEntity`: the bees inside and the flower one of them knew. -#[derive(Clone, Debug, Default, PartialEq)] -pub struct Beehive { - pub bees: Vec, - pub flower: Option<[i32; 3]>, -} -/// `BeehiveBlockEntity.Occupant`: a bee's tag (with its `id`), its time in -/// the hive and the time it stays at least. -#[derive(Clone, Debug, PartialEq)] -pub struct Occupant { - pub entity_data: Tag, - pub ticks_in_hive: i32, - pub min_ticks_in_hive: i32, -} /// `BeehiveBlockEntity.BeeReleaseStatus`. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Release { diff --git a/crates/horizon-client/src/block_entities.rs b/crates/horizon-client/src/block_entities.rs index 03fff85ca..944579b83 100644 --- a/crates/horizon-client/src/block_entities.rs +++ b/crates/horizon-client/src/block_entities.rs @@ -23,7 +23,7 @@ //! Which states tick is a table by state id, built by the game from the block //! registry; whether a slot is made is decided when the slot is promoted at //! the start of the next tick, from the block state then. -use crate::containers::BlockEntity; +use crate::chunk::block_entity::BlockEntity; use crate::fast_hash::{FxHashMap, FxHashSet}; use std::collections::HashMap; diff --git a/crates/horizon-client/src/brewing.rs b/crates/horizon-client/src/brewing.rs index b3cda5f90..e058f0fe7 100644 --- a/crates/horizon-client/src/brewing.rs +++ b/crates/horizon-client/src/brewing.rs @@ -2,12 +2,13 @@ //! in vanilla, ported as tables), data-driven `minecraft:brewing` recipes from //! `data/minecraft/recipe/brewing`, potion colours and tooltips, and //! `BrewingStandBlockEntity.serverTick`. +pub use crate::chunk::block_entity::BrewingStand; use crate::{ containers::Providers, items::{ItemStack, Items}, }; use serde_json::Value; -use std::{collections::HashSet, fs, path::Path, sync::Arc}; +use std::{collections::HashSet, fs, path::Path}; pub use horizon_client_core::potions::{ BASE_POTION_COLOR, Category, EFFECTS, EffectDef, POTIONS, PotionDef, effect, namespaced, @@ -190,18 +191,6 @@ impl Brewing { } } -/// `BrewingStandBlockEntity` state. Slots: 0-2 bottles, 3 ingredient, 4 fuel. -#[derive(Clone, Debug, Default)] -pub struct BrewingStand { - pub items: [Option; 5], - pub brew_time: i32, - pub total_brew_time: i32, - pub fuel: i32, - pub total_fuel: i32, - pub speed: f32, - pub ingredient: Option>, - pub last_bottles: Option<[bool; 3]>, -} /// What a brewing tick asks the world to do. #[derive(Clone, Debug, Default, PartialEq)] pub struct BrewTick { diff --git a/crates/horizon-client/src/chunk.rs b/crates/horizon-client/src/chunk.rs index b0dd377c7..c405f5522 100644 --- a/crates/horizon-client/src/chunk.rs +++ b/crates/horizon-client/src/chunk.rs @@ -6,6 +6,8 @@ use horizon_world::nbt::{Compound, Tag}; use horizon_world::registry::BlockStateId; use std::sync::{Arc, OnceLock}; +pub mod block_entity; + pub const MIN_Y: i32 = -64; pub const HEIGHT: i32 = 384; pub const SECTIONS: usize = (HEIGHT / 16) as usize; @@ -149,7 +151,7 @@ pub struct Chunk { /// Newly generated (not loaded from a save): eligible for generation-time spawns. pub fresh: bool, /// Block entities read from a save, moved into the world on arrival. - pub block_entities: Vec<((i32, i32, i32), crate::containers::BlockEntity)>, + pub block_entities: Vec<((i32, i32, i32), block_entity::BlockEntity)>, /// `CustomName` of the container block entities (shown as the screen title). pub block_names: Vec<((i32, i32, i32), String)>, /// Generation postprocessing marks (ProtoChunk.postProcessing) as world diff --git a/crates/horizon-client/src/chunk/block_entity.rs b/crates/horizon-client/src/chunk/block_entity.rs new file mode 100644 index 000000000..0547228d8 --- /dev/null +++ b/crates/horizon-client/src/chunk/block_entity.rs @@ -0,0 +1,409 @@ +//! Block entity data: what a chunk stores for its block entities (`BlockEntity` +//! and its payloads). Plain data, as vanilla's block entities are world data; +//! their behaviour lives with the features that use them (`containers`, +//! `brewing`, `signs`, ...), which re-export these types under their old paths. +use horizon_client_core::{text::Text, who::Who}; +use horizon_items::{banner_layer::BannerLayer, items::ItemStack}; +use horizon_world::{ + level::sculk::SculkSpreader, + nbt::{Compound, Tag}, +}; +use std::{collections::HashMap, sync::Arc}; + +/// An entity's UUID as its four ints (`UUIDUtil.CODEC`). +pub type Uuid = [i32; 4]; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum FurnaceKind { + Furnace, + BlastFurnace, + Smoker, +} +#[derive(Clone, Debug)] +pub struct Furnace { + pub kind: FurnaceKind, + /// input, fuel, result + pub items: [Option; 3], + pub lit_remaining: i32, + pub lit_total: i32, + pub cooking_timer: i32, + pub cooking_total: i32, + pub speed: f32, + /// `AbstractFurnaceBlockEntity.recipesUsed`: how many of each recipe this furnace has + /// made since a player last took from its output. The experience is owed per recipe + /// rather than per item, and a save keeps it as the `RecipesUsed` compound. + pub recipes_used: HashMap, +} +#[derive(Clone, Debug)] +pub enum BlockEntity { + Chest(Vec>), + Furnace(Furnace), + Brewing(Box), + /// Signs and hanging signs (`SignBlockEntity`). + Sign(SignData), + /// `JukeboxBlockEntity`: the record and its `JukeboxSongPlayer`. + Jukebox(Jukebox), + /// `BeaconBlockEntity`. + Beacon(Box), + /// `LecternBlockEntity`: the book and the open page. + Lectern(Lectern), + /// Sculk sensors, shriekers and catalysts (`crate::sculk`). + Sculk(Box), + /// `CreakingHeartBlockEntity`: its protector and the hurt emitter. + CreakingHeart(Box), + /// `BannerBlockEntity`: the pattern layers. + Banner(Banner), + /// `CrafterBlockEntity`. + Crafter(Crafter), + /// `SpawnerBlockEntity` (`crate::mobs::spawner`). + Spawner(Box), + /// `TrialSpawnerBlockEntity` (`crate::mobs::trial_spawner`). + TrialSpawner(Box), + /// `VaultBlockEntity` (`crate::vault`). + Vault(Box), + /// `BeehiveBlockEntity` of beehives and bee nests (`crate::beehive`). + Beehive(Box), + /// `CommandBlockEntity` of the three command blocks (`crate::command_block`). + CommandBlock(Box), + /// `CampfireBlockEntity` of campfires and soul campfires. + Campfire(Box), + /// `SkullBlockEntity` of every skull and head block. + Skull(Skull), + /// `TheEndGatewayBlockEntity`: its age and configured destination. + EndGateway(EndGateway), + /// `DaylightDetectorBlockEntity`: no data, only the ticker + /// (`DaylightDetectorBlock.tickEntity`). + DaylightDetector, + /// `ChiseledBookShelfBlockEntity`: a `Container` of six one-book slots. + ChiseledBookshelf(Box), +} +/// `ChiseledBookShelfBlockEntity`: six slots and `lastInteractedSlot`. +#[derive(Clone, Debug, PartialEq)] +pub struct ChiseledBookshelf { + pub items: [Option; 6], + pub last_slot: i32, +} +impl Default for ChiseledBookshelf { + fn default() -> Self { + Self { + items: Default::default(), + last_slot: -1, + } + } +} +/// `TheEndGatewayBlockEntity`. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct EndGateway { + pub age: i64, + pub exit_portal: Option<[i32; 3]>, + pub exact_teleport: bool, +} +/// `CampfireBlockEntity`: four cooking slots, each with its +/// `cookingProgress` and `cookingTime` (the recipe's). +#[derive(Clone, Debug, Default, PartialEq)] +pub struct Campfire { + pub items: [Option; 4], + pub progress: [i32; 4], + pub total: [i32; 4], +} +/// `SkullBlockEntity`: the player profile the head shows (kept as loaded: +/// a compound, or a string when it names a player only), the note block +/// sound and `custom_name`. +#[derive(Clone, Debug, Default)] +pub struct Skull { + pub profile: Option, + pub note_block_sound: Option, + pub custom_name: Option, + /// What `profile` draws, read from it once rather than every frame. + pub skin: Option, +} +/// `SkullBlockRenderer.getPlayerSkin` of a head's profile: its skin's URL +/// with the `skins/` sprite it downloads to, and the +/// `DefaultPlayerSkin` drawn until that sprite is there (steve for a +/// profile with no id). +#[derive(Clone, Debug)] +pub struct SkullSkin { + pub download: Option<(String, String)>, + /// `ResolvableProfile.nameOrId` when the profile is `Dynamic` (a name or + /// an id only): the key `skins::Resolver` resolves it by. + pub dynamic: Option, + pub fallback: &'static str, +} +/// `Either`: the key a `Dynamic` `ResolvableProfile` resolves +/// by (`ResolvableProfile.nameOrId`). +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum NameOrId { + Name(String), + Id(u128), +} +/// `CrafterBlockEntity`: nine slots that can be disabled, the powered flag +/// and the ticks left in the crafting animation. +#[derive(Clone, Debug, Default)] +pub struct Crafter { + pub items: [Option; 9], + pub disabled: [bool; 9], + pub triggered: bool, + pub crafting_ticks: i32, + /// The block state's `triggered` when last ticked (not saved): the block + /// entity flag follows it only when it changes (`CrafterBlock.neighborChanged`). + pub state_triggered: Option, +} +#[derive(Clone, Debug, Default)] +pub struct Banner { + pub layers: Vec, + pub name: Option, + /// Components not consumed by BannerBlockEntity's implicit fields. + pub components: horizon_world::nbt::Compound, +} +/// `CreakingHeartBlockEntity` (the logic is in `game/creaking.rs`). +#[derive(Clone, Debug, Default)] +pub struct CreakingHeart { + /// The protector: its entity id once resolved, and its UUID (the saved form). + pub creaking: Option<(Option, [i32; 4])>, + pub ticks_existed: i64, + pub ticker: i32, + /// Ticks left of the hurt effect (`emitter`) and where it heads. + pub emitter: i32, + pub emitter_target: Option<[f64; 3]>, + pub output_signal: i32, +} +#[derive(Clone, Debug, Default)] +pub struct Lectern { + pub book: Option, + pub page: i32, +} +#[derive(Clone, Debug, Default)] +pub struct Beacon { + pub levels: i32, + /// Effect ids (`minecraft:speed`). + pub primary: Option, + pub secondary: Option, + /// Beam sections (RGB colour, height) and the ones being rebuilt. + pub sections: Vec<(u32, i32)>, + pub checking: Vec<(u32, i32)>, + pub last_check_y: i32, +} +#[derive(Clone, Debug, Default)] +pub struct Jukebox { + pub record: Option, + /// `ticks_since_song_started`, while a song plays. + pub ticks: i64, + pub playing: bool, +} +/// `BrewingStandBlockEntity` state. Slots: 0-2 bottles, 3 ingredient, 4 fuel. +#[derive(Clone, Debug, Default)] +pub struct BrewingStand { + pub items: [Option; 5], + pub brew_time: i32, + pub total_brew_time: i32, + pub fuel: i32, + pub total_fuel: i32, + pub speed: f32, + pub ingredient: Option>, + pub last_bottles: Option<[bool; 3]>, +} +/// One sign side (`SignText`). +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SignText { + pub lines: [String; 4], + /// `DyeColor` id. + pub color: u8, + pub glowing: bool, +} +/// `SignBlockEntity` / `HangingSignBlockEntity` data. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SignData { + pub hanging: bool, + pub front: SignText, + pub back: SignText, + pub waxed: bool, +} +/// `VibrationInfo`. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Info { + pub event: &'static str, + pub distance: f32, + pub pos: [f64; 3], + pub source: Option, + pub owner: Option, +} +/// `VibrationSelector`: the closest, then highest-frequency candidate of a tick. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct Selector { + pub(crate) current: Option<(Info, i64)>, +} +/// `VibrationSystem.Data`. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct Data { + pub current: Option, + pub travel: i32, + pub selector: Selector, +} +/// Which sculk block entity this is. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SculkKind { + Sensor, + Calibrated, + Shrieker, + Catalyst, +} +/// `SculkSensorBlockEntity`, `CalibratedSculkSensorBlockEntity`, +/// `SculkShriekerBlockEntity` and `SculkCatalystBlockEntity`. +#[derive(Clone, Debug)] +pub struct SculkEntity { + pub kind: SculkKind, + pub vibration: Data, + /// `last_vibration_frequency` (sensors). + pub last_frequency: i32, + /// `warning_level` (shriekers). + pub warning_level: i32, + /// The catalyst's charge cursors. + pub spreader: SculkSpreader, +} +/// `SpawnData.CustomSpawnRules`: the block and sky light the spawn block +/// must have (inclusive ranges within 0..=15). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct CustomSpawnRules { + pub block: (i32, i32), + pub sky: (i32, i32), +} +/// `SpawnData`: the entity to spawn, optional light rules and an equipment +/// table (kept as loaded). +#[derive(Clone, Debug, Default, PartialEq)] +pub struct SpawnData { + pub entity: Compound, + pub custom_spawn_rules: Option, + pub equipment: Option, +} +/// Which `SpawnData` is next: one of the potentials (the same object in +/// vanilla, so changing it changes the potential) or its own. +#[derive(Clone, Debug, PartialEq)] +pub(crate) enum Next { + Potential(usize), + Own(SpawnData), +} +/// `BaseSpawner`. +#[derive(Clone, Debug, PartialEq)] +pub struct Spawner { + pub spawn_delay: i32, + /// `SpawnPotentials`: data and weight. + pub potentials: Vec<(SpawnData, i32)>, + pub(crate) next: Option, + pub min_spawn_delay: i32, + pub max_spawn_delay: i32, + pub spawn_count: i32, + pub max_nearby_entities: i32, + pub required_player_range: i32, + pub spawn_range: i32, + /// The client copy's delay (`spawnDelay` on the client level), which + /// speeds the spin up as the next spawn nears. + pub client_delay: i32, + pub spin: f64, + pub o_spin: f64, +} +/// `TrialSpawnerConfig`. +#[derive(Clone, Debug, PartialEq)] +pub struct TrialConfig { + pub spawn_range: i32, + pub total_mobs: f32, + pub simultaneous_mobs: f32, + pub total_mobs_added_per_player: f32, + pub simultaneous_mobs_added_per_player: f32, + pub ticks_between_spawn: i32, + pub spawn_potentials: Vec<(SpawnData, i32)>, + pub loot_tables_to_eject: Vec<(String, i32)>, + pub items_to_drop_when_ominous: String, +} +/// A `Holder` as saved: a registry id +/// (`minecraft:trial_chamber/breeze/normal`) or an inline config. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct ConfigSlot { + /// The value as loaded, written back as is; None for the default. + pub saved: Option, + pub config: TrialConfig, +} +/// `TrialSpawner` with its `TrialSpawnerStateData`. +#[derive(Clone, Debug, PartialEq)] +pub struct TrialSpawner { + pub normal: ConfigSlot, + pub ominous: ConfigSlot, + pub target_cooldown_length: i32, + pub required_player_range: i32, + pub detected_players: Vec, + pub current_mobs: Vec, + pub cooldown_ends_at: i64, + pub next_mob_spawns_at: i64, + pub total_mobs_spawned: i32, + pub next_spawn_data: Option, + pub ejecting_loot_table: Option, + /// `isOminous`, set from the block on every server tick. + pub is_ominous: bool, + /// The server's display entity exists (`getOrCreateDisplayEntity`): it is + /// made once per loaded block entity and kept. + pub has_display: bool, + pub spin: f64, + pub o_spin: f64, + /// `getDispensingItems`, kept once rolled (not saved): the ominous items + /// and their weights (the rolled counts). + pub dispensing: Option>, +} +/// `VaultConfig`. +#[derive(Clone, Debug, PartialEq)] +pub struct VaultConfig { + pub loot_table: String, + pub activation_range: f64, + pub deactivation_range: f64, + /// None when the config has no key (`ItemStack.EMPTY`): the vault never opens. + pub key_item: Option, + pub override_loot_table_to_display: Option, +} +/// `VaultBlockEntity`: its config, server data and shared data. +#[derive(Clone, Debug, PartialEq)] +pub struct Vault { + pub config: VaultConfig, + /// `VaultServerData`, oldest rewarded player first. + pub rewarded_players: Vec, + pub state_updating_resumes_at: i64, + /// Popped from the end. + pub items_to_eject: Vec, + pub total_ejections_needed: i32, + pub last_insert_fail_timestamp: i64, + /// `VaultSharedData`. + pub display_item: Option, + pub connected_players: Vec, + pub connected_particles_range: f64, + /// The shared data changed since the last tick sent it. + pub shared_dirty: bool, + /// `VaultClientData`: the display item's spin and the previous tick's, in degrees. + pub spin: f32, + pub o_spin: f32, +} +/// `BeehiveBlockEntity`: the bees inside and the flower one of them knew. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct Beehive { + pub bees: Vec, + pub flower: Option<[i32; 3]>, +} +/// `BeehiveBlockEntity.Occupant`: a bee's tag (with its `id`), its time in +/// the hive and the time it stays at least. +#[derive(Clone, Debug, PartialEq)] +pub struct Occupant { + pub entity_data: Tag, + pub ticks_in_hive: i32, + pub min_ticks_in_hive: i32, +} +#[derive(Clone, Debug, PartialEq)] +pub struct CommandBlock { + pub command: String, + pub success_count: i32, + /// `CustomName` as saved (a text component). + pub custom_name: Option, + pub track_output: bool, + pub last_output: Option, + pub update_last_execution: bool, + /// The game time of the last run, -1 for none. + pub last_execution: i64, + pub powered: bool, + pub auto: bool, + pub condition_met: bool, +} diff --git a/crates/horizon-client/src/command_block.rs b/crates/horizon-client/src/command_block.rs index 47f808932..1d3fe16ec 100644 --- a/crates/horizon-client/src/command_block.rs +++ b/crates/horizon-client/src/command_block.rs @@ -1,6 +1,7 @@ //! `CommandBlockEntity` with its `BaseCommandBlock`: the command, the success //! count comparators read, the output it keeps, and the powered, automatic //! and condition flags `CommandBlock.tick` runs on. +pub use crate::chunk::block_entity::CommandBlock; use crate::{ save::{number, text}, text::Text, @@ -76,22 +77,6 @@ pub fn automatic_block(name: &str) -> bool { name == "minecraft:chain_command_block" } -#[derive(Clone, Debug, PartialEq)] -pub struct CommandBlock { - pub command: String, - pub success_count: i32, - /// `CustomName` as saved (a text component). - pub custom_name: Option, - pub track_output: bool, - pub last_output: Option, - pub update_last_execution: bool, - /// The game time of the last run, -1 for none. - pub last_execution: i64, - pub powered: bool, - pub auto: bool, - pub condition_met: bool, -} - impl Default for CommandBlock { fn default() -> Self { Self { diff --git a/crates/horizon-client/src/containers.rs b/crates/horizon-client/src/containers.rs index af867903f..cd232ca94 100644 --- a/crates/horizon-client/src/containers.rs +++ b/crates/horizon-client/src/containers.rs @@ -1,6 +1,10 @@ //! Container block entities: chests/barrels (27 slots) and the three furnace //! kinds, ticking as `AbstractFurnaceBlockEntity.serverTick` with fuel times //! from the data-driven `context_int_provider`/`context_float_provider` files. +pub use crate::chunk::block_entity::{ + Banner, Beacon, BlockEntity, Campfire, ChiseledBookshelf, Crafter, CreakingHeart, EndGateway, + Furnace, FurnaceKind, Jukebox, Lectern, Skull, SkullSkin, +}; use crate::{ crafting::Recipes, items::{ItemStack, Items}, @@ -8,12 +12,6 @@ use crate::{ use serde_json::Value; use std::{collections::HashMap, fs, path::Path, sync::Arc}; -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum FurnaceKind { - Furnace, - BlastFurnace, - Smoker, -} impl FurnaceKind { pub fn recipe_type(self) -> &'static str { match self { @@ -34,21 +32,6 @@ impl FurnaceKind { } } } -#[derive(Clone, Debug)] -pub struct Furnace { - pub kind: FurnaceKind, - /// input, fuel, result - pub items: [Option; 3], - pub lit_remaining: i32, - pub lit_total: i32, - pub cooking_timer: i32, - pub cooking_total: i32, - pub speed: f32, - /// `AbstractFurnaceBlockEntity.recipesUsed`: how many of each recipe this furnace has - /// made since a player last took from its output. The experience is owed per recipe - /// rather than per item, and a save keeps it as the `RecipesUsed` compound. - pub recipes_used: HashMap, -} /// The key `recipes_used` keeps a recipe under: its full id, as vanilla's `ResourceKey` /// writes it in `RecipesUsed`. Horizon's own recipe ids are file names without the namespace, so /// keeping both forms would write a key twice, and vanilla refuses such a furnace @@ -63,78 +46,6 @@ pub fn recipe_used_key(id: &str) -> String { /// A hopper's slots: the only plain container with five. pub const HOPPER_SLOTS: usize = 5; -#[derive(Clone, Debug)] -pub enum BlockEntity { - Chest(Vec>), - Furnace(Furnace), - Brewing(Box), - /// Signs and hanging signs (`SignBlockEntity`). - Sign(crate::signs::SignData), - /// `JukeboxBlockEntity`: the record and its `JukeboxSongPlayer`. - Jukebox(Jukebox), - /// `BeaconBlockEntity`. - Beacon(Box), - /// `LecternBlockEntity`: the book and the open page. - Lectern(Lectern), - /// Sculk sensors, shriekers and catalysts (`crate::sculk`). - Sculk(Box), - /// `CreakingHeartBlockEntity`: its protector and the hurt emitter. - CreakingHeart(Box), - /// `BannerBlockEntity`: the pattern layers. - Banner(Banner), - /// `CrafterBlockEntity`. - Crafter(Crafter), - /// `SpawnerBlockEntity` (`crate::mobs::spawner`). - Spawner(Box), - /// `TrialSpawnerBlockEntity` (`crate::mobs::trial_spawner`). - TrialSpawner(Box), - /// `VaultBlockEntity` (`crate::vault`). - Vault(Box), - /// `BeehiveBlockEntity` of beehives and bee nests (`crate::beehive`). - Beehive(Box), - /// `CommandBlockEntity` of the three command blocks (`crate::command_block`). - CommandBlock(Box), - /// `CampfireBlockEntity` of campfires and soul campfires. - Campfire(Box), - /// `SkullBlockEntity` of every skull and head block. - Skull(Skull), - /// `TheEndGatewayBlockEntity`: its age and configured destination. - EndGateway(EndGateway), - /// `DaylightDetectorBlockEntity`: no data, only the ticker - /// (`DaylightDetectorBlock.tickEntity`). - DaylightDetector, - /// `ChiseledBookShelfBlockEntity`: a `Container` of six one-book slots. - ChiseledBookshelf(Box), -} -/// `ChiseledBookShelfBlockEntity`: six slots and `lastInteractedSlot`. -#[derive(Clone, Debug, PartialEq)] -pub struct ChiseledBookshelf { - pub items: [Option; 6], - pub last_slot: i32, -} -impl Default for ChiseledBookshelf { - fn default() -> Self { - Self { - items: Default::default(), - last_slot: -1, - } - } -} -/// `TheEndGatewayBlockEntity`. -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct EndGateway { - pub age: i64, - pub exit_portal: Option<[i32; 3]>, - pub exact_teleport: bool, -} -/// `CampfireBlockEntity`: four cooking slots, each with its -/// `cookingProgress` and `cookingTime` (the recipe's). -#[derive(Clone, Debug, Default, PartialEq)] -pub struct Campfire { - pub items: [Option; 4], - pub progress: [i32; 4], - pub total: [i32; 4], -} impl Campfire { /// `placeFood`'s slot: the first empty one, given the item's /// `campfire_cooking` time; it takes one item of `held`. @@ -182,17 +93,6 @@ impl Campfire { changed } } -/// `SkullBlockEntity`: the player profile the head shows (kept as loaded: -/// a compound, or a string when it names a player only), the note block -/// sound and `custom_name`. -#[derive(Clone, Debug, Default)] -pub struct Skull { - pub profile: Option, - pub note_block_sound: Option, - pub custom_name: Option, - /// What `profile` draws, read from it once rather than every frame. - pub skin: Option, -} impl Skull { pub fn new( profile: Option, @@ -208,18 +108,6 @@ impl Skull { } } } -/// `SkullBlockRenderer.getPlayerSkin` of a head's profile: its skin's URL -/// with the `skins/` sprite it downloads to, and the -/// `DefaultPlayerSkin` drawn until that sprite is there (steve for a -/// profile with no id). -#[derive(Clone, Debug)] -pub struct SkullSkin { - pub download: Option<(String, String)>, - /// `ResolvableProfile.nameOrId` when the profile is `Dynamic` (a name or - /// an id only): the key `skins::Resolver` resolves it by. - pub dynamic: Option, - pub fallback: &'static str, -} impl SkullSkin { pub fn of(profile: &horizon_world::nbt::Tag) -> Self { let download = crate::skins::profile_skin_url_nbt(profile).and_then(|url| { @@ -248,18 +136,6 @@ impl SkullSkin { fn is_skull(name: &str) -> bool { name.ends_with("_skull") || (name.ends_with("_head") && name != "minecraft:piston_head") } -/// `CrafterBlockEntity`: nine slots that can be disabled, the powered flag -/// and the ticks left in the crafting animation. -#[derive(Clone, Debug, Default)] -pub struct Crafter { - pub items: [Option; 9], - pub disabled: [bool; 9], - pub triggered: bool, - pub crafting_ticks: i32, - /// The block state's `triggered` when last ticked (not saved): the block - /// entity flag follows it only when it changes (`CrafterBlock.neighborChanged`). - pub state_triggered: Option, -} impl Crafter { /// `slotCanBeDisabled`: only empty slots. pub fn slot_can_be_disabled(&self, slot: usize) -> bool { @@ -299,48 +175,6 @@ impl Crafter { }) } } -#[derive(Clone, Debug, Default)] -pub struct Banner { - pub layers: Vec, - pub name: Option, - /// Components not consumed by BannerBlockEntity's implicit fields. - pub components: horizon_world::nbt::Compound, -} -/// `CreakingHeartBlockEntity` (the logic is in `game/creaking.rs`). -#[derive(Clone, Debug, Default)] -pub struct CreakingHeart { - /// The protector: its entity id once resolved, and its UUID (the saved form). - pub creaking: Option<(Option, [i32; 4])>, - pub ticks_existed: i64, - pub ticker: i32, - /// Ticks left of the hurt effect (`emitter`) and where it heads. - pub emitter: i32, - pub emitter_target: Option<[f64; 3]>, - pub output_signal: i32, -} -#[derive(Clone, Debug, Default)] -pub struct Lectern { - pub book: Option, - pub page: i32, -} -#[derive(Clone, Debug, Default)] -pub struct Beacon { - pub levels: i32, - /// Effect ids (`minecraft:speed`). - pub primary: Option, - pub secondary: Option, - /// Beam sections (RGB colour, height) and the ones being rebuilt. - pub sections: Vec<(u32, i32)>, - pub checking: Vec<(u32, i32)>, - pub last_check_y: i32, -} -#[derive(Clone, Debug, Default)] -pub struct Jukebox { - pub record: Option, - /// `ticks_since_song_started`, while a song plays. - pub ticks: i64, - pub playing: bool, -} impl BlockEntity { pub fn for_block(name: &str) -> Option { if let Some(hanging) = crate::signs::sign_kind(name) { diff --git a/crates/horizon-client/src/mobs.rs b/crates/horizon-client/src/mobs.rs index cd8962754..f52bd8e39 100644 --- a/crates/horizon-client/src/mobs.rs +++ b/crates/horizon-client/src/mobs.rs @@ -503,12 +503,7 @@ impl MobTypes { Ok(Self { types: out }) } } -/// Who caused damage or is being targeted. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -pub enum Who { - Player, - Mob(u32), -} +pub use horizon_client_core::who::Who; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum DamageKind { PlayerAttack, diff --git a/crates/horizon-client/src/mobs/spawner.rs b/crates/horizon-client/src/mobs/spawner.rs index 78b091b7b..4479ada31 100644 --- a/crates/horizon-client/src/mobs/spawner.rs +++ b/crates/horizon-client/src/mobs/spawner.rs @@ -12,24 +12,11 @@ use super::{ spawn_rules::{self, Reason}, spawn_selection::{self, Category, NaturalLevel}, }; +use crate::chunk::block_entity::Next; +pub use crate::chunk::block_entity::{CustomSpawnRules, SpawnData, Spawner}; use horizon_core::{geometry::Aabb, geometry::Vec3, random::RandomSource}; use horizon_world::nbt::{Compound, JavaString, Tag}; -/// `SpawnData.CustomSpawnRules`: the block and sky light the spawn block -/// must have (inclusive ranges within 0..=15). -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct CustomSpawnRules { - pub block: (i32, i32), - pub sky: (i32, i32), -} -/// `SpawnData`: the entity to spawn, optional light rules and an equipment -/// table (kept as loaded). -#[derive(Clone, Debug, Default, PartialEq)] -pub struct SpawnData { - pub entity: Compound, - pub custom_spawn_rules: Option, - pub equipment: Option, -} impl SpawnData { /// The `SpawnData` constructor: an `id` that is a valid identifier is /// stored normalised (`zombie` becomes `minecraft:zombie`), any other is @@ -200,33 +187,6 @@ fn encode_range((lo, hi): (i32, i32)) -> Tag { } } -/// Which `SpawnData` is next: one of the potentials (the same object in -/// vanilla, so changing it changes the potential) or its own. -#[derive(Clone, Debug, PartialEq)] -enum Next { - Potential(usize), - Own(SpawnData), -} - -/// `BaseSpawner`. -#[derive(Clone, Debug, PartialEq)] -pub struct Spawner { - pub spawn_delay: i32, - /// `SpawnPotentials`: data and weight. - pub potentials: Vec<(SpawnData, i32)>, - next: Option, - pub min_spawn_delay: i32, - pub max_spawn_delay: i32, - pub spawn_count: i32, - pub max_nearby_entities: i32, - pub required_player_range: i32, - pub spawn_range: i32, - /// The client copy's delay (`spawnDelay` on the client level), which - /// speeds the spin up as the next spawn nears. - pub client_delay: i32, - pub spin: f64, - pub o_spin: f64, -} impl Default for Spawner { fn default() -> Self { Self { diff --git a/crates/horizon-client/src/mobs/trial_spawner.rs b/crates/horizon-client/src/mobs/trial_spawner.rs index 2314d9f45..e789f14fb 100644 --- a/crates/horizon-client/src/mobs/trial_spawner.rs +++ b/crates/horizon-client/src/mobs/trial_spawner.rs @@ -13,6 +13,7 @@ use super::{ spawn_selection, spawner::{self, SpawnData, SpawnerLevel, SpawnerSpawn}, }; +pub use crate::chunk::block_entity::{ConfigSlot, TrialConfig, TrialSpawner}; use horizon_core::{position::BlockPos, random::RandomSource}; use horizon_world::nbt::{Compound, JavaString, Tag}; use std::{collections::HashMap, path::Path, sync::OnceLock}; @@ -32,19 +33,6 @@ const TICKS_BETWEEN_ITEM_SPAWNERS: i64 = 160; /// Bad Omen. pub const TRIAL_OMEN_TICKS_PER_LEVEL: i32 = 18000; -/// `TrialSpawnerConfig`. -#[derive(Clone, Debug, PartialEq)] -pub struct TrialConfig { - pub spawn_range: i32, - pub total_mobs: f32, - pub simultaneous_mobs: f32, - pub total_mobs_added_per_player: f32, - pub simultaneous_mobs_added_per_player: f32, - pub ticks_between_spawn: i32, - pub spawn_potentials: Vec<(SpawnData, i32)>, - pub loot_tables_to_eject: Vec<(String, i32)>, - pub items_to_drop_when_ominous: String, -} impl Default for TrialConfig { /// `TrialSpawnerConfig.Builder`'s defaults. fn default() -> Self { @@ -202,15 +190,6 @@ fn identifier(t: Option<&Tag>) -> Option { } } -/// A `Holder` as saved: a registry id -/// (`minecraft:trial_chamber/breeze/normal`) or an inline config. -#[derive(Clone, Debug, Default, PartialEq)] -pub struct ConfigSlot { - /// The value as loaded, written back as is; None for the default. - pub saved: Option, - pub config: TrialConfig, -} - /// `TrialSpawnerState`. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum TrialState { @@ -335,31 +314,6 @@ pub trait TrialSpawnerLevel: SpawnerLevel { fn seed(&self) -> i64; } -/// `TrialSpawner` with its `TrialSpawnerStateData`. -#[derive(Clone, Debug, PartialEq)] -pub struct TrialSpawner { - pub normal: ConfigSlot, - pub ominous: ConfigSlot, - pub target_cooldown_length: i32, - pub required_player_range: i32, - pub detected_players: Vec, - pub current_mobs: Vec, - pub cooldown_ends_at: i64, - pub next_mob_spawns_at: i64, - pub total_mobs_spawned: i32, - pub next_spawn_data: Option, - pub ejecting_loot_table: Option, - /// `isOminous`, set from the block on every server tick. - pub is_ominous: bool, - /// The server's display entity exists (`getOrCreateDisplayEntity`): it is - /// made once per loaded block entity and kept. - pub has_display: bool, - pub spin: f64, - pub o_spin: f64, - /// `getDispensingItems`, kept once rolled (not saved): the ominous items - /// and their weights (the rolled counts). - pub dispensing: Option>, -} impl Default for TrialSpawner { fn default() -> Self { Self { diff --git a/crates/horizon-client/src/sculk.rs b/crates/horizon-client/src/sculk.rs index eefedaa98..97a2187cb 100644 --- a/crates/horizon-client/src/sculk.rs +++ b/crates/horizon-client/src/sculk.rs @@ -1,6 +1,7 @@ //! Game events and the vibration system (`GameEvent`, `VibrationSystem`, //! `VibrationSelector`) shared by sculk sensors, shriekers and wardens, the //! sculk block entities and the per-player warden spawn tracker. +pub use crate::chunk::block_entity::{Data, Info, SculkEntity, SculkKind, Selector}; use crate::mobs::Who; use horizon_core::{ geometry::{Direction, Vec3, traverse_blocks}, @@ -185,20 +186,6 @@ pub fn valid_vibration(listens: bool, event: &str, ctx: &Context) -> bool { !ctx.dampening_state } -/// `VibrationInfo`. -#[derive(Clone, Copy, Debug, PartialEq)] -pub struct Info { - pub event: &'static str, - pub distance: f32, - pub pos: [f64; 3], - pub source: Option, - pub owner: Option, -} -/// `VibrationSelector`: the closest, then highest-frequency candidate of a tick. -#[derive(Clone, Debug, Default, PartialEq)] -pub struct Selector { - current: Option<(Info, i64)>, -} impl Selector { pub fn add_candidate(&mut self, new: Info, tick: i64) { if self.should_replace(&new, tick) { @@ -233,13 +220,6 @@ impl Selector { self.current } } -/// `VibrationSystem.Data`. -#[derive(Clone, Debug, Default, PartialEq)] -pub struct Data { - pub current: Option, - pub travel: i32, - pub selector: Selector, -} /// What one `VibrationSystem.Ticker.tick` did. #[derive(Clone, Debug, Default, PartialEq)] pub struct Tick { @@ -382,27 +362,6 @@ pub fn is_occluded( true } -/// Which sculk block entity this is. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum SculkKind { - Sensor, - Calibrated, - Shrieker, - Catalyst, -} -/// `SculkSensorBlockEntity`, `CalibratedSculkSensorBlockEntity`, -/// `SculkShriekerBlockEntity` and `SculkCatalystBlockEntity`. -#[derive(Clone, Debug)] -pub struct SculkEntity { - pub kind: SculkKind, - pub vibration: Data, - /// `last_vibration_frequency` (sensors). - pub last_frequency: i32, - /// `warning_level` (shriekers). - pub warning_level: i32, - /// The catalyst's charge cursors. - pub spreader: SculkSpreader, -} impl SculkEntity { pub fn new(kind: SculkKind) -> Self { Self { diff --git a/crates/horizon-client/src/signs.rs b/crates/horizon-client/src/signs.rs index 799210797..50559e9ae 100644 --- a/crates/horizon-client/src/signs.rs +++ b/crates/horizon-client/src/signs.rs @@ -1,6 +1,7 @@ //! Sign block entities (`SignBlockEntity`, `SignText`): two four-line text //! sides with a dye colour and glow, saved in vanilla's `front_text` / //! `back_text` NBT layout. +pub use crate::chunk::block_entity::{SignData, SignText}; use horizon_world::nbt::Tag; /// `DyeColor` names and `getTextColor` values by id. @@ -24,14 +25,6 @@ pub const DYE_TEXT: [(&str, u32); 16] = [ ]; const BLACK: u8 = 15; -/// One sign side (`SignText`). -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct SignText { - pub lines: [String; 4], - /// `DyeColor` id. - pub color: u8, - pub glowing: bool, -} impl Default for SignText { fn default() -> Self { Self { @@ -61,14 +54,6 @@ impl SignText { } } -/// `SignBlockEntity` / `HangingSignBlockEntity` data. -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct SignData { - pub hanging: bool, - pub front: SignText, - pub back: SignText, - pub waxed: bool, -} impl SignData { pub fn id(&self) -> &'static str { if self.hanging { diff --git a/crates/horizon-client/src/skins.rs b/crates/horizon-client/src/skins.rs index 740208973..76bb79f6c 100644 --- a/crates/horizon-client/src/skins.rs +++ b/crates/horizon-client/src/skins.rs @@ -160,13 +160,7 @@ pub(crate) fn nbt_text(tag: Option<&horizon_world::nbt::Tag>) -> Option } } -/// `Either`: the key a `Dynamic` `ResolvableProfile` resolves -/// by (`ResolvableProfile.nameOrId`). -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub enum NameOrId { - Name(String), - Id(u128), -} +pub use crate::chunk::block_entity::NameOrId; /// `GameProfile` as `fetchProfile` returns it: the account's id, name and /// property map (its `textures` property is the skin). diff --git a/crates/horizon-client/src/vault.rs b/crates/horizon-client/src/vault.rs index 38e5d29d7..919ba8e56 100644 --- a/crates/horizon-client/src/vault.rs +++ b/crates/horizon-client/src/vault.rs @@ -7,6 +7,7 @@ //! the next one. The level's random draws the display item and throws the //! rewards; the loot tables go through the caller, so the game and a //! differential test world roll them the same way. +pub use crate::chunk::block_entity::{Vault, VaultConfig}; use crate::items::ItemStack; use horizon_core::random::RandomSource; use horizon_world::nbt::{Compound, Tag}; @@ -26,17 +27,6 @@ const UNLOCKING_DELAY: i64 = 14; const INSERT_FAIL_SOUND_BUFFER: i64 = 15; const STATE_DELAY: i64 = 20; -/// `VaultConfig`. -#[derive(Clone, Debug, PartialEq)] -pub struct VaultConfig { - pub loot_table: String, - pub activation_range: f64, - pub deactivation_range: f64, - /// None when the config has no key (`ItemStack.EMPTY`): the vault never opens. - pub key_item: Option, - pub override_loot_table_to_display: Option, -} - impl Default for VaultConfig { /// `VaultConfig.DEFAULT`: the trial key and the trial chambers' reward. fn default() -> Self { @@ -169,28 +159,6 @@ pub trait VaultLevel { /// the table has one. pub type Roll<'a, R> = dyn FnMut(&str, &mut R, bool) -> Vec + 'a; -/// `VaultBlockEntity`: its config, server data and shared data. -#[derive(Clone, Debug, PartialEq)] -pub struct Vault { - pub config: VaultConfig, - /// `VaultServerData`, oldest rewarded player first. - pub rewarded_players: Vec, - pub state_updating_resumes_at: i64, - /// Popped from the end. - pub items_to_eject: Vec, - pub total_ejections_needed: i32, - pub last_insert_fail_timestamp: i64, - /// `VaultSharedData`. - pub display_item: Option, - pub connected_players: Vec, - pub connected_particles_range: f64, - /// The shared data changed since the last tick sent it. - pub shared_dirty: bool, - /// `VaultClientData`: the display item's spin and the previous tick's, in degrees. - pub spin: f32, - pub o_spin: f32, -} - impl Default for Vault { fn default() -> Self { Self { diff --git a/tools/test_spawner_parity.py b/tools/test_spawner_parity.py index 34f6ec726..f94324cc9 100644 --- a/tools/test_spawner_parity.py +++ b/tools/test_spawner_parity.py @@ -232,6 +232,7 @@ def main(): oracle = lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/VanillaNaturalSpawnOracle.java' sources = [oracle, Path(__file__), lab.ROOT / 'crates/horizon-client/examples/probe/natural_spawn_fixture.rs', lab.ROOT / 'crates/horizon-client/src/mobs/spawner.rs', + lab.ROOT / 'crates/horizon-client/src/chunk/block_entity.rs', lab.ROOT / 'crates/horizon-client/src/mobs/spawn_selection.rs', lab.ROOT / 'crates/horizon-client/src/mobs/spawn_rules.rs'] tested = base.source_hashes(sources) diff --git a/tools/test_trial_spawner_parity.py b/tools/test_trial_spawner_parity.py index 43d26338a..6f588b785 100644 --- a/tools/test_trial_spawner_parity.py +++ b/tools/test_trial_spawner_parity.py @@ -197,6 +197,7 @@ def main(): sources = [oracle, Path(__file__), lab.ROOT / 'crates/horizon-client/examples/probe/natural_spawn_fixture.rs', lab.ROOT / 'crates/horizon-client/src/mobs/trial_spawner.rs', lab.ROOT / 'crates/horizon-client/src/mobs/spawner.rs', + lab.ROOT / 'crates/horizon-client/src/chunk/block_entity.rs', lab.ROOT / 'crates/horizon-client/src/mobs/spawn_selection.rs', lab.ROOT / 'crates/horizon-client/src/mobs/spawn_rules.rs'] tested = base.source_hashes(sources) diff --git a/tools/test_vault_parity.py b/tools/test_vault_parity.py index 00e6422f5..d3025623d 100644 --- a/tools/test_vault_parity.py +++ b/tools/test_vault_parity.py @@ -174,7 +174,8 @@ def main(): resources = lab.LOCAL / 'horizon-client/26.3' oracle = lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/VanillaNaturalSpawnOracle.java' sources = [oracle, Path(__file__), lab.ROOT / 'crates/horizon-client/examples/probe/natural_spawn_fixture.rs', - lab.ROOT / 'crates/horizon-client/src/vault.rs'] + lab.ROOT / 'crates/horizon-client/src/vault.rs', + lab.ROOT / 'crates/horizon-client/src/chunk/block_entity.rs'] tested = base.source_hashes(sources) inputs = build / 'inputs.txt' cases = corpus() From 8d2b2982534d6422dedd7257f3dbae0462e400fd Mon Sep 17 00:00:00 2001 From: leifiyoo Date: Tue, 27 Oct 2026 16:53:00 +0100 Subject: [PATCH 5/8] Keep long Microsoft sign-in secrets in several keychain items Credential Manager takes at most 2560 bytes per item. The secrets of a Microsoft sign-in (the refresh token and the Minecraft token) are longer, so saving them failed on Windows with "password encoded as UTF-16 is longer than platform limit of 2560 chars" and the sign-in did not finish. On macOS and Windows a secret is now split over the items ``, `.1`, `.2`... of at most 1000 UTF-8 bytes each (under 2560 bytes of UTF-16). Reads join the pieces, a write removes the leftovers of a longer secret, and sign-out removes all of them. Existing single items keep working. Co-Authored-By: Claude Haiku 5.5 --- docs/microsoft-account.md | 3 +- launcher/src/account.rs | 96 +++++++++++++++++++++++++++++++---- launcher/src/account/tests.rs | 30 +++++++++++ 3 files changed, 117 insertions(+), 12 deletions(-) diff --git a/docs/microsoft-account.md b/docs/microsoft-account.md index 380aa0785..eeb63f56a 100644 --- a/docs/microsoft-account.md +++ b/docs/microsoft-account.md @@ -74,7 +74,8 @@ one is selected. "Sign out" deletes the account's secrets from the keychain. - Secrets (the Microsoft refresh token, the Minecraft access token and its expiry) go to the OS keychain, one item per account: Keychain on macOS, Credential Manager on Windows, the Secret Service (GNOME Keyring, KWallet) on Linux. Service name `dev.playhorizon.launcher`, account = the - Minecraft UUID. + Minecraft UUID. Credential Manager takes at most 2560 bytes per item, which the secrets of a sign-in + exceed: on macOS and Windows a longer secret is split over the items ``, `.1`, `.2`... - `accounts.json` in the launcher's data folder keeps only public data: UUID, name, skin URL, which account is selected. Never a token. - No keychain available (a Linux box without a Secret Service, a locked keychain): sign-in still diff --git a/launcher/src/account.rs b/launcher/src/account.rs index 257055911..313dbb2f8 100644 --- a/launcher/src/account.rs +++ b/launcher/src/account.rs @@ -580,28 +580,102 @@ impl SecretStore for SystemStore { } } -/// Keychain on macOS, Credential Manager on Windows (the keyring crate). +/// Keychain on macOS, Credential Manager on Windows (the keyring crate). Credential Manager takes at +/// most 2560 bytes of UTF-16 per item and a sign-in's secrets are longer, so a secret is kept in +/// pieces: the first under the account's key, the rest under `key.1`, `key.2`... #[cfg(any(target_os = "macos", target_os = "windows"))] pub struct SystemStore; + +/// Bytes of UTF-8 per keychain item: at most twice as many in UTF-16, well under 2560. +#[cfg(any(target_os = "macos", target_os = "windows"))] +const PIECE: usize = 1000; + +#[cfg(any(target_os = "macos", target_os = "windows"))] +fn piece_key(key: &str, n: usize) -> String { + if n == 0 { + key.to_string() + } else { + format!("{key}.{n}") + } +} + +/// `secret` cut into pieces of at most `PIECE` bytes, on character boundaries. +#[cfg(any(target_os = "macos", target_os = "windows"))] +fn split_secret(mut secret: &str) -> Vec<&str> { + let mut pieces = Vec::new(); + while secret.len() > PIECE { + let mut cut = PIECE; + while !secret.is_char_boundary(cut) { + cut -= 1; + } + let (head, tail) = secret.split_at(cut); + pieces.push(head); + secret = tail; + } + pieces.push(secret); + pieces +} + +#[cfg(any(target_os = "macos", target_os = "windows"))] +fn read_item(key: &str) -> Result, String> { + match keyring::Entry::new(SERVICE, key).and_then(|e| e.get_password()) { + Ok(secret) => Ok(Some(secret)), + Err(keyring::Error::NoEntry) => Ok(None), + Err(e) => Err(e.to_string()), + } +} + +#[cfg(any(target_os = "macos", target_os = "windows"))] +fn write_item(key: &str, secret: &str) -> Result<(), String> { + keyring::Entry::new(SERVICE, key) + .and_then(|e| e.set_password(secret)) + .map_err(|e| e.to_string()) +} + +/// Removes one item; false when there was none. +#[cfg(any(target_os = "macos", target_os = "windows"))] +fn remove_item(key: &str) -> Result { + match keyring::Entry::new(SERVICE, key).and_then(|e| e.delete_credential()) { + Ok(()) => Ok(true), + Err(keyring::Error::NoEntry) => Ok(false), + Err(e) => Err(e.to_string()), + } +} + #[cfg(any(target_os = "macos", target_os = "windows"))] impl SecretStore for SystemStore { fn get(&self, key: &str) -> Result, String> { - match keyring::Entry::new(SERVICE, key).and_then(|e| e.get_password()) { - Ok(secret) => Ok(Some(secret)), - Err(keyring::Error::NoEntry) => Ok(None), - Err(e) => Err(e.to_string()), + let Some(mut secret) = read_item(key)? else { + return Ok(None); + }; + for n in 1.. { + match read_item(&piece_key(key, n))? { + Some(piece) => secret.push_str(&piece), + None => break, + } } + Ok(Some(secret)) } fn set(&self, key: &str, secret: &str) -> Result<(), String> { - keyring::Entry::new(SERVICE, key) - .and_then(|e| e.set_password(secret)) - .map_err(|e| e.to_string()) + let pieces = split_secret(secret); + for (n, piece) in pieces.iter().enumerate() { + write_item(&piece_key(key, n), piece)?; + } + // Pieces left over from a longer secret would be read back as part of this one. + for n in pieces.len().. { + if !remove_item(&piece_key(key, n))? { + break; + } + } + Ok(()) } fn delete(&self, key: &str) -> Result<(), String> { - match keyring::Entry::new(SERVICE, key).and_then(|e| e.delete_credential()) { - Ok(()) | Err(keyring::Error::NoEntry) => Ok(()), - Err(e) => Err(e.to_string()), + for n in 0.. { + if !remove_item(&piece_key(key, n))? { + break; + } } + Ok(()) } fn persistent(&self) -> bool { true diff --git a/launcher/src/account/tests.rs b/launcher/src/account/tests.rs index ec3766c7e..6d0152722 100644 --- a/launcher/src/account/tests.rs +++ b/launcher/src/account/tests.rs @@ -372,6 +372,36 @@ fn accounts_keep_secrets_in_the_store_only() { assert!(!store.persistent()); } +/// The OS keychain itself: Credential Manager and the Keychain take one secret per item, and a +/// sign-in's secrets are longer than one item holds. Ignored by default: it writes to the keychain +/// (a test key, removed again). +#[cfg(any(target_os = "macos", target_os = "windows"))] +#[test] +#[ignore = "writes to the OS keychain"] +fn system_keychain_keeps_long_secrets() { + let key = "test-long-secret"; + let secret = format!("{{\"refresh_token\":\"{}\"}}", "r".repeat(3000)); + SystemStore.set(key, &secret).unwrap(); + assert_eq!(SystemStore.get(key).unwrap(), Some(secret.clone())); + SystemStore.delete(key).unwrap(); + assert_eq!(SystemStore.get(key).unwrap(), None); +} + +#[cfg(any(target_os = "macos", target_os = "windows"))] +#[test] +fn long_secrets_split_into_keychain_sized_pieces() { + // Mixed 1-, 2- and 4-byte characters, so the cuts land inside some of them. + let secret = "aé🔑".repeat(300); + let pieces = split_secret(&secret); + assert!(pieces.len() > 1); + assert_eq!(pieces.concat(), secret); + for piece in &pieces { + assert!(piece.len() <= PIECE); + assert!(piece.encode_utf16().count() * 2 <= 2560); + } + assert_eq!(split_secret("short"), vec!["short"]); +} + #[test] fn session_files_are_private_and_complete() { let dir = std::env::temp_dir().join(format!("horizon-launcher-session-{}", std::process::id())); From 7afb41a1285e418a3e3440374f6e0f99796b9713 Mon Sep 17 00:00:00 2001 From: Ludovic Marie <136255825+ludovic111@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:25:30 +0100 Subject: [PATCH 6/8] launcher: the owner gate's screen, once for offline players and on a refused Play A player without an account that owns Minecraft: Java Edition sees one screen when the launcher opens (once) and whenever Play is refused: why Horizon needs the account, like other launchers, that worlds, offline names and settings stay where they are, and Sign in, which runs the device-code sign-in on the same screen. Not now closes it; Home keeps its Sign in row, which now opens this screen. Co-Authored-By: Claude Opus 5.5 --- docs/microsoft-account.md | 5 ++ launcher/lang/fr.json | 6 +- launcher/lang/ru.json | 6 +- launcher/src/backend.rs | 4 + launcher/src/main.rs | 24 ++++-- launcher/src/owner_page.rs | 157 +++++++++++++++++++++++++++++++++++++ 6 files changed, 194 insertions(+), 8 deletions(-) create mode 100644 launcher/src/owner_page.rs diff --git a/docs/microsoft-account.md b/docs/microsoft-account.md index 314115fb3..9c457f8a0 100644 --- a/docs/microsoft-account.md +++ b/docs/microsoft-account.md @@ -77,6 +77,11 @@ possible; the launcher just doesn't offer it on its own. again for each account, the selected one first, until one owns the game: the token renewal (below) if due, then entitlements and profile. Without an owner the game doesn't start and Play says "Sign in with a Microsoft account that owns Minecraft: Java Edition to play", with Sign in. +- **The window says it once.** A player without an owner (one who played offline until now) sees + the gate's screen once when the launcher opens: why Horizon needs the account ("like other + launchers"), that worlds, offline names and settings stay where they are, and Sign in, which shows + the device code on the same screen. It comes back whenever Play is refused; Not now closes it, + and Home keeps a Sign in row (`launcher/src/owner_page.rs`). - **The answer lives with the tokens.** Each account's keychain item keeps the last answer (`owns`), so no plain file can say yes. With no network (or a sign-in that can't be renewed) the last answer stands: being offline never blocks a player who passed before, and the game starts diff --git a/launcher/lang/fr.json b/launcher/lang/fr.json index f15700ad3..85c8238e8 100644 --- a/launcher/lang/fr.json +++ b/launcher/lang/fr.json @@ -167,5 +167,9 @@ "Sign in with a Microsoft account that owns Minecraft: Java Edition to play": "Connectez-vous avec un compte Microsoft qui possède Minecraft: Java Edition pour jouer", "Horizon needs a Microsoft account that owns Minecraft: Java Edition, like other launchers. It also plays online servers with your name and skin.": "Horizon a besoin d'un compte Microsoft qui possède Minecraft: Java Edition, comme les autres launchers. Il permet aussi de jouer sur les serveurs en ligne avec votre nom et votre skin.", "Sign in": "Se connecter", - "Sign in with Microsoft": "Se connecter avec Microsoft" + "Sign in with Microsoft": "Se connecter avec Microsoft", + "Not now": "Pas maintenant", + "Horizon needs a Microsoft account that owns Minecraft: Java Edition, like other launchers.": "Horizon a besoin d'un compte Microsoft qui possède Minecraft: Java Edition, comme les autres launchers.", + "Your worlds, offline names and settings stay where they are.": "Vos mondes, vos noms hors ligne et vos réglages restent où ils sont.", + "Open {link} in your browser and enter this code:": "Ouvrez {link} dans votre navigateur et entrez ce code :" } diff --git a/launcher/lang/ru.json b/launcher/lang/ru.json index ca14e22d7..8c8458a55 100644 --- a/launcher/lang/ru.json +++ b/launcher/lang/ru.json @@ -167,5 +167,9 @@ "Sign in with a Microsoft account that owns Minecraft: Java Edition to play": "Чтобы играть, войдите в аккаунт Microsoft, у которого есть Minecraft: Java Edition", "Horizon needs a Microsoft account that owns Minecraft: Java Edition, like other launchers. It also plays online servers with your name and skin.": "Как и другим лаунчерам, Horizon нужен аккаунт Microsoft, у которого есть Minecraft: Java Edition. С ним вы также играете на онлайн-серверах под своим именем и со своим скином.", "Sign in": "Войти", - "Sign in with Microsoft": "Войти через Microsoft" + "Sign in with Microsoft": "Войти через Microsoft", + "Not now": "Не сейчас", + "Horizon needs a Microsoft account that owns Minecraft: Java Edition, like other launchers.": "Как и другим лаунчерам, Horizon нужен аккаунт Microsoft, у которого есть Minecraft: Java Edition.", + "Your worlds, offline names and settings stay where they are.": "Ваши миры, офлайн-имена и настройки остаются на месте.", + "Open {link} in your browser and enter this code:": "Откройте {link} в браузере и введите этот код:" } diff --git a/launcher/src/backend.rs b/launcher/src/backend.rs index 2b97ecf6c..aacdf81ec 100644 --- a/launcher/src/backend.rs +++ b/launcher/src/backend.rs @@ -2625,6 +2625,10 @@ pub struct Settings { /// first-run question, or on the Play page once if they answered that before it existed. #[serde(default)] pub ping_notice_seen: bool, + /// The player without an account that owns the game has read, once, why the launcher needs + /// one now (the owner gate's screen). + #[serde(default)] + pub owner_notice_seen: bool, /// Seconds the sidebar's partner banner has been on screen, over every start, up to /// `partner::SEEN_BEFORE_HIDE`: only then can the player hide it. #[serde(default)] diff --git a/launcher/src/main.rs b/launcher/src/main.rs index fd3726da6..15ec943f2 100644 --- a/launcher/src/main.rs +++ b/launcher/src/main.rs @@ -27,6 +27,7 @@ mod modpacks_page; mod mods_page; mod mods_project; mod multiplayer_page; +mod owner_page; mod packs_page; mod partner; mod perf; @@ -210,6 +211,8 @@ struct Launcher { /// Whether an account here owns the game, by the keychain's last answers (the owner gate); /// none until the keychain has been read. owner: Option, + /// Play was refused (or Home's Sign in pressed): the owner gate's screen is open. + owner_screen_open: bool, /// The game is waiting for its session (a token renewal) before it starts. starting_client: Option, /// Join a friend: the code typed on the Play page, and the one the game @@ -415,6 +418,7 @@ impl Launcher { sign_in_state: None, keychain: true, owner: None, + owner_screen_open: false, starting_client: None, friend_code: String::new(), friend_focused: false, @@ -648,6 +652,7 @@ impl Launcher { Ok(pass) => pass, Err(account::NoOwner) => { self.owner = Some(false); + self.owner_screen_open = true; self.client_error = Some(format!("{}.", tr(account::NEEDS_OWNER))); self.starting_pack = None; self.joining = None; @@ -917,7 +922,12 @@ impl Launcher { self.accounts = Accounts::load(); // Sign-in refuses accounts that don't own the game. self.owner = Some(true); + self.owner_screen_open = false; self.client_error = None; + if !self.settings.owner_notice_seen { + self.settings.owner_notice_seen = true; + self.settings.save(); + } } self.sign_in_state = Some(state); changed = true; @@ -1088,7 +1098,7 @@ impl Launcher { }); } - /// Starts the Microsoft sign-in; Settings shows its code. + /// Starts the Microsoft sign-in; Settings and the owner gate's screen show its code. fn start_sign_in(&mut self) { if let Some(old) = self.sign_in.replace(SignIn::start()) { old.cancel(); @@ -2301,8 +2311,7 @@ impl Launcher { Some( button("gate-sign-in", "Sign in", Kind::Primary, &t) .on_click(cx.listener(|this, _: &ClickEvent, _, cx| { - this.start_sign_in(); - this.go(Page::Settings); + this.sign_in_to_play(); cx.notify(); })) .into_any_element(), @@ -3201,9 +3210,9 @@ impl Launcher { .flex() .flex_col() .gap(px(6.)) - .child(format!( - "Open {} in your browser and enter this code:", - code.verification_uri + .child(trf( + "Open {link} in your browser and enter this code:", + &[("link", &code.verification_uri)], )) .child( text(theme::HEADING) @@ -3907,6 +3916,9 @@ impl Render for Launcher { if self.asking_consent() { root = root.child(self.consent_screen(cx)); } + if self.asking_owner() { + root = root.child(self.owner_screen(cx)); + } if let Some(opening) = &self.opening { // The wordmark arrives letter by letter, then gives way to the launcher; a click or // a key skips ahead. diff --git a/launcher/src/owner_page.rs b/launcher/src/owner_page.rs new file mode 100644 index 000000000..5a27b064b --- /dev/null +++ b/launcher/src/owner_page.rs @@ -0,0 +1,157 @@ +//! The owner gate in the window (`horizon_launcher::account::launch`): the screen that asks for a +//! Microsoft account that owns Minecraft: Java Edition. It covers the launcher once for players +//! without one (those who played offline before the gate), and again whenever Play is refused. +//! The sign-in runs on it, code and all. +use super::*; + +impl Launcher { + /// Whether the screen covers the window: no owner, and either Play was refused or the player + /// hasn't read it yet. The data question comes first. + pub(crate) fn asking_owner(&self) -> bool { + self.owner == Some(false) + && (self.owner_screen_open || !self.settings.owner_notice_seen) + && !self.asking_consent() + && self.opening.is_none() + } + + /// Opens the screen and starts the sign-in on it. + pub(crate) fn sign_in_to_play(&mut self) { + self.owner_screen_open = true; + self.start_sign_in(); + } + + /// Closes it; it comes back with the next refused Play. + fn close_owner_screen(&mut self) { + if let Some(s) = self.sign_in.take() { + s.cancel(); + } + self.sign_in_state = None; + self.owner_screen_open = false; + if !self.settings.owner_notice_seen { + self.settings.owner_notice_seen = true; + self.settings.save(); + } + } + + pub(crate) fn owner_screen(&mut self, cx: &mut Context) -> Stateful
{ + let t = self.theme; + let configured = account::client_id().is_some(); + let mut body = div() + .flex() + .flex_col() + .gap(px(12.)) + .p(px(28.)) + .child(caps(tr("Microsoft account")).text_color(t.muted)) + .child( + text(theme::HEADING) + .text_color(t.ink) + .child(format!("{}.", tr(account::NEEDS_OWNER))), + ) + .child(text(theme::UI).text_color(t.muted).child(tr( + "Horizon needs a Microsoft account that owns Minecraft: Java Edition, like other launchers.", + ))) + .child(text(theme::UI).text_color(t.muted).child(tr( + "Your worlds, offline names and settings stay where they are.", + ))); + let status = match &self.sign_in_state { + _ if !configured => Some(div().child(tr(account::NOT_CONFIGURED))), + Some(SignInState::Starting) => Some(div().child(tr("Asking Microsoft…"))), + Some(SignInState::Code(code)) => Some( + div() + .flex() + .flex_col() + .gap(px(6.)) + .child(trf( + "Open {link} in your browser and enter this code:", + &[("link", &code.verification_uri)], + )) + .child( + text(theme::HEADING) + .font_family(theme::MONO) + .text_color(t.ink) + .child(code.user_code.clone()), + ), + ), + Some(SignInState::Done(Err(e))) => Some(div().text_color(t.danger).child(e.clone())), + Some(SignInState::Done(Ok(_))) | None => None, + }; + body = body.children(status.map(|s| { + plate(&t) + .p(px(16.)) + .text_size(px(theme::UI.px)) + .text_color(t.ink_soft) + .child(s) + })); + let mut answers = div() + .flex() + .justify_end() + .gap(px(12.)) + .px(px(28.)) + .pb(px(28.)) + .child( + button("owner-later", "Not now", Kind::Secondary, &t).on_click(cx.listener( + |this, _: &ClickEvent, _, cx| { + this.close_owner_screen(); + cx.notify(); + }, + )), + ); + answers = match &self.sign_in_state { + Some(SignInState::Code(code)) => { + let (uri, user_code) = (code.verification_uri.clone(), code.user_code.clone()); + answers + .child( + button("owner-copy", "Copy code", Kind::Secondary, &t).on_click( + move |_, _, cx| { + copy_to_clipboard( + cx, + gpui::ClipboardItem::new_string(user_code.clone()), + ) + }, + ), + ) + .child( + button("owner-open", "Open link", Kind::Primary, &t) + .on_click(move |_, _, cx| cx.open_url(&uri)), + ) + } + Some(SignInState::Starting) => answers, + _ if !configured => answers, + _ => answers.child( + button("owner-sign-in", "Sign in", Kind::Primary, &t).on_click(cx.listener( + |this, _: &ClickEvent, _, cx| { + this.sign_in_to_play(); + cx.notify(); + }, + )), + ), + }; + let card = plate(&t) + .flex() + .flex_col() + // A set width: text measured at it wraps where it is drawn. + .w(px(560.)) + .max_w_full() + .bg(t.page) + .border_1() + .border_color(t.rule) + .child(body) + .child(answers); + div() + .id("owner-gate") + .absolute() + .top_0() + .left_0() + .size_full() + .flex() + .flex_col() + .items_center() + .justify_center() + .px(px(32.)) + .py(px(24.)) + .bg(t.page.opacity(0.92)) + .on_scroll_wheel(|_, _, cx| cx.stop_propagation()) + .occlude() + .child(card) + } +} From 610c2f3c10ff5cdda4408afd17463e8aaad85a1f Mon Sep 17 00:00:00 2001 From: Ludovic Marie <136255825+ludovic111@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:33:53 +0100 Subject: [PATCH 7/8] launcher: Download and prepare, from Mojang, for an account that owns the game With the owner gate in place, a signed-in owner no longer needs another launcher first (brief mc-download-1, Ludovic 2026-10-27). - The Play page's Resources row offers an owner "Download and prepare" and "From my install"; without an owner it asks to sign in first. - `horizon-launcher prepare --download` does the same. - Every file from Mojang must have the size and sha1 Mojang lists. - The download shows its progress and has Cancel; files already whole are kept, so the next try goes on where it stopped. Co-Authored-By: Claude Opus 5.5 --- docs/launcher-commands.md | 3 +- docs/microsoft-account.md | 11 +++ launcher/README.md | 9 +++ launcher/lang/fr.json | 7 +- launcher/lang/ru.json | 7 +- launcher/src/backend.rs | 41 +++++++--- launcher/src/cli.rs | 21 ++++- launcher/src/main.rs | 86 ++++++++++++++++++-- launcher/src/mojang.rs | 85 ++++++++++++++++---- launcher/src/mojang_tests.rs | 148 ++++++++++++++++++++++++++++++++--- launcher/src/servers_page.rs | 2 +- 11 files changed, 367 insertions(+), 53 deletions(-) diff --git a/docs/launcher-commands.md b/docs/launcher-commands.md index df098faa6..d5ed3491a 100644 --- a/docs/launcher-commands.md +++ b/docs/launcher-commands.md @@ -36,11 +36,12 @@ MCP: `remove`, needs `confirm: true`. ## `horizon-launcher prepare` -Prepare a version's resources from your own Minecraft install. +Prepare a version's resources from your own Minecraft install, or from Mojang. horizon-launcher prepare - `--minecraft`: The game folder of the official launcher, Prism Launcher, MultiMC, the Modrinth App, ATLauncher or GDLauncher (default: the launcher's, then every launcher's default) +- `--download`: Download Minecraft from Mojang for the signed-in account that owns it, instead of using an install MCP: `prepare`. diff --git a/docs/microsoft-account.md b/docs/microsoft-account.md index 9c457f8a0..33207da33 100644 --- a/docs/microsoft-account.md +++ b/docs/microsoft-account.md @@ -93,6 +93,17 @@ possible; the launcher just doesn't offer it on its own. the launcher's tests use accounts in a memory store (`MemoryStore`, and `HORIZON_NO_KEYCHAIN` for the CLI and MCP tests, which then have no owner). +## Downloading the game for an owner + +Decided 2026-10-27. This is the only exception to "Horizon never downloads the game", and it +applies only on the player's machine, for an account the owner gate just asked about. The +launcher's "Download and prepare" (`horizon-launcher prepare --download`) fetches the pinned +version straight from Mojang (`launcher/src/mojang.rs`): the version file, the client jar, +libraries, the asset index and the assets, each checked against the size and sha1 Mojang lists. +They go to `minecraft/` in the launcher's data folder, and the preparation runs as usual. The +download can be cancelled and goes on where it stopped. CI, agents, harnesses and the repo never +download or keep Mojang's files. + Refresh: the Minecraft token is renewed from the Microsoft refresh token (steps 2-5 with `grant_type=refresh_token`) when it has less than an hour left, before every launch; steps 6-7 run at every launch (the owner gate). A refused diff --git a/launcher/README.md b/launcher/README.md index b2b497753..9ded5d621 100644 --- a/launcher/README.md +++ b/launcher/README.md @@ -24,6 +24,15 @@ The opening, then Play ([clip](docs/opening.webm)): preparation reuses checksum-verified game jars and libraries from the current or another installed Horizon version, so the original Minecraft install is only required when no valid imported copy exists. +- **Download and prepare**: for a signed-in account that owns the game (asked again first), + the launcher downloads the pinned version from Mojang itself (piston-meta, piston-data, + libraries.minecraft.net, resources.download.minecraft.net; never a mirror) into `minecraft/` + in its data folder, in the official launcher's layout: the version file, the client jar, this + system's libraries, the asset index and the assets. Every file must have the size and sha1 + Mojang lists. The Play page shows how far it is and has Cancel; files already whole are kept, + so the next try goes on from there. **From my install** prepares from the player's own launcher + instead, and an install that lacks the version falls back to the download. Without an owner, + the Play page asks to sign in first. `horizon-launcher prepare --download`. - **Mods**: search Modrinth (CurseForge support is coming soon) for resource packs and modpacks, each marked Works, Partly or Doesn't run in Horizon; Java mod jars are never offered. Install puts a pack and what it needs in diff --git a/launcher/lang/fr.json b/launcher/lang/fr.json index 85c8238e8..a8532491e 100644 --- a/launcher/lang/fr.json +++ b/launcher/lang/fr.json @@ -171,5 +171,10 @@ "Not now": "Pas maintenant", "Horizon needs a Microsoft account that owns Minecraft: Java Edition, like other launchers.": "Horizon a besoin d'un compte Microsoft qui possède Minecraft: Java Edition, comme les autres launchers.", "Your worlds, offline names and settings stay where they are.": "Vos mondes, vos noms hors ligne et vos réglages restent où ils sont.", - "Open {link} in your browser and enter this code:": "Ouvrez {link} dans votre navigateur et entrez ce code :" + "Open {link} in your browser and enter this code:": "Ouvrez {link} dans votre navigateur et entrez ce code :", + "From my install": "Depuis mon installation", + "Download and prepare": "Télécharger et préparer", + "Not prepared. Horizon can download Minecraft {version} from Mojang for your account, or use your own install.": "Pas préparé. Horizon peut télécharger Minecraft {version} depuis Mojang pour votre compte, ou utiliser votre propre installation.", + "Not prepared. Sign in with the Microsoft account that owns Minecraft {version} first.": "Pas préparé. Connectez-vous d'abord avec le compte Microsoft qui possède Minecraft {version}.", + "Cancel": "Annuler" } diff --git a/launcher/lang/ru.json b/launcher/lang/ru.json index 8c8458a55..9fa58b990 100644 --- a/launcher/lang/ru.json +++ b/launcher/lang/ru.json @@ -171,5 +171,10 @@ "Not now": "Не сейчас", "Horizon needs a Microsoft account that owns Minecraft: Java Edition, like other launchers.": "Как и другим лаунчерам, Horizon нужен аккаунт Microsoft, у которого есть Minecraft: Java Edition.", "Your worlds, offline names and settings stay where they are.": "Ваши миры, офлайн-имена и настройки остаются на месте.", - "Open {link} in your browser and enter this code:": "Откройте {link} в браузере и введите этот код:" + "Open {link} in your browser and enter this code:": "Откройте {link} в браузере и введите этот код:", + "From my install": "Из моей установки", + "Download and prepare": "Скачать и подготовить", + "Not prepared. Horizon can download Minecraft {version} from Mojang for your account, or use your own install.": "Не подготовлено. Horizon может скачать Minecraft {version} у Mojang для вашего аккаунта или взять вашу установку.", + "Not prepared. Sign in with the Microsoft account that owns Minecraft {version} first.": "Не подготовлено. Сначала войдите в аккаунт Microsoft, у которого есть Minecraft {version}.", + "Cancel": "Отмена" } diff --git a/launcher/src/backend.rs b/launcher/src/backend.rs index aacdf81ec..cd72440a6 100644 --- a/launcher/src/backend.rs +++ b/launcher/src/backend.rs @@ -1227,7 +1227,7 @@ fn download_for_owner( sources::Os::MacOs => "osx", sources::Os::Windows => "windows", }; - mojang::download(pins, dir, os, 8, fetch, log).map_err(|e| { + mojang::download(pins, dir, os, 8, fetch, &mojang::CANCEL, log).map_err(|e| { format!("\n • Horizon tried to download {v} from Mojang for {name}, but: {e}") }) } @@ -1299,7 +1299,8 @@ fn java( Ok(body) }; let runtime = data_dir().join("minecraft").join("runtime"); - let home = mojang::java_runtime(&component, &runtime, 8, &fetch, log).map_err(|e| { + let home = mojang::java_runtime(&component, &runtime, 8, &fetch, &mojang::CANCEL, log) + .map_err(|e| { format!("{missing}\n • Horizon also tried to download Minecraft's own Java from Mojang, but: {e}") })?; find(major, &[home]).map(|jdk| jdk.home()) @@ -1351,6 +1352,16 @@ fn assets_folder( } } +/// Where the preparation takes Minecraft's files from. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum MinecraftFrom { + /// The player's own install (any launcher's); Mojang only when none has the version and an + /// account that owns the game is signed in. + Install, + /// Mojang, for the signed-in account that owns the game ("Download and prepare"). + Mojang, +} + /// Checks what the preparation needs and picks how to run it. PLAY.txt describes the same /// steps: the player's own launcher install, the server jar from that version's own json, and /// Java (and Python, for versions before horizon-prepare). Also returns the Minecraft folder @@ -1359,6 +1370,7 @@ fn assets_folder( pub fn preparer( tag: &str, minecraft: &Path, + from: MinecraftFrom, log: &dyn Fn(String), ) -> Result<(Preparer, PathBuf), String> { let version = game_version(tag); @@ -1386,16 +1398,23 @@ pub fn preparer( Ok(body) }; let linked = data_dir().join("linked-minecraft"); - let resolved = sources::resolve(minecraft, &pins, &known, &linked, &fetch, log); - if reused { - assets_folder(resolved, &chosen, &version, log) + if from == MinecraftFrom::Mojang { + // Straight from Mojang (what is already there and whole is kept), for an owner only. + download_for_owner(&pins, &own, &fetch, log) + .map_err(|e| e.trim_start_matches("\n • ").to_string())?; + own } else { - match resolved? { - sources::Resolved::Folder(dir) => dir, - sources::Resolved::NotFound(checked) => { - let not_found = sources::not_found(&version, &checked, minecraft); - download_for_owner(&pins, &own, &fetch, log).map_err(|e| not_found + &e)?; - own + let resolved = sources::resolve(minecraft, &pins, &known, &linked, &fetch, log); + if reused { + assets_folder(resolved, &chosen, &version, log) + } else { + match resolved? { + sources::Resolved::Folder(dir) => dir, + sources::Resolved::NotFound(checked) => { + let not_found = sources::not_found(&version, &checked, minecraft); + download_for_owner(&pins, &own, &fetch, log).map_err(|e| not_found + &e)?; + own + } } } } diff --git a/launcher/src/cli.rs b/launcher/src/cli.rs index 6fb73af5d..7d71084f3 100644 --- a/launcher/src/cli.rs +++ b/launcher/src/cli.rs @@ -548,12 +548,16 @@ enum Cmd { Install { tag: String }, /// Remove an installed version Remove { tag: String }, - /// Prepare a version's resources from your own Minecraft install + /// Prepare a version's resources from your own Minecraft install, or from Mojang Prepare { /// The game folder of the official launcher, Prism Launcher, MultiMC, the Modrinth App, /// ATLauncher or GDLauncher (default: the launcher's, then every launcher's default) - #[arg(long)] + #[arg(long, conflicts_with = "download")] minecraft: Option, + /// Download Minecraft from Mojang for the signed-in account that owns it, instead of + /// using an install + #[arg(long)] + download: bool, /// The installed version (default: the selected one) #[arg(long)] version: Option, @@ -974,11 +978,20 @@ fn execute(command: Cmd, as_json: bool) -> Result<(), String> { backend::remove_version(&tag)?; println!("Removed {tag}."); } - Cmd::Prepare { minecraft, version } => { + Cmd::Prepare { + minecraft, + download, + version, + } => { let tag = backend::resolve_version(version.as_deref())?; let minecraft = backend::minecraft_dir(minecraft)?; + let from = if download { + backend::MinecraftFrom::Mojang + } else { + backend::MinecraftFrom::Install + }; let (preparer, minecraft) = - backend::preparer(&tag, &minecraft, &|line| eprintln!("{line}"))?; + backend::preparer(&tag, &minecraft, from, &|line| eprintln!("{line}"))?; backend::sounds(&tag, &minecraft, &|line| println!("{line}")); match preparer { backend::Preparer::Process(cmd) => attached(cmd)?, diff --git a/launcher/src/main.rs b/launcher/src/main.rs index 15ec943f2..135769376 100644 --- a/launcher/src/main.rs +++ b/launcher/src/main.rs @@ -1021,10 +1021,13 @@ impl Launcher { }); } - fn prepare(&mut self) { + /// Prepares the selected version, its Minecraft files from the player's install or, for an + /// owner who asks, from Mojang. + fn prepare(&mut self, from: backend::MinecraftFrom) { let Some(tag) = self.selected().cloned() else { return; }; + horizon_launcher::mojang::CANCEL.store(false, Ordering::Relaxed); let minecraft = self .settings .minecraft_dir @@ -1040,7 +1043,7 @@ impl Launcher { let log = |line| { let _ = tx.send(Msg::Line(Job::Prepare, line)); }; - let result = backend::preparer(&tag, &minecraft, &log) + let result = backend::preparer(&tag, &minecraft, from, &log) .inspect(|(_, minecraft)| backend::sounds(&tag, minecraft, &log)) .and_then(|(p, _)| match p { backend::Preparer::Process(cmd) => { @@ -2232,10 +2235,22 @@ impl Launcher { .child(play), ); - let resources_value = if self.preparing { - tr("Preparing…").to_string() - } else { - backend::resources_summary(prepared, !silent, &version) + // Not prepared: an owner can have Horizon download Minecraft from Mojang; without an + // owner there is nothing to play, so signing in comes first. + let first = (!prepared && !self.preparing) + .then_some(self.owner) + .flatten(); + let resources_value = match first { + _ if self.preparing => tr("Preparing…").to_string(), + Some(true) => trf( + "Not prepared. Horizon can download Minecraft {version} from Mojang for your account, or use your own install.", + &[("version", &version)], + ), + Some(false) => trf( + "Not prepared. Sign in with the Microsoft account that owns Minecraft {version} first.", + &[("version", &version)], + ), + None => backend::resources_summary(prepared, !silent, &version), }; let mut prepare = button( "prepare", @@ -2251,10 +2266,44 @@ impl Launcher { prepare = disabled(prepare); } else { prepare = prepare.on_click(cx.listener(|this, _: &ClickEvent, _, cx| { - this.prepare(); + this.prepare(backend::MinecraftFrom::Install); cx.notify(); })); } + let prepare = match first { + Some(true) => div() + .flex() + .gap(px(8.)) + .child( + button("prepare-install", "From my install", Kind::Ghost, &t).on_click( + cx.listener(|this, _: &ClickEvent, _, cx| { + this.prepare(backend::MinecraftFrom::Install); + cx.notify(); + }), + ), + ) + .child( + button( + "prepare-download", + "Download and prepare", + Kind::Secondary, + &t, + ) + .on_click(cx.listener(|this, _: &ClickEvent, _, cx| { + this.prepare(backend::MinecraftFrom::Mojang); + cx.notify(); + })), + ), + Some(false) => div().child( + button("prepare-sign-in", "Sign in", Kind::Secondary, &t).on_click(cx.listener( + |this, _: &ClickEvent, _, cx| { + this.sign_in_to_play(); + cx.notify(); + }, + )), + ), + None => div().child(prepare), + }; let minecraft = self.settings.minecraft_dir.as_deref().map_or_else( || { div() @@ -2394,12 +2443,33 @@ impl Launcher { // looks after pressing Prepare, not below the fold (#666). if self.preparing { let last = self.prepare_log.back().cloned().unwrap_or_default(); + // A download from Mojang shows how far it is, and can stop (it goes on from there + // next time). + let download = horizon_launcher::mojang::progress(); + let cancel = download.map(|_| { + button("download-cancel", "Cancel", Kind::Ghost, &t).on_click(|_, _, _| { + horizon_launcher::mojang::CANCEL.store(true, Ordering::Relaxed) + }) + }); page = page.child( div() .flex() .flex_col() .gap(px(10.)) - .child(progress_bar(None, &t)) + .child( + div() + .flex() + .items_center() + .gap(px(12.)) + .child(progress_bar( + download.map(|(done, total)| { + let f = done as f32 / total as f32; + (f, f) + }), + &t, + )) + .children(cancel), + ) .child( text(theme::MICRO) .text_color(t.muted) diff --git a/launcher/src/mojang.rs b/launcher/src/mojang.rs index f242672b9..32688f667 100644 --- a/launcher/src/mojang.rs +++ b/launcher/src/mojang.rs @@ -3,12 +3,16 @@ //! jar, this system's libraries and the assets (the sounds), each checked against its sha1, //! in the official launcher's layout in the launcher's data folder. sources.rs then finds them //! there like any launcher's. Nothing of Mojang's is part of Horizon. +//! +//! Each file is checked against the size and sha1 Mojang lists, and written whole. A download +//! stopped half way (cancelled, no network) keeps the files it finished: the next one fetches +//! only the rest. -use crate::sources::{fetch_checked, fetch_manifest, sha1_file, write_file, Fetch, Pins}; +use crate::sources::{fetch_checked, fetch_manifest, sha1_file, sha1_hex, write_file, Fetch, Pins}; use serde_json::Value; use std::fs; use std::path::{Component, Path, PathBuf}; -use std::sync::atomic::{AtomicUsize, Ordering::Relaxed}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering::Relaxed}; use std::sync::Mutex; use std::time::Duration; @@ -16,22 +20,38 @@ const OBJECTS: &str = "https://resources.download.minecraft.net"; /// The index of Mojang's Java runtimes, the one the official launcher reads. const JAVA_RUNTIMES: &str = "https://launchermeta.mojang.com/v1/products/java-runtime/2ec0cc96c44e5a76b9c8b7c39df7210883d12871/all.json"; -/// One file to download. +/// Set to stop the download under way (the window's Cancel); cleared when a preparation starts. +pub static CANCEL: AtomicBool = AtomicBool::new(false); +/// The download under way: files done, of how many (0 when none runs). +static DONE: AtomicUsize = AtomicUsize::new(0); +static TOTAL: AtomicUsize = AtomicUsize::new(0); + +/// How far the download under way is, files done of how many; none when none runs. +pub fn progress() -> Option<(usize, usize)> { + let total = TOTAL.load(Relaxed); + (total > 0).then(|| (DONE.load(Relaxed).min(total), total)) +} + +const CANCELLED: &str = "Download cancelled. Download again to go on where it stopped."; + +/// One file to download, with the size Mojang lists for it when it lists one. struct File { url: String, sha1: String, + size: Option, path: PathBuf, } /// Downloads the pinned version into `dir`, `jobs` files at a time; files already there and /// whole are kept. An asset that fails is a line in the log (the game is then partly silent), -/// anything else an error. +/// anything else an error. `cancel` stops it between files. pub fn download( pins: &Pins, dir: &Path, os: &str, jobs: usize, fetch: Fetch, + cancel: &AtomicBool, log: &dyn Fn(String), ) -> Result<(), String> { let v = &pins.version; @@ -43,6 +63,7 @@ pub fn download( let mut game = vec![File { url: text(client, "url")?, sha1: pins.client_sha1.clone(), + size: client["size"].as_u64(), path: versions.join(format!("{v}.jar")), }]; let libraries = manifest["libraries"] @@ -56,6 +77,7 @@ pub fn download( game.push(File { url: text(artifact, "url")?, sha1: text(artifact, "sha1")?, + size: artifact["size"].as_u64(), path: dir .join("libraries") .join(relative(&text(artifact, "path")?)?), @@ -65,23 +87,26 @@ pub fn download( "Downloading Minecraft {v} from Mojang: the game and {} libraries", game.len() - 1 )); - fetch_all(&game, jobs, fetch, log)?; + fetch_all(&game, jobs, fetch, cancel, log)?; let index = &manifest["assetIndex"]; let index = File { url: text(index, "url")?, sha1: text(index, "sha1")?, + size: index["size"].as_u64(), path: dir .join("assets/indexes") .join(relative(&format!("{}.json", text(index, "id")?))?), }; - let assets = fetch_all(std::slice::from_ref(&index), 1, fetch, log) + let assets = fetch_all(std::slice::from_ref(&index), 1, fetch, cancel, log) .and_then(|_| objects(&read_json(&index.path)?, dir)) .and_then(|objects| { log(format!("Downloading the sounds ({} files)", objects.len())); - fetch_all(&objects, jobs, fetch, log) + fetch_all(&objects, jobs, fetch, cancel, log) }); - if let Err(e) = assets { - log(format!("The sounds could not all be downloaded: {e}")); + match assets { + Err(e) if e == CANCELLED => return Err(e), + Err(e) => log(format!("The sounds could not all be downloaded: {e}")), + Ok(()) => {} } Ok(()) } @@ -110,6 +135,7 @@ pub fn java_runtime( runtime: &Path, jobs: usize, fetch: Fetch, + cancel: &AtomicBool, log: &dyn Fn(String), ) -> Result { let platform = java_platform().ok_or("Mojang has no Java for this system")?; @@ -144,6 +170,7 @@ pub fn java_runtime( downloads.push(File { url: text(raw, "url")?, sha1: text(raw, "sha1")?, + size: raw["size"].as_u64(), path, }); } @@ -156,7 +183,7 @@ pub fn java_runtime( "Downloading Minecraft's Java {name} from Mojang ({} files)", downloads.len() )); - fetch_all(&downloads, jobs, fetch, log)?; + fetch_all(&downloads, jobs, fetch, cancel, log)?; #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -243,6 +270,10 @@ fn objects(index: &Value, dir: &Path) -> Result, String> { let objects = index["objects"] .as_object() .ok_or("The asset index has no objects")?; + let sizes: std::collections::HashMap<&str, u64> = objects + .values() + .filter_map(|o| Some((o["hash"].as_str()?, o["size"].as_u64()?))) + .collect(); let mut hashes: Vec<&str> = objects .values() .filter_map(|o| o["hash"].as_str()) @@ -259,23 +290,42 @@ fn objects(index: &Value, dir: &Path) -> Result, String> { Ok(File { url: format!("{OBJECTS}/{path}"), sha1: hash.to_string(), + size: sizes.get(hash).copied(), path: dir.join("assets/objects").join(path), }) }) .collect() } -/// Downloads the files not already there, `jobs` at a time, with a line every few seconds; -/// stops at the first error. +/// One file from Mojang, refused unless it has the size and sha1 Mojang lists. +fn fetch_file(fetch: Fetch, file: &File) -> Result, String> { + let body = fetch(&file.url)?; + if let Some(size) = file.size.filter(|&size| size != body.len() as u64) { + return Err(format!( + "is {} bytes, not the {size} Mojang lists", + body.len() + )); + } + match sha1_hex(&body) { + actual if actual == file.sha1 => Ok(body), + actual => Err(format!("is not the one Horizon pins (sha1 {actual})")), + } +} + +/// Downloads the files not already there, `jobs` at a time, with a line every second; stops at +/// the first error, or between files once `cancel` is set. fn fetch_all( files: &[File], jobs: usize, fetch: Fetch, + cancel: &AtomicBool, log: &dyn Fn(String), ) -> Result<(), String> { let next = AtomicUsize::new(0); let done = AtomicUsize::new(0); let failed = Mutex::new(None); + DONE.store(0, Relaxed); + TOTAL.store(files.len(), Relaxed); std::thread::scope(|scope| { let workers: Vec<_> = (0..jobs.clamp(1, files.len().max(1))) .map(|_| { @@ -284,15 +334,19 @@ fn fetch_all( if failed.lock().unwrap().is_some() { break; } + if cancel.load(Relaxed) { + failed.lock().unwrap().get_or_insert(CANCELLED.to_string()); + break; + } if sha1_file(&file.path).as_ref() != Some(&file.sha1) { - let got = fetch_checked(fetch, &file.url, &file.sha1) + let got = fetch_file(fetch, file) .map_err(|e| format!("{} {e}", file.url)) .and_then(|body| write_file(&file.path, &body)); if let Err(e) = got { failed.lock().unwrap().get_or_insert(e); } } - done.fetch_add(1, Relaxed); + DONE.store(done.fetch_add(1, Relaxed) + 1, Relaxed); } }) }) @@ -301,11 +355,12 @@ fn fetch_all( while !workers.iter().all(|w| w.is_finished()) { std::thread::sleep(Duration::from_millis(100)); waited += 1; - if waited % 50 == 0 { + if waited % 10 == 0 { log(format!("{} of {} files", done.load(Relaxed), files.len())); } } }); + TOTAL.store(0, Relaxed); failed.into_inner().unwrap().map_or(Ok(()), Err) } diff --git a/launcher/src/mojang_tests.rs b/launcher/src/mojang_tests.rs index cd32e4c79..310fd72c2 100644 --- a/launcher/src/mojang_tests.rs +++ b/launcher/src/mojang_tests.rs @@ -8,6 +8,8 @@ const COMMON: &[u8] = b"a library every system uses"; const LINUX: &[u8] = b"a linux native"; const SOUND: &[u8] = b"a fake ogg"; const LANG: &[u8] = b"{}"; +/// Never cancelled. +static GO: AtomicBool = AtomicBool::new(false); /// The fake version file, asset index and files, by URL. fn mojang() -> (Pins, HashMap>) { @@ -28,7 +30,7 @@ fn mojang() -> (Pins, HashMap>) { "id": "26.3", "assetIndex": {"id": "34", "sha1": sha1_hex(index.as_bytes()), "url": "https://piston-meta.mojang.com/v1/packages/i/34.json"}, - "downloads": {"client": {"sha1": sha1_hex(CLIENT), "url": "https://piston-data.mojang.com/client.jar"}}, + "downloads": {"client": {"sha1": sha1_hex(CLIENT), "size": CLIENT.len(), "url": "https://piston-data.mojang.com/client.jar"}}, "libraries": [ {"name": "org.fake:common:1", "downloads": {"artifact": artifact("common", COMMON)}}, {"name": "org.fake:linux:1", "downloads": {"artifact": artifact("linux", LINUX)}, @@ -112,9 +114,15 @@ fn an_owner_gets_the_official_layout_with_this_systems_libraries() { }; let dir = scratch("layout"); let logged = Mutex::new(Vec::new()); - download(&pins, &dir, "linux", 3, &|url| server.fetch(url), &|line| { - logged.lock().unwrap().push(line) - }) + download( + &pins, + &dir, + "linux", + 3, + &|url| server.fetch(url), + &GO, + &|line| logged.lock().unwrap().push(line), + ) .unwrap(); assert_eq!( fs::read(dir.join("versions/26.3/26.3.jar")).unwrap(), @@ -156,7 +164,16 @@ fn an_owner_gets_the_official_layout_with_this_systems_libraries() { &|_| {}, ); assert_eq!(found, Ok(Resolved::Folder(dir.clone()))); - download(&pins, &dir, "linux", 3, &|url| server.fetch(url), &|_| {}).unwrap(); + download( + &pins, + &dir, + "linux", + 3, + &|url| server.fetch(url), + &GO, + &|_| {}, + ) + .unwrap(); assert_eq!(server.asked(), Vec::::new()); fs::remove_dir_all(&dir).unwrap(); } @@ -169,7 +186,16 @@ fn macos_skips_what_its_rules_disallow() { asked: Mutex::new(Vec::new()), }; let dir = scratch("osx"); - download(&pins, &dir, "osx", 2, &|url| server.fetch(url), &|_| {}).unwrap(); + download( + &pins, + &dir, + "osx", + 2, + &|url| server.fetch(url), + &GO, + &|_| {}, + ) + .unwrap(); let libraries = dir.join("libraries/org/fake"); assert!(libraries.join("common/1/common-1.jar").is_file()); assert!(!libraries.join("linux").exists()); @@ -182,14 +208,23 @@ fn a_file_that_is_not_the_pinned_one_stops_the_download() { let (pins, mut files) = mojang(); files.insert( "https://piston-data.mojang.com/client.jar".into(), - b"something else".to_vec(), + b"fake client JAR".to_vec(), ); let server = Server { files, asked: Mutex::new(Vec::new()), }; let dir = scratch("bad-jar"); - let error = download(&pins, &dir, "linux", 2, &|url| server.fetch(url), &|_| {}).unwrap_err(); + let error = download( + &pins, + &dir, + "linux", + 2, + &|url| server.fetch(url), + &GO, + &|_| {}, + ) + .unwrap_err(); assert!( error.starts_with("https://piston-data.mojang.com/client.jar is not the one Horizon pins"), "{error}" @@ -209,9 +244,15 @@ fn missing_sounds_are_a_line_in_the_log_not_an_error() { }; let dir = scratch("no-sound"); let logged = Mutex::new(Vec::new()); - download(&pins, &dir, "linux", 1, &|url| server.fetch(url), &|line| { - logged.lock().unwrap().push(line) - }) + download( + &pins, + &dir, + "linux", + 1, + &|url| server.fetch(url), + &GO, + &|line| logged.lock().unwrap().push(line), + ) .unwrap(); assert!(dir.join("versions/26.3/26.3.jar").is_file()); let last = logged.lock().unwrap().last().cloned().unwrap(); @@ -290,6 +331,7 @@ fn java_runtime_lays_out_mojangs_runtime_with_its_executables_and_links() { &dir, 2, &|url| server.fetch(url), + &GO, &|_| {}, ) .unwrap(); @@ -316,6 +358,7 @@ fn java_runtime_lays_out_mojangs_runtime_with_its_executables_and_links() { &dir, 2, &|url| server.fetch(url), + &GO, &|_| {}, ) .unwrap(); @@ -325,8 +368,91 @@ fn java_runtime_lays_out_mojangs_runtime_with_its_executables_and_links() { &dir, 2, &|url| server.fetch(url), + &GO, &|_| {} ) .is_err()); fs::remove_dir_all(&dir).unwrap(); } + +#[test] +fn a_file_of_another_size_than_mojang_lists_is_refused() { + let (pins, mut files) = mojang(); + // A cut-off body: its size says so before its sha1. + files.insert( + "https://piston-data.mojang.com/client.jar".into(), + CLIENT[..4].to_vec(), + ); + let server = Server { + files, + asked: Mutex::new(Vec::new()), + }; + let dir = scratch("size"); + let error = download( + &pins, + &dir, + "linux", + 2, + &|url| server.fetch(url), + &GO, + &|_| {}, + ) + .unwrap_err(); + assert!( + error.ends_with(&format!( + "is 4 bytes, not the {} Mojang lists", + CLIENT.len() + )), + "{error}" + ); + assert!(!dir.join("versions/26.3/26.3.jar").exists()); + fs::remove_dir_all(&dir).unwrap(); +} + +#[test] +fn a_cancelled_download_goes_on_where_it_stopped() { + let (pins, files) = mojang(); + let server = Server { + files, + asked: Mutex::new(Vec::new()), + }; + let dir = scratch("cancel"); + let cancel = AtomicBool::new(false); + // Cancelled once the client jar has arrived: one job, so nothing after it starts. + let fetch = |url: &str| { + let body = server.fetch(url); + if url.ends_with("client.jar") { + cancel.store(true, Relaxed); + } + body + }; + let error = download(&pins, &dir, "linux", 1, &fetch, &cancel, &|_| {}).unwrap_err(); + assert_eq!(error, CANCELLED); + assert_eq!( + fs::read(dir.join("versions/26.3/26.3.jar")).unwrap(), + CLIENT + ); + assert!(!dir.join("libraries").exists()); + // Again: the version file and the jar are kept, only the rest is fetched. + cancel.store(false, Relaxed); + server.asked(); + download( + &pins, + &dir, + "linux", + 1, + &|url| server.fetch(url), + &cancel, + &|_| {}, + ) + .unwrap(); + let asked = server.asked(); + assert!( + !asked + .iter() + .any(|u| u.ends_with("client.jar") || u.ends_with("26.3.json")), + "{asked:#?}" + ); + assert_eq!(asked.len(), 6, "{asked:#?}"); + fs::remove_dir_all(&dir).unwrap(); +} diff --git a/launcher/src/servers_page.rs b/launcher/src/servers_page.rs index e3ed4f441..49ca08ddd 100644 --- a/launcher/src/servers_page.rs +++ b/launcher/src/servers_page.rs @@ -782,7 +782,7 @@ impl Launcher { self.servers.create = None; self.go(Page::Play); if !self.preparing { - self.prepare(); + self.prepare(backend::MinecraftFrom::Install); } } }, From d87b081c3471e2aaaa62db53f4d60430845cbd42 Mon Sep 17 00:00:00 2001 From: Ludovic Marie <136255825+ludovic111@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:21:08 +0100 Subject: [PATCH 8/8] Trusted reviewers triage on GitHub: /close, /reopen, /label, /unlabel One list (.github/trusted-reviewers.txt) now governs /nightly and the triage commands. Co-Authored-By: Claude Sonnet 5.5 --- .github/nightly-reviewers.txt | 7 - .github/trusted-reviewers.txt | 8 + .github/workflows/nightly-command.yml | 4 +- .github/workflows/triage-command.yml | 48 +++++ docs/release-flow.md | 34 +++- tests/test_triage_command.py | 239 +++++++++++++++++++++++ tools/ci/nightly_set.py | 4 +- tools/ci/triage_command.py | 270 ++++++++++++++++++++++++++ 8 files changed, 599 insertions(+), 15 deletions(-) delete mode 100644 .github/nightly-reviewers.txt create mode 100644 .github/trusted-reviewers.txt create mode 100644 .github/workflows/triage-command.yml create mode 100644 tests/test_triage_command.py create mode 100644 tools/ci/triage_command.py diff --git a/.github/nightly-reviewers.txt b/.github/nightly-reviewers.txt deleted file mode 100644 index a2f9606aa..000000000 --- a/.github/nightly-reviewers.txt +++ /dev/null @@ -1,7 +0,0 @@ -# Trusted reviewers: they pick the pull requests the Nightly holds with `/nightly 2640 2642` in any issue or pull -# request (docs/release-flow.md "Nightly channel", tools/ci/nightly_set.py). Ludovic is always allowed and isn't -# listed. One GitHub login per line, with who added it and when. Removing a line revokes that login at once. -# A line with only `paused` stops every reviewer's `/nightly` (Ludovic's still work). -kacpherek # added by Ludovic, 2026-10-27 -rdbtCVS # added by Ludovic, 2026-10-27 -Online5277 # added by Ludovic, 2026-10-27 (his top three by merged pull requests) diff --git a/.github/trusted-reviewers.txt b/.github/trusted-reviewers.txt new file mode 100644 index 000000000..550c528cc --- /dev/null +++ b/.github/trusted-reviewers.txt @@ -0,0 +1,8 @@ +# Trusted reviewers: one list for two things (docs/release-flow.md "Trusted reviewers"). +# - `/nightly 2640 2642` picks the pull requests the Nightly holds (tools/ci/nightly_set.py); +# - `/close`, `/reopen`, `/label`, `/unlabel` triage issues and pull requests (tools/ci/triage_command.py). +# Ludovic is always allowed and isn't listed. One GitHub login per line, with who added it and when. Removing a +# line revokes that login at once. A line with only `paused` stops both for every reviewer (Ludovic's still work). +kacpherek # added by Ludovic, 2026-10-27 +rdbtCVS # added by Ludovic, 2026-10-27 +Online5277 # added by Ludovic, 2026-10-27 (his top three by merged pull requests) diff --git a/.github/workflows/nightly-command.yml b/.github/workflows/nightly-command.yml index 340988d93..aef5c957b 100644 --- a/.github/workflows/nightly-command.yml +++ b/.github/workflows/nightly-command.yml @@ -1,5 +1,5 @@ name: nightly command -# `/nightly …` comments: the trusted reviewers in .github/nightly-reviewers.txt (and Ludovic) pick the pull requests +# `/nightly …` comments: the trusted reviewers in .github/trusted-reviewers.txt (and Ludovic) pick the pull requests # the Nightly holds (docs/release-flow.md "Nightly channel", tools/ci/nightly_set.py). This job only checks who asked, # saves the set on the nightly-state branch, starts `nightly build` and answers in the thread. # @@ -33,7 +33,7 @@ jobs: persist-credentials: false sparse-checkout: | .github/CODEOWNERS - .github/nightly-reviewers.txt + .github/trusted-reviewers.txt tools/ci/nightly_set.py sparse-checkout-cone-mode: false - name: Run the command diff --git a/.github/workflows/triage-command.yml b/.github/workflows/triage-command.yml new file mode 100644 index 000000000..938bbc337 --- /dev/null +++ b/.github/workflows/triage-command.yml @@ -0,0 +1,48 @@ +name: triage command +# `/close`, `/reopen`, `/label`, `/unlabel` comments: the trusted reviewers in .github/trusted-reviewers.txt (and +# Ludovic) triage issues and pull requests without any repository role (docs/release-flow.md "Trusted reviewers", +# tools/ci/triage_command.py). +# +# Why no pull request's code runs here, though the token can write: +# - `issue_comment` always runs this file from main, whoever comments and wherever. +# - It checks out main's script and list only, never a pull request; it reads threads through the API. +# - The comment's text and author reach the script as env vars, never in a shell line. The token stays in the step. +on: + issue_comment: + types: [created] +permissions: + contents: read +jobs: + command: + # Every other comment skips here, before a runner starts. + if: >- + github.event.comment.user.type != 'Bot' && ( + startsWith(github.event.comment.body, '/close') || + startsWith(github.event.comment.body, '/reopen') || + startsWith(github.event.comment.body, '/label') || + startsWith(github.event.comment.body, '/unlabel')) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + issues: write + pull-requests: write + steps: + - uses: actions/checkout@v4 + with: + ref: main + persist-credentials: false + sparse-checkout: | + .github/trusted-reviewers.txt + tools/ci/nightly_set.py + tools/ci/triage_command.py + sparse-checkout-cone-mode: false + - name: Run the command + env: + GH_TOKEN: ${{ github.token }} + BODY: ${{ github.event.comment.body }} + AUTHOR: ${{ github.event.comment.user.login }} + ISSUE: ${{ github.event.issue.number }} + REPO: ${{ github.repository }} + COMMENT_URL: ${{ github.event.comment.html_url }} + run: python3 tools/ci/triage_command.py command diff --git a/docs/release-flow.md b/docs/release-flow.md index 09565bd2e..feadaa0a1 100644 --- a/docs/release-flow.md +++ b/docs/release-flow.md @@ -420,10 +420,36 @@ than main's head and holds something main lacks, otherwise main (`tools/ci/night new publishes nothing. A Nightly can include pull requests not reviewed yet: that is what it is for, playing them before the gate. -### Trusted reviewers pick the pull requests +### Trusted reviewers + +One list, `.github/trusted-reviewers.txt`, governs two things for the people on it (kacpherek, rdbtCVS and +Online5277 since 2026-10-27) and for Ludovic, with no repository role: `/nightly` (below) and triage by comment. + +```text +/close closes an issue as completed; a pull request is closed without merging +/close not planned closes an issue as not planned +/close duplicate of #123 comments "Duplicate of #123", closes as a duplicate +/reopen +/label bug, good first issue existing labels only; never creates one +/unlabel bug +``` + +- **Where.** On the first line of a comment on the issue or pull request (`.github/workflows/triage-command.yml`, + `tools/ci/triage_command.py`). The workflow answers with one comment, `🤖 Triage: , on behalf of @login`, + linking the comment that asked; a refusal answers too. +- **Refused.** Any label with `release` in its name (`release-now`, `release-blocker`, `skip-release`) and + `full-parity`; anything on Ludovic's own issues and pull requests; a merged pull request; a locked thread; more + than 60 actions per reviewer in 24 hours (counted from the workflow's own comments). +- **Safety.** Same as `/nightly`: main's copy of the workflow runs, no pull request's code is checked out or run, + the comment reaches the script as env vars, and the token has `issues` and `pull-requests` write only. +- **Ludovic stays in charge.** A one-line pull request removing a login from the list revokes it as soon as it merges + (the file is his under CODEOWNERS). A line `paused` stops `/nightly` and the triage commands for every reviewer; + his own still work. + +### Trusted reviewers pick the Nightly's pull requests The Nightly is where contributors test pull requests before the gate. The trusted reviewers listed in -`.github/nightly-reviewers.txt` (kacpherek, rdbtCVS and Online5277 since 2026-10-27) and Ludovic pick them with a +`.github/trusted-reviewers.txt` (kacpherek, rdbtCVS and Online5277 since 2026-10-27) and Ludovic pick them with a comment on any issue or pull request (`.github/workflows/nightly-command.yml`, `tools/ci/nightly_set.py`): ```text @@ -458,9 +484,9 @@ comment on any issue or pull request (`.github/workflows/nightly-command.yml`, ` - **Ludovic stays in charge.** - Ludovic's `/nightly clear` or `/nightly remove` always applies. A pull request he removed can only come back when he adds it. - - A one-line pull request removing a login from `.github/nightly-reviewers.txt` revokes it as soon as it merges + - A one-line pull request removing a login from `.github/trusted-reviewers.txt` revokes it as soon as it merges (the file is his under CODEOWNERS). - - A line `paused` in that file stops every reviewer's `/nightly`; his own still work. + - A line `paused` in that file stops every reviewer's `/nightly` (and triage commands); his own still work. - Main, the gate and releases don't change: picks reach only players who turned Nightly on. - **Jobs.** `pick` chooses or assembles the commit and writes the notes (each pull request in it with its title, diff --git a/tests/test_triage_command.py b/tests/test_triage_command.py new file mode 100644 index 000000000..2dbf14592 --- /dev/null +++ b/tests/test_triage_command.py @@ -0,0 +1,239 @@ +import unittest +from datetime import datetime, timezone +from pathlib import Path + +from tools.ci import nightly_set as ns +from tools.ci import triage_command as tc + +ROOT = Path(__file__).resolve().parents[1] +NOW = datetime(2026, 10, 28, 12, tzinfo=timezone.utc) +URL = "https://github.com/ludovic111/horizon/issues/5#issuecomment-1" +LIST = ns.read_reviewers("kacpherek\nrdbtCVS\n") + + +def thread(state="open", user="dev", labels=("bug",), pr=False, merged=False, locked=False): + t = {"state": state, "user": {"login": user}, "labels": [{"name": n} for n in labels], "locked": locked} + if pr: + t["pull_request"] = {"merged_at": "2026-10-27T00:00:00Z" if merged else None} + return t + + +class Fake: + def __init__(self, threads=None, labels=("bug", "good first issue", "release-now", "skip-release"), past=()): + self.threads = {5: thread()} if threads is None else threads + self.labels, self.past, self.calls = list(labels), list(past), [] + + def issue(self, n): + return self.threads.get(n) + + def label(self, name): + return next((l for l in self.labels if l.lower() == name.lower()), None) + + def add_labels(self, n, names): + self.calls.append(("add", n, names)) + + def remove_label(self, n, name): + self.calls.append(("remove", n, name)) + + def close(self, n, reason): + self.calls.append(("close", n, reason)) + + def reopen(self, n): + self.calls.append(("reopen", n)) + + def comment(self, n, text): + self.calls.append(("comment", n, text)) + + def recent_comments(self, since): + return self.past + + +def do(text, api=None, login="kacpherek", reviewers=LIST, n=5): + api = api or Fake() + return tc.run(api, tc.parse(text), login, n, URL, reviewers, NOW), api + + +class ParseTests(unittest.TestCase): + def test_commands_and_case(self): + self.assertEqual(tc.parse("/close"), tc.Command("close", "completed")) + self.assertEqual(tc.parse(" /CLOSE Not Planned "), tc.Command("close", "not_planned")) + self.assertEqual(tc.parse("/close completed"), tc.Command("close", "completed")) + self.assertEqual(tc.parse("/close duplicate of #123"), tc.Command("close", "duplicate", 123)) + self.assertEqual(tc.parse("/Close duplicate of 7"), tc.Command("close", "duplicate", 7)) + self.assertEqual(tc.parse("/reopen"), tc.Command("reopen")) + self.assertEqual(tc.parse("/label bug, good first issue ,bug"), tc.Command("label", labels=("bug", "good first issue"))) + self.assertEqual(tc.parse("/unlabel bug"), tc.Command("unlabel", labels=("bug",))) + + def test_first_line_only(self): + self.assertEqual(tc.parse("/close\n/label release-now"), tc.Command("close", "completed")) + self.assertIsNone(tc.parse("thanks\n/close")) + self.assertEqual(tc.parse("\n\n/reopen\nwhy"), tc.Command("reopen")) + + def test_not_commands(self): + for text in ("", "hello", "/nightly 5", "/closed", "/labels bug", "/closer", "close", "/unlabeled x"): + self.assertIsNone(tc.parse(text), text) + + def test_malformed(self): + for text in ("/close maybe", "/close duplicate of", "/reopen now", "/label", "/label ,", "/unlabel a, b", + "/label " + ",".join(str(i) for i in range(11))): + with self.assertRaises(tc.Refused, msg=text): + tc.parse(text) + + def test_quoting_is_safe(self): + with self.assertRaises(tc.Refused) as e: + tc.parse("/close @ludovic111 [x](http://evil)") + self.assertNotIn("@", str(e.exception)) + self.assertNotIn("](", str(e.exception)) + + +class AllowTests(unittest.TestCase): + def test_listed_and_ludovic(self): + do("/close", login="RDBTcvs") + do("/close", login="ludovic111") + + def test_stranger_refused(self): + with self.assertRaisesRegex(tc.Refused, "Only Ludovic"): + do("/close", login="someone") + + def test_paused(self): + paused = ns.read_reviewers("kacpherek\npaused\n") + with self.assertRaisesRegex(tc.Refused, "paused"): + do("/close", reviewers=paused) + do("/close", reviewers=paused, login="ludovic111") + + def test_the_repos_list_is_the_nightlys(self): + self.assertEqual(ns.REVIEWERS, ".github/trusted-reviewers.txt") + listed = ns.read_reviewers((ROOT / ns.REVIEWERS).read_text()) + self.assertEqual(listed.logins, {"kacpherek", "rdbtcvs", "online5277"}) + self.assertFalse(listed.paused) + + +class ActionTests(unittest.TestCase): + def test_close_issue(self): + text, api = do("/close not planned") + self.assertEqual(api.calls, [("close", 5, "not_planned")]) + self.assertIn("on behalf of @kacpherek", text) + self.assertIn(URL, text) + self.assertTrue(text.startswith("🤖 Triage: ")) + + def test_close_pr_has_no_reason(self): + api = Fake({5: thread(pr=True)}) + text, _ = do("/close", api) + self.assertEqual(api.calls, [("close", 5, None)]) + self.assertIn("without merging", text) + + def test_duplicate(self): + api = Fake({5: thread(), 9: thread()}) + text, _ = do("/close duplicate of #9", api) + self.assertEqual(api.calls, [("comment", 5, "Duplicate of #9"), ("close", 5, "duplicate")]) + self.assertIn("#9", text) + + def test_duplicate_of_missing_or_itself(self): + with self.assertRaisesRegex(tc.Refused, "no #9"): + do("/close duplicate of #9") + with self.assertRaisesRegex(tc.Refused, "itself"): + do("/close duplicate of #5") + + def test_reopen(self): + _, api = do("/reopen", Fake({5: thread("closed")})) + self.assertEqual(api.calls, [("reopen", 5)]) + with self.assertRaisesRegex(tc.Refused, "already open"): + do("/reopen") + with self.assertRaisesRegex(tc.Refused, "already closed"): + do("/close", Fake({5: thread("closed")})) + + def test_label_uses_the_real_name_and_skips_present(self): + _, api = do("/label Good First Issue, BUG") + self.assertEqual(api.calls, [("add", 5, ["good first issue"])]) + with self.assertRaisesRegex(tc.Refused, "already has"): + do("/label bug") + + def test_label_must_exist(self): + with self.assertRaisesRegex(tc.Refused, "no label"): + do("/label nonsense") + + def test_unlabel(self): + _, api = do("/unlabel BUG") + self.assertEqual(api.calls, [("remove", 5, "bug")]) + with self.assertRaisesRegex(tc.Refused, "doesn't have"): + do("/unlabel good first issue") + + +class RefusalTests(unittest.TestCase): + def test_release_and_parity_labels(self): + for name in ("release-now", "Release-Blocker", "skip-release", "full-parity", "prerelease"): + for verb in ("/label", "/unlabel"): + with self.assertRaisesRegex(tc.Refused, "release or parity", msg=name): + do(f"{verb} {name}", Fake(labels=(name,))) + with self.assertRaises(tc.Refused): + do("/label bug, release-now") + + def test_ludovics_threads(self): + for text in ("/close", "/label bug", "/unlabel bug", "/reopen"): + for t in (thread(user="ludovic111", state="closed" if text == "/reopen" else "open"), + thread(user="Ludovic111", pr=True)): + with self.assertRaisesRegex(tc.Refused, "Ludovic's"): + do(text, Fake({5: t})) + + def test_merged_pr(self): + for text in ("/close", "/reopen", "/label bug", "/unlabel bug"): + with self.assertRaisesRegex(tc.Refused, "merged"): + do(text, Fake({5: thread("closed", pr=True, merged=True)})) + + def test_locked(self): + for text in ("/close", "/label good first issue", "/unlabel bug"): + with self.assertRaisesRegex(tc.Refused, "locked"): + do(text, Fake({5: thread(locked=True)})) + + def test_missing_thread(self): + with self.assertRaisesRegex(tc.Refused, "no such"): + do("/close", Fake({})) + + def test_nothing_written_on_refusal(self): + api = Fake({5: thread(user="ludovic111")}) + with self.assertRaises(tc.Refused): + do("/close", api) + self.assertEqual(api.calls, []) + + +def past(login, n, who="github-actions[bot]"): + return [{"user": who, "body": f"🤖 Triage: closed as completed, on behalf of @{login} ([their comment](u))."} + for _ in range(n)] + + +class LimitTests(unittest.TestCase): + def test_sixty_a_day(self): + with self.assertRaisesRegex(tc.Refused, "60"): + do("/close", Fake(past=past("kacpherek", 60))) + do("/close", Fake(past=past("kacpherek", 59))) + + def test_other_reviewers_and_forged_comments_dont_count(self): + do("/close", Fake(past=past("rdbtcvs", 80))) + do("/close", Fake(past=past("kacpherek", 80, who="someone"))) + + def test_refusals_dont_count(self): + refusals = [{"user": "github-actions[bot]", "body": "🤖 Triage: refused, nope (asked by @kacpherek)"}] * 70 + do("/close", Fake(past=refusals)) + + def test_counts_case_insensitively(self): + with self.assertRaises(tc.Refused): + do("/close", Fake(past=past("kacpherek", 60)), login="Kacpherek") + + def test_ludovic_is_unlimited(self): + do("/close", Fake(past=past("ludovic111", 100)), login="ludovic111") + + +class WorkflowTests(unittest.TestCase): + def test_workflow_is_safe(self): + text = (ROOT / ".github/workflows/triage-command.yml").read_text() + run_lines = [l for l in text.splitlines() if l.strip().startswith("run:")] + self.assertEqual(run_lines, [" run: python3 tools/ci/triage_command.py command"]) + self.assertIn("persist-credentials: false", text) + self.assertNotIn("pull_request_target", text) + for perm in ("issues: write", "pull-requests: write", "contents: read"): + self.assertIn(perm, text) + self.assertNotIn("contents: write", text) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/ci/nightly_set.py b/tools/ci/nightly_set.py index a192993d4..8122a8d1c 100644 --- a/tools/ci/nightly_set.py +++ b/tools/ci/nightly_set.py @@ -4,7 +4,7 @@ nightly_set.py command one `/nightly …` comment (nightly-command.yml): checks who asked, updates the set, starts the build and answers in the thread -A comment by Ludovic or by a login in .github/nightly-reviewers.txt, on any issue or pull request: +A comment by Ludovic or by a login in .github/trusted-reviewers.txt, on any issue or pull request: /nightly 2640 2642 the set becomes these pull requests, merged onto main in this order; builds now /nightly add 2650 adds one, or takes its newest head again; builds now @@ -35,7 +35,7 @@ from pathlib import Path from typing import Callable OWNER = "ludovic111" -REVIEWERS = ".github/nightly-reviewers.txt" +REVIEWERS = ".github/trusted-reviewers.txt" CODEOWNERS = ".github/CODEOWNERS" # What CODEOWNERS guards today, refused even if the file moved: what builds, signs and checks releases. ALWAYS_GUARDED = ["/.github/", "/tools/ci/", "/crates/horizon-release-sign/", "/crates/horizon-server/src/update.rs", diff --git a/tools/ci/triage_command.py b/tools/ci/triage_command.py new file mode 100644 index 000000000..6639bd8fc --- /dev/null +++ b/tools/ci/triage_command.py @@ -0,0 +1,270 @@ +#!/usr/bin/env python3 +"""Trusted reviewers triage issues and pull requests with comment commands (docs/release-flow.md "Trusted reviewers"). + + triage_command.py command one comment (triage-command.yml): checks who asked, acts, answers in the thread + +A comment by Ludovic or by a login in .github/trusted-reviewers.txt, on the first line: + + /close [completed|not planned] closes an issue (a pull request: closes it without merging) + /close duplicate of #123 comments "Duplicate of #123" and closes it as a duplicate + /reopen + /label bug, good first issue existing labels only; never creates one + /unlabel bug + +Refused with a one-line answer: a label with `release` in it or `full-parity`; anything by Ludovic (his issues and +pull requests); a merged pull request; a locked thread; more than 60 actions per reviewer in 24 hours. No code of any +pull request is read or run: only the comment's text, its author and the API's answers, which reach this script as +env vars. +""" +from __future__ import annotations + +import json +import os +import re +import subprocess +import sys +from dataclasses import dataclass +from datetime import datetime, timedelta, timezone +from pathlib import Path +from typing import Callable + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import nightly_set as ns # noqa: E402 (the same list, owner and paused rule as /nightly) + +MAX_PER_DAY = 60 +PREFIX = "🤖 Triage: " +COMMANDS = ("close", "reopen", "label", "unlabel") +REASONS = {"completed": "completed", "not planned": "not_planned", "not_planned": "not_planned"} +DUPLICATE = re.compile(r"^(?:as\s+)?(?:a\s+)?duplicate\s+of\s+#?(\d{1,7})$", re.I) +USAGE = ("Use `/close`, `/close not planned`, `/close duplicate of #123`, `/reopen`, `/label bug, good first issue` " + "or `/unlabel bug`.") + + +class Refused(Exception): + """A command that changes nothing; its text is the answer.""" + + +@dataclass +class Command: + verb: str # close, reopen, label, unlabel + reason: str | None = None # close: completed, not_planned, duplicate + original: int | None = None # close duplicate of #N + labels: tuple[str, ...] = () + + +def safe(text: str) -> str: + """Text of a comment, fit to quote back: no markdown, no mentions.""" + return re.sub(r"[^\w#:. -]", "", text)[:60].strip() or "?" + + +def parse(body: str) -> Command | None: + """The command on a comment's first line; None when it isn't one. Raises Refused when it is one, but wrong.""" + line = next((line.strip() for line in body.splitlines() if line.strip()), "") + m = re.match(r"^/(\w+)(?:\s+(.*))?$", line) + if not m or m.group(1).lower() not in COMMANDS: + return None + verb, rest = m.group(1).lower(), (m.group(2) or "").strip() + if verb == "reopen": + if rest: + raise Refused(f"`/reopen` takes no arguments. {USAGE}") + return Command("reopen") + if verb == "close": + if not rest: + return Command("close", "completed") + dup = DUPLICATE.match(rest) + if dup: + return Command("close", "duplicate", int(dup.group(1))) + reason = REASONS.get(" ".join(rest.lower().split())) + if not reason: + raise Refused(f"`{safe(rest)}` isn't a way to close. {USAGE}") + return Command("close", reason) + names = tuple(dict.fromkeys(n.strip() for n in rest.split(",") if n.strip())) + if not names: + raise Refused(f"`/{verb}` needs a label name. {USAGE}") + if verb == "unlabel" and len(names) > 1: + raise Refused(f"`/unlabel` takes one label. {USAGE}") + if len(names) > 10: + raise Refused("At most 10 labels at once.") + return Command(verb, labels=names) + + +def label_refusal(name: str) -> str | None: + low = name.lower() + if "release" in low or low == "full-parity": + return f"`{safe(name)}` is a release or parity label: only Ludovic's side touches those." + return None + + +def authorize(login: str, reviewers: ns.Reviewers) -> None: + """Ludovic always; a listed reviewer unless the list is paused; nobody else.""" + if ns.is_owner(login): + return + if login.lower() not in reviewers.logins: + raise Refused(f"Only Ludovic and the trusted reviewers listed in `{ns.REVIEWERS}` can triage this way.") + if reviewers.paused: + raise Refused(f"Triage commands are paused: Ludovic stopped them in `{ns.REVIEWERS}`.") + + +def target_refusal(issue: dict, cmd: Command, login: str) -> str | None: + """Why this thread can't be acted on (None when it can). `issue` is GitHub's issue object.""" + is_pr = "pull_request" in issue + kind = "pull request" if is_pr else "issue" + if not ns.is_owner(login) and ns.is_owner((issue.get("user") or {}).get("login", "")): + return f"This {kind} is Ludovic's: triage commands don't touch it." + if is_pr and (issue["pull_request"] or {}).get("merged_at"): + return "That pull request is already merged." + if issue.get("locked"): + return f"This {kind} is locked: triage commands don't act on it." + closed = issue.get("state") == "closed" + if cmd.verb == "close" and closed: + return f"This {kind} is already closed." + if cmd.verb == "reopen" and not closed: + return f"This {kind} is already open." + return None + + +def names_of(issue: dict) -> set[str]: + return {str(l.get("name", "") if isinstance(l, dict) else l).lower() for l in issue.get("labels", [])} + + +# --- GitHub ------------------------------------------------------------------------------------------------------ + + +def gh(*args: str, check: bool = True) -> str: + r = subprocess.run(["gh", *args], capture_output=True, text=True) + if check and r.returncode != 0: + raise RuntimeError(f"gh {args[0]} {args[1] if len(args) > 1 else ''}: {r.stderr.strip()[:300]}") + return r.stdout.strip() if r.returncode == 0 else "" + + +class Api: + """The few GitHub calls this needs; tests replace it.""" + + def __init__(self, repo: str): + self.repo = repo + + def issue(self, n: int) -> dict | None: + raw = gh("api", f"repos/{self.repo}/issues/{n}", check=False) + return json.loads(raw) if raw else None + + def label(self, name: str) -> str | None: + """The label's real name, or None when the repository has no such label.""" + raw = gh("api", f"repos/{self.repo}/labels/{name.replace('/', '%2F').replace(' ', '%20')}", check=False) + return json.loads(raw)["name"] if raw else None + + def add_labels(self, n: int, names: list[str]) -> None: + args = ["api", f"repos/{self.repo}/issues/{n}/labels"] + for name in names: + args += ["-f", f"labels[]={name}"] + gh(*args) + + def remove_label(self, n: int, name: str) -> None: + gh("api", "-X", "DELETE", f"repos/{self.repo}/issues/{n}/labels/{name.replace(' ', '%20')}") + + def close(self, n: int, reason: str | None) -> None: + args = ["api", "-X", "PATCH", f"repos/{self.repo}/issues/{n}", "-f", "state=closed"] + if reason: + args += ["-f", f"state_reason={reason}"] + gh(*args) + + def reopen(self, n: int) -> None: + gh("api", "-X", "PATCH", f"repos/{self.repo}/issues/{n}", "-f", "state=open") + + def comment(self, n: int, text: str) -> None: + gh("api", f"repos/{self.repo}/issues/{n}/comments", "-f", f"body={text}") + + def recent_comments(self, since: str) -> list[dict]: + """Comments of the whole repository since a time (ISO 8601), as {user, body}.""" + raw = gh("api", "--paginate", f"repos/{self.repo}/issues/comments?since={since}&per_page=100", "--jq", + '.[] | {user: .user.login, body: .body}') + return [json.loads(line) for line in raw.splitlines() if line.strip()] + + +def actions_by(api, login: str, now: datetime) -> int: + """Actions this workflow answered for `login` in the last 24 h, counted from its own comments.""" + since = (now - timedelta(hours=24)).strftime("%Y-%m-%dT%H:%M:%SZ") + tag = f"on behalf of @{login.lower()}" + return sum(1 for c in api.recent_comments(since) + if c["user"] == "github-actions[bot]" and c["body"].startswith(PREFIX) + and c["body"].split("\n", 1)[0].lower().count(tag)) + + +def say(action: str, login: str, url: str) -> str: + return f"{PREFIX}{action}, on behalf of @{login} ([their comment]({url}))." + + +def run(api, cmd: Command, login: str, number: int, url: str, reviewers: ns.Reviewers, now: datetime) -> str: + """Does the command; returns the comment to post. Raises Refused for a refusal.""" + authorize(login, reviewers) + for name in cmd.labels: + if (why := label_refusal(name)): + raise Refused(why) + issue = api.issue(number) + if issue is None: + raise Refused("There is no such issue or pull request.") + if (why := target_refusal(issue, cmd, login)): + raise Refused(why) + if cmd.original is not None: + if cmd.original == number: + raise Refused("An item cannot be a duplicate of itself.") + if api.issue(cmd.original) is None: + raise Refused(f"There is no #{cmd.original} in Horizon.") + if not ns.is_owner(login) and actions_by(api, login, now) >= MAX_PER_DAY: + raise Refused(f"You used your {MAX_PER_DAY} triage actions of the last 24 hours. Try again later.") + is_pr = "pull_request" in issue + if cmd.verb == "reopen": + api.reopen(number) + return say("reopened", login, url) + if cmd.verb == "close": + if cmd.reason == "duplicate": + api.comment(number, f"Duplicate of #{cmd.original}") + api.close(number, None if is_pr else "duplicate") + return say(f"closed as a duplicate of #{cmd.original}", login, url) + api.close(number, None if is_pr else cmd.reason) + how = "closed without merging" if is_pr else f"closed as {cmd.reason.replace('_', ' ')}" + return say(how, login, url) + have = names_of(issue) + real: list[str] = [] + for name in cmd.labels: + found = api.label(name) + if found is None: + raise Refused(f"There is no label `{safe(name)}` in Horizon: triage doesn't create new ones.") + if (why := label_refusal(found)): + raise Refused(why) + real.append(found) + if cmd.verb == "label": + new = [n for n in real if n.lower() not in have] + if not new: + raise Refused("It already has " + ("that label." if len(real) == 1 else "those labels.")) + api.add_labels(number, new) + return say("added " + ", ".join(f"`{n}`" for n in new), login, url) + name = real[0] + if name.lower() not in have: + raise Refused(f"It doesn't have `{name}`.") + api.remove_label(number, name) + return say(f"removed `{name}`", login, url) + + +def command() -> int: + body, login = os.environ["BODY"], os.environ["AUTHOR"] + number, url, repo = int(os.environ["ISSUE"]), os.environ.get("COMMENT_URL", ""), os.environ["REPO"] + api: Api = Api(repo) + try: + cmd = parse(body) + if cmd is None: + return 0 + reviewers = ns.read_reviewers(Path(ns.REVIEWERS).read_text()) + text = run(api, cmd, login, number, url, reviewers, datetime.now(timezone.utc)) + except Refused as e: + text = f"{PREFIX}refused, {e} (asked by @{safe(login)})" + except RuntimeError as e: + print(e, file=sys.stderr) + text = f"{PREFIX}GitHub refused that, nothing more was done (asked by @{safe(login)})." + api.comment(number, text) + return 0 + + +if __name__ == "__main__": + if sys.argv[1:] != ["command"]: + sys.exit(__doc__) + sys.exit(command())