From 0014807a7d13350e6163a77b4c00e49d30c04821 Mon Sep 17 00:00:00 2001 From: TestingPlant <44930139+TestingPlant@users.noreply.github.com> Date: Tue, 21 Jan 2025 03:40:40 +0000 Subject: [PATCH 1/2] refactor: remove events with RuntimeLifetime This removes ChatMessage and Command --- Cargo.lock | 1 + crates/hyperion-command/Cargo.toml | 1 + crates/hyperion-command/src/system.rs | 89 +++++++++---------- crates/hyperion/src/simulation/event.rs | 12 +-- crates/hyperion/src/simulation/handlers.rs | 36 +------- .../hyperion/src/storage/event/queue/mod.rs | 2 - events/tag/src/module/chat.rs | 66 ++++++++------ 7 files changed, 86 insertions(+), 121 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5393ca2c..7c382686 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2876,6 +2876,7 @@ dependencies = [ "hyperion-utils", "indexmap", "tracing", + "valence_protocol", ] [[package]] diff --git a/crates/hyperion-command/Cargo.toml b/crates/hyperion-command/Cargo.toml index e9048e01..ca97703a 100644 --- a/crates/hyperion-command/Cargo.toml +++ b/crates/hyperion-command/Cargo.toml @@ -12,6 +12,7 @@ hyperion = { workspace = true } hyperion-utils = { workspace = true } indexmap = { workspace = true } tracing = { workspace = true } +valence_protocol = { workspace = true } [lints] workspace = true diff --git a/crates/hyperion-command/src/system.rs b/crates/hyperion-command/src/system.rs index db2b0a81..d8ec637a 100644 --- a/crates/hyperion-command/src/system.rs +++ b/crates/hyperion-command/src/system.rs @@ -1,16 +1,17 @@ use std::fmt::Write; use flecs_ecs::{ - core::{EntityViewGet, QueryBuilderImpl, SystemAPI, TermBuilderImpl, World, WorldGet}, - macros::{Component, system}, + core::{World, WorldGet}, + macros::Component, prelude::Module, }; use hyperion::{ net::agnostic, - simulation::{event, handlers::PacketSwitchQuery, packet::HandlerRegistry}, - storage::{CommandCompletionRequest, EventQueue}, + simulation::{handlers::PacketSwitchQuery, packet::HandlerRegistry}, + storage::CommandCompletionRequest, }; use hyperion_utils::LifetimeHandle; +use valence_protocol::packets::play::CommandExecutionC2s; use crate::component::CommandRegistry; @@ -19,55 +20,51 @@ pub struct CommandSystemModule; impl Module for CommandSystemModule { fn module(world: &World) { - system!( - "execute_command", - world, - &mut EventQueue($), - &CommandRegistry($) - ) - .each_iter(|it, _, (event_queue, registry)| { - let system = it.system(); - - let world = it.world(); - for event::Command { raw, by } in event_queue.drain() { - let raw = raw.get(); - let Some(first_word) = raw.split_whitespace().next() else { - tracing::warn!("command is empty"); - continue; - }; - - let Some(command) = registry.commands.get(first_word) else { - tracing::debug!("command {first_word} not found"); - - let mut msg = String::new(); - write!(&mut msg, "§cAvailable commands: §r[").unwrap(); - - for w in registry.get_permitted(&world, by).intersperse(", ") { - write!(&mut msg, "{w}").unwrap(); - } + world.get::<&mut HandlerRegistry>(|registry| { + registry.add_handler(Box::new( + |pkt: &CommandExecutionC2s<'_>, + _: &dyn LifetimeHandle<'_>, + query: &mut PacketSwitchQuery<'_>| { + let raw = pkt.command.0; + let by = query.id; + let Some(first_word) = raw.split_whitespace().next() else { + tracing::warn!("command is empty"); + return Ok(()); + }; - write!(&mut msg, "]").unwrap(); + query.world.get::<&CommandRegistry>(|command_registry| { + if let Some(command) = command_registry.commands.get(first_word) { + tracing::debug!("executing command {first_word}"); - let chat = agnostic::chat(msg); + let command = command.on_execute; + command(raw, query.system, by); + } else { + tracing::debug!("command {first_word} not found"); - world.get::<&hyperion::net::Compose>(|compose| { - by.entity_view(world) - .get::<&hyperion::net::ConnectionId>(|stream| { - compose.unicast(&chat, *stream, system).unwrap(); - }); - }); + let mut msg = String::new(); + write!(&mut msg, "§cAvailable commands: §r[").unwrap(); - continue; - }; + for w in command_registry + .get_permitted(query.world, by) + .intersperse(", ") + { + write!(&mut msg, "{w}").unwrap(); + } - tracing::debug!("executing command {first_word}"); + write!(&mut msg, "]").unwrap(); - let command = command.on_execute; - command(raw, system, by); - } - }); + let chat = agnostic::chat(msg); - world.get::<&mut HandlerRegistry>(|registry| { + query + .compose + .unicast(&chat, query.io_ref, query.system) + .unwrap(); + } + }); + + Ok(()) + }, + )); registry.add_handler(Box::new( |completion: &CommandCompletionRequest<'_>, _: &dyn LifetimeHandle<'_>, diff --git a/crates/hyperion/src/simulation/event.rs b/crates/hyperion/src/simulation/event.rs index 592c0def..ffd0e5ab 100644 --- a/crates/hyperion/src/simulation/event.rs +++ b/crates/hyperion/src/simulation/event.rs @@ -3,7 +3,7 @@ use derive_more::Constructor; use flecs_ecs::{core::Entity, macros::Component}; use glam::{IVec3, Vec3}; -use hyperion_utils::{Lifetime, RuntimeLifetime}; +use hyperion_utils::Lifetime; use valence_generated::block::BlockState; use valence_protocol::{ Hand, ItemStack, @@ -27,11 +27,6 @@ pub struct ItemInteract { pub sequence: i32, } -pub struct ChatMessage { - pub msg: RuntimeLifetime<&'static str>, - pub by: Entity, -} - #[derive(Debug)] pub struct SetSkin { pub skin: PlayerSkin, @@ -122,11 +117,6 @@ pub struct PostureUpdate { pub state: Posture, } -pub struct Command { - pub raw: RuntimeLifetime<&'static str>, - pub by: Entity, -} - pub struct BlockInteract {} #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/crates/hyperion/src/simulation/handlers.rs b/crates/hyperion/src/simulation/handlers.rs index af75e287..dc740ad1 100644 --- a/crates/hyperion/src/simulation/handlers.rs +++ b/crates/hyperion/src/simulation/handlers.rs @@ -8,7 +8,7 @@ use anyhow::bail; use flecs_ecs::core::{Entity, EntityView, World}; use geometry::aabb::Aabb; use glam::{IVec3, Vec3}; -use hyperion_utils::{EntityExt, LifetimeHandle, RuntimeLifetime}; +use hyperion_utils::{EntityExt, LifetimeHandle}; use tracing::{info, instrument, warn}; use valence_generated::{ block::{BlockKind, BlockState, PropName}, @@ -200,24 +200,6 @@ fn position_and_on_ground( Ok(()) } -fn chat_command<'a>( - pkt: &play::CommandExecutionC2s<'a>, - handle: &dyn LifetimeHandle<'a>, - query: &mut PacketSwitchQuery<'_>, -) -> anyhow::Result<()> { - let command = RuntimeLifetime::new(pkt.command.0, handle); - - query.events.push( - event::Command { - raw: command, - by: query.id, - }, - query.world, - ); - - Ok(()) -} - fn hand_swing( &packet: &play::HandSwingC2s, _: &dyn LifetimeHandle<'_>, @@ -519,20 +501,6 @@ fn click_slot( Ok(()) } -fn chat_message<'a>( - pkt: &play::ChatMessageC2s<'a>, - handle: &dyn LifetimeHandle<'a>, - query: &mut PacketSwitchQuery<'_>, -) -> anyhow::Result<()> { - let msg = RuntimeLifetime::new(pkt.message.0, handle); - - query - .events - .push(event::ChatMessage { msg, by: query.id }, query.world); - - Ok(()) -} - pub fn request_command_completions<'a>( play::RequestCommandCompletionsC2s { transaction_id, @@ -573,11 +541,9 @@ pub fn client_status( } pub fn add_builtin_handlers(registry: &mut HandlerRegistry) { - registry.add_handler(Box::new(chat_message)); registry.add_handler(Box::new(click_slot)); registry.add_handler(Box::new(client_command)); registry.add_handler(Box::new(client_status)); - registry.add_handler(Box::new(chat_command)); registry.add_handler(Box::new(creative_inventory_action)); registry.add_handler(Box::new(full)); registry.add_handler(Box::new(hand_swing)); diff --git a/crates/hyperion/src/storage/event/queue/mod.rs b/crates/hyperion/src/storage/event/queue/mod.rs index 599ec6b7..e8e3522e 100644 --- a/crates/hyperion/src/storage/event/queue/mod.rs +++ b/crates/hyperion/src/storage/event/queue/mod.rs @@ -48,8 +48,6 @@ define_events! { event::ItemInteract, event::SetSkin, event::AttackEntity, - event::ChatMessage, - event::Command, event::DestroyBlock, event::ItemDropEvent, event::PlaceBlock, diff --git a/events/tag/src/module/chat.rs b/events/tag/src/module/chat.rs index bb43b248..656f102c 100644 --- a/events/tag/src/module/chat.rs +++ b/events/tag/src/module/chat.rs @@ -1,15 +1,17 @@ use flecs_ecs::{ - core::{EntityViewGet, QueryBuilderImpl, SystemAPI, TableIter, TermBuilderImpl, World, flecs}, - macros::{Component, system}, + core::{EntityViewGet, World, WorldGet, flecs}, + macros::Component, prelude::Module, }; use hyperion::{ - net::ConnectionId, - simulation::{Name, Player, Position, event}, - storage::EventQueue, - valence_protocol::{packets::play, text::IntoText}, + simulation::{Name, Player, handlers::PacketSwitchQuery, packet::HandlerRegistry}, + valence_protocol::{ + packets::play::{self, ChatMessageC2s}, + text::IntoText, + }, }; use hyperion_rank_tree::Team; +use hyperion_utils::LifetimeHandle; use tracing::info_span; const CHAT_COOLDOWN_SECONDS: i64 = 15; // 15 seconds @@ -32,42 +34,45 @@ impl Module for ChatModule { .component::() .add_trait::<(flecs::With, ChatCooldown)>(); - system!("handle_chat_messages", world, &mut EventQueue($), &hyperion::net::Compose($)) - .multi_threaded() - .each_iter(move |it: TableIter<'_, false>, _: usize, (event_queue, compose): (&mut EventQueue, &hyperion::net::Compose)| { - let world = it.world(); - let span = info_span!("handle_chat_messages"); - let _enter = span.enter(); + world.get::<&mut HandlerRegistry>(|registry| { + registry.add_handler(Box::new( + |packet: &ChatMessageC2s<'_>, + _: &dyn LifetimeHandle<'_>, + query: &mut PacketSwitchQuery<'_>| { + let span = info_span!("handle_chat_messages"); + let _enter = span.enter(); - let system = it.system(); - - let current_tick = compose.global().tick; - - for event::ChatMessage { msg, by } in event_queue.drain() { - let msg = msg.get(); - let by = world.entity_from_id(by); + let current_tick = query.compose.global().tick; + let by = query.view; // todo: we should not need this; death should occur such that this is always valid if !by.is_alive() { - continue; + return Ok(()); } // Check cooldown // todo: try_get if entity is dead/not found what will happen? - by.get::<(&Name, &Position, &mut ChatCooldown, &ConnectionId, &Team)>(|(name, position, cooldown, io, team)| { + by.get::<(&Name, &mut ChatCooldown, &Team)>(|(name, cooldown, team)| { // Check if player is still on cooldown if cooldown.expires > current_tick { let remaining_ticks = cooldown.expires - current_tick; let remaining_secs = remaining_ticks as f32 / 20.0; - let cooldown_msg = format!("§cPlease wait {remaining_secs:.2} seconds before sending another message").into_cow_text(); + let cooldown_msg = format!( + "§cPlease wait {remaining_secs:.2} seconds before sending another \ + message" + ) + .into_cow_text(); let packet = play::GameMessageS2c { chat: cooldown_msg, overlay: false, }; - compose.unicast(&packet, *io, system).unwrap(); + query + .compose + .unicast(&packet, query.io_ref, query.system) + .unwrap(); return; } @@ -80,19 +85,26 @@ impl Module for ChatModule { Team::Yellow => "§e", }; + let msg = packet.message; + let chat = format!("§8<{color_prefix}{name}§8>§r {msg}").into_cow_text(); let packet = play::GameMessageS2c { chat, overlay: false, }; - let center = position.to_chunk(); + let center = query.position.to_chunk(); - compose.broadcast_local(&packet, center, system) + query + .compose + .broadcast_local(&packet, center, query.system) .send() .unwrap(); }); - } - }); + + Ok(()) + }, + )); + }); } } From 6450a1655b6ecb6ea57d8f953c985e0124bfb153 Mon Sep 17 00:00:00 2001 From: TestingPlant <44930139+TestingPlant@users.noreply.github.com> Date: Tue, 21 Jan 2025 04:04:18 +0000 Subject: [PATCH 2/2] refactor: remove RuntimeLifetime --- Cargo.lock | 3 - crates/hyperion-command/Cargo.toml | 1 - crates/hyperion-command/src/system.rs | 9 +- crates/hyperion-gui/Cargo.toml | 1 - crates/hyperion-gui/src/lib.rs | 5 +- crates/hyperion-item/Cargo.toml | 1 - crates/hyperion-item/src/lib.rs | 5 +- crates/hyperion-respawn/src/lib.rs | 6 +- crates/hyperion-utils/src/lifetime.rs | 147 +-------------------- crates/hyperion/src/ingress/mod.rs | 5 +- crates/hyperion/src/net/mod.rs | 4 - crates/hyperion/src/simulation/handlers.rs | 40 ++---- crates/hyperion/src/simulation/packet.rs | 50 +++---- events/tag/src/module/attack.rs | 6 +- events/tag/src/module/chat.rs | 5 +- 15 files changed, 35 insertions(+), 253 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7c382686..32999eaa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2873,7 +2873,6 @@ version = "0.1.0" dependencies = [ "flecs_ecs", "hyperion", - "hyperion-utils", "indexmap", "tracing", "valence_protocol", @@ -2917,7 +2916,6 @@ dependencies = [ "flecs_ecs", "hyperion", "hyperion-inventory", - "hyperion-utils", "serde", "tracing", "valence_protocol", @@ -2945,7 +2943,6 @@ dependencies = [ "flecs_ecs", "hyperion", "hyperion-inventory", - "hyperion-utils", "valence_protocol", ] diff --git a/crates/hyperion-command/Cargo.toml b/crates/hyperion-command/Cargo.toml index ca97703a..352d24fc 100644 --- a/crates/hyperion-command/Cargo.toml +++ b/crates/hyperion-command/Cargo.toml @@ -9,7 +9,6 @@ publish = false [dependencies] flecs_ecs = { workspace = true } hyperion = { workspace = true } -hyperion-utils = { workspace = true } indexmap = { workspace = true } tracing = { workspace = true } valence_protocol = { workspace = true } diff --git a/crates/hyperion-command/src/system.rs b/crates/hyperion-command/src/system.rs index d8ec637a..37608267 100644 --- a/crates/hyperion-command/src/system.rs +++ b/crates/hyperion-command/src/system.rs @@ -10,7 +10,6 @@ use hyperion::{ simulation::{handlers::PacketSwitchQuery, packet::HandlerRegistry}, storage::CommandCompletionRequest, }; -use hyperion_utils::LifetimeHandle; use valence_protocol::packets::play::CommandExecutionC2s; use crate::component::CommandRegistry; @@ -22,9 +21,7 @@ impl Module for CommandSystemModule { fn module(world: &World) { world.get::<&mut HandlerRegistry>(|registry| { registry.add_handler(Box::new( - |pkt: &CommandExecutionC2s<'_>, - _: &dyn LifetimeHandle<'_>, - query: &mut PacketSwitchQuery<'_>| { + |pkt: &CommandExecutionC2s<'_>, query: &mut PacketSwitchQuery<'_>| { let raw = pkt.command.0; let by = query.id; let Some(first_word) = raw.split_whitespace().next() else { @@ -66,9 +63,7 @@ impl Module for CommandSystemModule { }, )); registry.add_handler(Box::new( - |completion: &CommandCompletionRequest<'_>, - _: &dyn LifetimeHandle<'_>, - query: &mut PacketSwitchQuery<'_>| { + |completion: &CommandCompletionRequest<'_>, query: &mut PacketSwitchQuery<'_>| { let input = completion.query; // should be in form "/{command}" diff --git a/crates/hyperion-gui/Cargo.toml b/crates/hyperion-gui/Cargo.toml index d4f68dd0..15f7e74c 100644 --- a/crates/hyperion-gui/Cargo.toml +++ b/crates/hyperion-gui/Cargo.toml @@ -8,7 +8,6 @@ anyhow = { workspace = true } flecs_ecs = { workspace = true } hyperion = { workspace = true } hyperion-inventory = { workspace = true } -hyperion-utils = { workspace = true } serde = { version = "1.0", features = ["derive"] } valence_protocol = { workspace = true } tracing = { workspace = true } diff --git a/crates/hyperion-gui/src/lib.rs b/crates/hyperion-gui/src/lib.rs index a3707a6e..edcd5ab8 100644 --- a/crates/hyperion-gui/src/lib.rs +++ b/crates/hyperion-gui/src/lib.rs @@ -13,7 +13,6 @@ use hyperion::{ }, }; use hyperion_inventory::{Inventory, InventoryState, OpenInventory}; -use hyperion_utils::LifetimeHandle; use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, Serialize, Deserialize)] @@ -68,9 +67,7 @@ impl Gui { world.get::<&mut HandlerRegistry>(|registry| { let items = self.items.clone(); registry.add_handler(Box::new( - move |event: &ClickSlotC2s<'_>, - _: &dyn LifetimeHandle<'_>, - query: &mut PacketSwitchQuery<'_>| { + move |event: &ClickSlotC2s<'_>, query: &mut PacketSwitchQuery<'_>| { let system = query.system; let world = system.world(); let button = event.mode; diff --git a/crates/hyperion-item/Cargo.toml b/crates/hyperion-item/Cargo.toml index 1510c3d7..6bebea99 100644 --- a/crates/hyperion-item/Cargo.toml +++ b/crates/hyperion-item/Cargo.toml @@ -12,7 +12,6 @@ bytemuck = "1.19.0" valence_protocol = { workspace = true } hyperion = { workspace = true } hyperion-inventory = { workspace = true } -hyperion-utils = { workspace = true } derive_more = { workspace = true } [lints] diff --git a/crates/hyperion-item/src/lib.rs b/crates/hyperion-item/src/lib.rs index 9b49d414..7b3a78cf 100644 --- a/crates/hyperion-item/src/lib.rs +++ b/crates/hyperion-item/src/lib.rs @@ -8,7 +8,6 @@ use hyperion::{ simulation::{handlers::PacketSwitchQuery, packet::HandlerRegistry}, storage::{EventFn, InteractEvent}, }; -use hyperion_utils::LifetimeHandle; use valence_protocol::nbt; pub mod builder; @@ -28,9 +27,7 @@ impl Module for ItemModule { world.get::<&mut HandlerRegistry>(|registry| { registry.add_handler(Box::new( - |event: &InteractEvent, - _: &dyn LifetimeHandle<'_>, - query: &mut PacketSwitchQuery<'_>| { + |event: &InteractEvent, query: &mut PacketSwitchQuery<'_>| { let world = query.world; let inventory = &mut *query.inventory; diff --git a/crates/hyperion-respawn/src/lib.rs b/crates/hyperion-respawn/src/lib.rs index 2ddc4f78..07038fd2 100644 --- a/crates/hyperion-respawn/src/lib.rs +++ b/crates/hyperion-respawn/src/lib.rs @@ -16,7 +16,7 @@ use hyperion::{ Pitch, Position, Uuid, Xp, Yaw, }, }; -use hyperion_utils::{EntityExt, LifetimeHandle}; +use hyperion_utils::EntityExt; #[derive(Component)] pub struct RespawnModule; @@ -25,9 +25,7 @@ impl Module for RespawnModule { fn module(world: &World) { world.get::<&mut HandlerRegistry>(|registry| { registry.add_handler(Box::new( - |event: &ClientStatusEvent, - _: &dyn LifetimeHandle<'_>, - query: &mut PacketSwitchQuery<'_>| { + |event: &ClientStatusEvent, query: &mut PacketSwitchQuery<'_>| { if event.status == ClientStatusCommand::RequestStats { return Ok(()); } diff --git a/crates/hyperion-utils/src/lifetime.rs b/crates/hyperion-utils/src/lifetime.rs index 313d2f0f..e89e3bf7 100644 --- a/crates/hyperion-utils/src/lifetime.rs +++ b/crates/hyperion-utils/src/lifetime.rs @@ -1,7 +1,4 @@ -use std::{ - mem::{ManuallyDrop, size_of}, - sync::atomic::{AtomicUsize, Ordering}, -}; +use std::mem::{ManuallyDrop, size_of}; /// # Safety /// Same safety requirements as [`std::mem::transmute`]. In addition, both types must have the same @@ -68,145 +65,3 @@ hyperion_packet_macros::for_each_lifetime_play_c2s_packet! { type WithLifetime<'a> = PACKET<'a>; } } - -pub struct RuntimeLifetime { - value: T, - references: *const AtomicUsize, -} - -impl RuntimeLifetime { - #[must_use] - pub fn new<'a>( - value: T, - handle: &dyn LifetimeHandle<'a>, - ) -> RuntimeLifetime> - where - T: 'a, - { - // SAFETY: RuntimeLifetime::get ensures that the 'static referencs are not exposed - // publicly and that users can only access T with an appropriate lifetime. - let value = unsafe { value.change_lifetime::<'static>() }; - - let references = unsafe { handle.__private_references(sealed::Sealed) }; - - // Relaxed ordering is used here because a shared reference is being held to the - // LifetimeTracker, meaning that LifetimeTracker::assert_no_references cannot be called - // concurrently in another thread becuase it requires an exclusive reference to the - // LifetimeTracker. In a multi-threaded scenario where the LifetimeTracker is shared - // across threads, there will always be a happens-before relationship where this increment - // occurs before LifetimeTracker::assert_no_references is called and reads this value - // because the synchronization primitive needed to get an exclusive reference to - // LifetimeTracker should form a happens-before relationship, so using a stricter ordering - // here is not needed. - references.fetch_add(1, Ordering::Relaxed); - - RuntimeLifetime { - value, - references: std::ptr::from_ref(references), - } - } - - #[must_use] - pub const fn get<'a>(&'a self) -> &'a T::WithLifetime<'a> { - // SAFETY: The program will abort if `self` is not dropped before - // LifetimeTracker::assert_no_references is called. 'a will expire before self is - // dropped. Therefore, it is safe to change these references to 'a, because if 'a - // were to live after LifetimeTracker::assert_no_references is called, the program - // would abort before user code could use the invalid reference. - unsafe { &*(&raw const self.value).cast::>() } - } -} - -unsafe impl Send for RuntimeLifetime where T: Send {} -unsafe impl Sync for RuntimeLifetime where T: Sync {} - -impl Drop for RuntimeLifetime { - fn drop(&mut self) { - // SAFETY: `self.references` is safe to dereference because the underlying LifetimeTracker would - // have already aborted if it were dropped before this - let references = unsafe { &*self.references }; - - // Relaxed ordering is used here because user code must guarantee that this will be dropped before another - // thread calls LifetimeTracker::assert_no_references, or else assert_no_references will abort. In a - // multi-threaded scenario, user code will already need to do something which would form a - // happens-before relationship to fulfill this guarantee, so any ordering stricter than - // Relaxed is not needed. - references.fetch_sub(1, Ordering::Relaxed); - - // Dropping the inner value is sound despite having 'static lifetime parameters because - // Drop implementations cannot be specialized, meaning that the Drop implementation cannot - // change its behavior to do something unsound (such as by keeping those 'static references - // after the value is dropped) when the type has 'static lifetime parameters. - } -} - -mod sealed { - pub struct Sealed; -} - -pub trait LifetimeHandle<'a> { - /// # Safety - /// The returned references value must only be used in increment-decrement pairs. In other words, it can only be - /// decremented if it were previously incremented. - #[must_use] - unsafe fn __private_references(&self, _: sealed::Sealed) -> &AtomicUsize; -} - -struct LifetimeHandleObject<'a> { - references: &'a AtomicUsize, -} - -impl<'a> LifetimeHandle<'a> for LifetimeHandleObject<'a> { - unsafe fn __private_references(&self, _: sealed::Sealed) -> &AtomicUsize { - self.references - } -} - -pub struct LifetimeTracker { - references: Box, -} - -impl LifetimeTracker { - pub fn assert_no_references(&mut self) { - let references = self.references.load(Ordering::Relaxed); - if references != 0 { - tracing::error!("{references} values were held too long - aborting"); - // abort is needed to avoid a panic handler allowing those values to continue being - // used - std::process::abort(); - } - } - - /// # Safety - /// Data which outlives the `'handle` lifetime and might have a [`RuntimeLifetime`] constructed with the resulting - /// [`LifetimeHandle`] must only be dropped after [`LifetimeTracker::assert_no_references`] is called on this - /// tracker. The only purpose of the `'handle` lifetime is to allow users to control which values can be wrapped - /// in a [`RuntimeLifetime`] since wrapped values must outlive `'handle`. The length of the `'handle` lifetime - /// itself does not matter, and `'handle` may expire before [`LifetimeTracker::assert_no_references`] is called. - #[must_use] - pub unsafe fn handle<'handle>(&'handle self) -> impl LifetimeHandle<'handle> { - // Storing the lifetime parameter in a trait (LifetimeHandle) instead of a struct is necessary to prohibit - // casts to a shorter lifetime. If the LifetimeHandle's lifetime could be shortened, the user could safely - // wrap values of any lifetime in RuntimeLifetime, which would defeat the purpose of the 'handle lifetime. - LifetimeHandleObject::<'handle> { - references: &self.references, - } - } -} - -impl Default for LifetimeTracker { - fn default() -> Self { - Self { - references: Box::new(AtomicUsize::new(0)), - } - } -} - -impl Drop for LifetimeTracker { - fn drop(&mut self) { - // Even if data associated with this tracker will live for 'static, the Box storing - // the references will be dropped, so this ensures that there are no - // RuntimeLifetimes which might still have a pointer to the references. - self.assert_no_references(); - } -} diff --git a/crates/hyperion/src/ingress/mod.rs b/crates/hyperion/src/ingress/mod.rs index 269d8be5..0d08e789 100644 --- a/crates/hyperion/src/ingress/mod.rs +++ b/crates/hyperion/src/ingress/mod.rs @@ -614,10 +614,9 @@ impl Module for IngressModule { }; // info_span!("ingress", ign = name).in_scope(|| { - // SAFETY: The packet bytes are allocated in the compose bump - if let Err(err) = unsafe { + if let Err(err) = crate::simulation::handlers::packet_switch(frame, &mut query) - } { + { error!("failed to process packet {frame:?}: {err}"); } // }); diff --git a/crates/hyperion/src/net/mod.rs b/crates/hyperion/src/net/mod.rs index 73d46efc..64293235 100644 --- a/crates/hyperion/src/net/mod.rs +++ b/crates/hyperion/src/net/mod.rs @@ -16,7 +16,6 @@ use flecs_ecs::{ }; use glam::I16Vec2; use hyperion_proto::{ChunkPosition, ServerToProxyMessage}; -use hyperion_utils::LifetimeTracker; use libdeflater::CompressionLvl; use rkyv::util::AlignedVec; use system_order::SystemOrder; @@ -109,7 +108,6 @@ pub struct Compose { global: Global, io_buf: IoBuf, pub bump: ThreadLocal, - pub bump_tracker: LifetimeTracker, } #[must_use] @@ -176,7 +174,6 @@ impl Compose { global, io_buf, bump: ThreadLocal::new_defaults(), - bump_tracker: LifetimeTracker::default(), } } @@ -307,7 +304,6 @@ impl Compose { } pub fn clear_bump(&mut self) { - self.bump_tracker.assert_no_references(); for bump in &mut self.bump { bump.reset(); } diff --git a/crates/hyperion/src/simulation/handlers.rs b/crates/hyperion/src/simulation/handlers.rs index dc740ad1..ec219feb 100644 --- a/crates/hyperion/src/simulation/handlers.rs +++ b/crates/hyperion/src/simulation/handlers.rs @@ -8,7 +8,7 @@ use anyhow::bail; use flecs_ecs::core::{Entity, EntityView, World}; use geometry::aabb::Aabb; use glam::{IVec3, Vec3}; -use hyperion_utils::{EntityExt, LifetimeHandle}; +use hyperion_utils::EntityExt; use tracing::{info, instrument, warn}; use valence_generated::{ block::{BlockKind, BlockState, PropName}, @@ -49,7 +49,6 @@ fn full( pitch, .. }: &play::FullC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { // check to see if the player is moving too fast @@ -181,7 +180,6 @@ fn has_block_collision(position: &Vec3, size: EntitySize, blocks: &Blocks) -> bo fn look_and_on_ground( &play::LookAndOnGroundC2s { yaw, pitch, .. }: &play::LookAndOnGroundC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { **query.yaw = yaw; @@ -192,7 +190,6 @@ fn look_and_on_ground( fn position_and_on_ground( &play::PositionAndOnGroundC2s { position, .. }: &play::PositionAndOnGroundC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { change_position_or_correct_client(query, position.as_vec3()); @@ -202,7 +199,6 @@ fn position_and_on_ground( fn hand_swing( &packet: &play::HandSwingC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { match packet.hand { @@ -220,7 +216,6 @@ fn hand_swing( #[instrument(skip_all)] fn player_interact_entity( packet: &play::PlayerInteractEntityC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { // attack @@ -267,7 +262,6 @@ pub struct PacketSwitchQuery<'a> { // i.e., shooting a bow, digging a block, etc fn player_action( &packet: &play::PlayerActionC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { let sequence = packet.sequence.0; @@ -312,7 +306,6 @@ fn player_action( // for sneaking/crouching/etc fn client_command( &packet: &play::ClientCommandC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { match packet.action { @@ -345,7 +338,6 @@ fn client_command( /// - Activating items with duration effects (e.g. chorus fruit teleport) pub fn player_interact_item( &play::PlayerInteractItemC2s { hand, sequence }: &play::PlayerInteractItemC2s, - handle: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { let event = InteractEvent { @@ -368,14 +360,13 @@ pub fn player_interact_item( query.events.push(flecs_event, query.world); } - query.handler_registry.trigger(&event, handle, query)?; + query.handler_registry.trigger(&event, query)?; Ok(()) } pub fn player_interact_block( &packet: &play::PlayerInteractBlockC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { // PlayerInteractBlockC2s contains: @@ -465,7 +456,6 @@ pub fn player_interact_block( pub fn update_selected_slot( &packet: &play::UpdateSelectedSlotC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { handle_update_selected_slot(packet, query); @@ -475,7 +465,6 @@ pub fn update_selected_slot( pub fn creative_inventory_action( play::CreativeInventoryActionC2s { slot, clicked_item }: &play::CreativeInventoryActionC2s, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { info!("creative inventory action: {slot} {clicked_item:?}"); @@ -493,7 +482,6 @@ pub fn creative_inventory_action( // keywords: inventory fn click_slot( pkt: &play::ClickSlotC2s<'_>, - _: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { handle_click_slot(pkt, query); @@ -501,12 +489,11 @@ fn click_slot( Ok(()) } -pub fn request_command_completions<'a>( +pub fn request_command_completions( play::RequestCommandCompletionsC2s { transaction_id, text, - }: &play::RequestCommandCompletionsC2s<'a>, - handle: &dyn LifetimeHandle<'a>, + }: &play::RequestCommandCompletionsC2s<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { let text = text.0; @@ -517,14 +504,13 @@ pub fn request_command_completions<'a>( id: transaction_id, }; - query.handler_registry.trigger(&completion, handle, query)?; + query.handler_registry.trigger(&completion, query)?; Ok(()) } pub fn client_status( pkt: &play::ClientStatusC2s, - handle: &dyn LifetimeHandle<'_>, query: &mut PacketSwitchQuery<'_>, ) -> anyhow::Result<()> { let command = ClientStatusEvent { @@ -535,7 +521,7 @@ pub fn client_status( }, }; - query.handler_registry.trigger(&command, handle, query)?; + query.handler_registry.trigger(&command, query)?; Ok(()) } @@ -557,26 +543,16 @@ pub fn add_builtin_handlers(registry: &mut HandlerRegistry) { registry.add_handler(Box::new(update_selected_slot)); } -/// # Safety -/// The [`BorrowedPacketFrame`] must borrow data from the [`Compose::bump`] in -/// [`PacketSwitchQuery::compose`] -pub unsafe fn packet_switch<'a>( +pub fn packet_switch<'a>( raw: BorrowedPacketFrame<'a>, query: &mut PacketSwitchQuery<'a>, ) -> anyhow::Result<()> { let packet_id = raw.id; let data = raw.body; - // SAFETY: The only data that HandlerRegistry::process_packet is aware of outliving 'a is the packet bytes. - // The packet bytes are stored in the compose bump allocator. - // LifetimeTracker::assert_no_references will be called on the bump tracker before the - // bump allocator is cleared. - let handle = unsafe { query.compose.bump_tracker.handle() }; - let handle: &dyn LifetimeHandle<'a> = &handle; - query .handler_registry - .process_packet(packet_id, data, handle, query)?; + .process_packet(packet_id, data, query)?; Ok(()) } diff --git a/crates/hyperion/src/simulation/packet.rs b/crates/hyperion/src/simulation/packet.rs index 5fba4c0b..a515af01 100644 --- a/crates/hyperion/src/simulation/packet.rs +++ b/crates/hyperion/src/simulation/packet.rs @@ -6,25 +6,16 @@ use std::{ use anyhow::Result; use flecs_ecs::macros::Component; -use hyperion_utils::{Lifetime, LifetimeHandle}; +use hyperion_utils::Lifetime; use rustc_hash::FxBuildHasher; use valence_protocol::{Decode, Packet}; use crate::simulation::handlers::{PacketSwitchQuery, add_builtin_handlers}; -type DeserializerFn = for<'packet> fn( - &HandlerRegistry, - &'packet [u8], - &dyn LifetimeHandle<'packet>, - &mut PacketSwitchQuery<'_>, -) -> Result<()>; +type DeserializerFn = fn(&HandlerRegistry, &[u8], &mut PacketSwitchQuery<'_>) -> Result<()>; type AnyFn = Box; type Handler = Box< - dyn for<'packet> Fn( - &::WithLifetime<'packet>, - &dyn LifetimeHandle<'packet>, - &mut PacketSwitchQuery<'_>, - ) -> Result<()> + dyn Fn(&::WithLifetime<'_>, &mut PacketSwitchQuery<'_>) -> Result<()> + Send + Sync, >; @@ -32,7 +23,6 @@ type Handler = Box< fn packet_deserializer<'p, P>( registry: &HandlerRegistry, mut bytes: &'p [u8], - handle: &dyn LifetimeHandle<'p>, query: &mut PacketSwitchQuery<'_>, ) -> Result<()> where @@ -52,7 +42,7 @@ where let packet = P::decode(unsafe { transmute::<&mut &'p [u8], &mut &'static [u8]>(&mut bytes) })? .shorten_lifetime::<'p>(); - registry.trigger(&packet, handle, query)?; + registry.trigger(&packet, query)?; Ok(()) } @@ -66,18 +56,14 @@ pub struct HandlerRegistry { impl HandlerRegistry { // Add a handler - // TODO: With this current system, closures infer that 'a is a specific lifetime if the type isn't specified. Unsure if there's a way to fix it while allowing P to be inferred. - pub fn add_handler<'a, P, F>(&mut self, handler: Box) + pub fn add_handler(&mut self, handler: Box) where P: Lifetime, - // Needed to allow compiler to infer type of P. - F: Fn(&P, &dyn LifetimeHandle<'a>, &mut PacketSwitchQuery<'_>) -> Result<()> + Send + Sync, + // Needed to allow compiler to infer type of P without the user needing to specify + // P::WithLifetime<'_>. + F: Fn(&P, &mut PacketSwitchQuery<'_>) -> Result<()> + Send + Sync, // Actual type bounds for Handler

- for<'packet> F: Fn( - &P::WithLifetime<'packet>, - &dyn LifetimeHandle<'packet>, - &mut PacketSwitchQuery<'_>, - ) -> Result<()> + F: Fn(&P::WithLifetime<'_>, &mut PacketSwitchQuery<'_>) -> Result<()> + Send + Sync + 'static, @@ -90,11 +76,10 @@ impl HandlerRegistry { } // Process a packet, calling all registered handlers - pub fn process_packet<'p>( + pub fn process_packet( &self, id: i32, - bytes: &'p [u8], - handle: &dyn LifetimeHandle<'p>, + bytes: &[u8], query: &mut PacketSwitchQuery<'_>, ) -> Result<()> { // Get the deserializer @@ -103,7 +88,7 @@ impl HandlerRegistry { .get(&id) .ok_or_else(|| anyhow::anyhow!("No deserializer registered for packet ID: {}", id))?; - deserializer(self, bytes, handle, query) + deserializer(self, bytes, query) } #[must_use] @@ -115,14 +100,9 @@ impl HandlerRegistry { .contains_key(&TypeId::of::>()) } - pub fn trigger<'packet, T>( - &self, - value: &T, - handle: &dyn LifetimeHandle<'packet>, - query: &mut PacketSwitchQuery<'_>, - ) -> Result<()> + pub fn trigger(&self, value: &T, query: &mut PacketSwitchQuery<'_>) -> Result<()> where - T: Lifetime + 'packet, + T: Lifetime, { // Get all handlers for this type let handlers = self @@ -141,7 +121,7 @@ impl HandlerRegistry { let handler = unsafe { &*std::ptr::from_ref(handler).cast::>() }; // shorten_lifetime is only needed because the handler accepts T::WithLifetime - handler(value.shorten_lifetime_ref(), handle, query)?; + handler(value.shorten_lifetime_ref(), query)?; } Ok(()) diff --git a/events/tag/src/module/attack.rs b/events/tag/src/module/attack.rs index 049ebb0a..b2047a39 100644 --- a/events/tag/src/module/attack.rs +++ b/events/tag/src/module/attack.rs @@ -39,7 +39,7 @@ use hyperion::{ }; use hyperion_inventory::PlayerInventory; use hyperion_rank_tree::Team; -use hyperion_utils::{EntityExt, LifetimeHandle}; +use hyperion_utils::EntityExt; use tracing::info_span; use valence_protocol::packets::play::player_position_look_s2c::PlayerPositionLookFlags; @@ -511,9 +511,7 @@ impl Module for AttackModule { world.get::<&mut HandlerRegistry>(|registry| { registry.add_handler(Box::new( - |client_status: &ClientStatusEvent, - _: &dyn LifetimeHandle<'_>, - query: &mut PacketSwitchQuery<'_>| { + |client_status: &ClientStatusEvent, query: &mut PacketSwitchQuery<'_>| { if client_status.status == ClientStatusCommand::RequestStats { return Ok(()); } diff --git a/events/tag/src/module/chat.rs b/events/tag/src/module/chat.rs index 656f102c..194616c5 100644 --- a/events/tag/src/module/chat.rs +++ b/events/tag/src/module/chat.rs @@ -11,7 +11,6 @@ use hyperion::{ }, }; use hyperion_rank_tree::Team; -use hyperion_utils::LifetimeHandle; use tracing::info_span; const CHAT_COOLDOWN_SECONDS: i64 = 15; // 15 seconds @@ -36,9 +35,7 @@ impl Module for ChatModule { world.get::<&mut HandlerRegistry>(|registry| { registry.add_handler(Box::new( - |packet: &ChatMessageC2s<'_>, - _: &dyn LifetimeHandle<'_>, - query: &mut PacketSwitchQuery<'_>| { + |packet: &ChatMessageC2s<'_>, query: &mut PacketSwitchQuery<'_>| { let span = info_span!("handle_chat_messages"); let _enter = span.enter();