refactor: restructure events

crates/hyperion-command/Cargo.toml

@@ -9,9 +9,9 @@ publish = false
 [dependencies]
 flecs_ecs = { workspace = true }
 hyperion = { workspace = true }
-hyperion-utils = { workspace = true }
 indexmap = { workspace = true }
 tracing = { workspace = true }
+valence_protocol = { workspace = true }
 
 [lints]
 workspace = true

crates/hyperion-command/src/system.rs

@@ -1,16 +1,16 @@
 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,59 +19,51 @@ pub struct CommandSystemModule;
 
 impl Module for CommandSystemModule {
     fn module(world: &World) {
-        system!(
-            "execute_command",
-            world,
-            &mut EventQueue<event::Command>($),
-            &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<'_>, 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<'_>,
-                 query: &mut PacketSwitchQuery<'_>| {
+                |completion: &CommandCompletionRequest<'_>, query: &mut PacketSwitchQuery<'_>| {
                     let input = completion.query;
 
                     // should be in form "/{command}"

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 }

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;

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]

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;
 

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(());
                     }

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<T> {
-    value: T,
-    references: *const AtomicUsize,
-}
-
-impl<T: Lifetime> RuntimeLifetime<T> {
-    #[must_use]
-    pub fn new<'a>(
-        value: T,
-        handle: &dyn LifetimeHandle<'a>,
-    ) -> RuntimeLifetime<T::WithLifetime<'static>>
-    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::<T::WithLifetime<'a>>() }
-    }
-}
-
-unsafe impl<T> Send for RuntimeLifetime<T> where T: Send {}
-unsafe impl<T> Sync for RuntimeLifetime<T> where T: Sync {}
-
-impl<T> Drop for RuntimeLifetime<T> {
-    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<AtomicUsize>,
-}
-
-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();
-    }
-}

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}");
                                 }
                                 // });