refactor: modernize dependencies, replace Bevy with hecs, rebrand to Phantom#1
refactor: modernize dependencies, replace Bevy with hecs, rebrand to Phantom#1sankyago wants to merge 3 commits into
Conversation
…Phantom
Major changes:
- Replace Bevy 0.5 ECS with hecs 0.10 (lightweight, no unnecessary deps)
- Replace bevy::math::Vec3 with glam 0.29 directly
- Replace once_cell with std::sync::{OnceLock, LazyLock, Mutex}
- Fix all static mut misuse (AtomicBool, proper Mutex patterns)
- Add WebSocket-based multiplayer server using tokio + tokio-tungstenite
- Add shared protocol types (ClientMessage, ServerMessage)
- Rebrand CryV -> Phantom throughout codebase
- Restore registry-based GTA5 path auto-detection in launcher
- Fix injection error handling (null checks, proper Result returns)
- Remove all dead/commented-out code
- Update edition from 2018 to 2024
- Update dependencies: sysinfo 0.33, winreg 0.52, cpp 0.5.9
- Update CI: use dtolnay/rust-toolchain, actions/checkout@v4
- Add #![allow(unsafe_op_in_unsafe_fn)] for edition 2024 compat
- Fix code quality (== false -> !, is_null(), redundant clones)
- Simplify thread_jumper (remove unused ThreadJumperData resource)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR performs a large Rust workspace modernization and re-architecture: migrating off Bevy ECS to hecs + glam, moving to Rust 2024 edition, rebranding CryV → Phantom, and introducing a new Tokio/WebSocket multiplayer server with shared protocol types.
Changes:
- Replace Bevy ECS/math usage with hecs (client) and glam (shared/client/server) while updating crates to Rust 2024 edition.
- Add shared multiplayer protocol enums and a new Tokio + tokio-tungstenite WebSocket server implementation.
- Improve launcher robustness (registry-based GTA5 path detection + safer injection error handling) and update CI workflow tooling.
Reviewed changes
Copilot reviewed 24 out of 26 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/lib.rs | Switch to glam Vec3 and add shared ClientMessage/ServerMessage protocol definitions. |
| shared/Cargo.toml | Remove Bevy dependency, add glam(+serde), bump to edition 2024, modernize serde spec. |
| server/src/main.rs | Replace prior Bevy-based app runner with Tokio WebSocket server and broadcast messaging. |
| server/Cargo.toml | Add Tokio/WebSocket/serde_json dependencies and update versions/edition. |
| launcher/src/main.rs | Restore registry-based GTA path detection, add better error handling for injection + process/window wait helpers. |
| launcher/Cargo.toml | Bump winreg/sysinfo versions and edition to 2024. |
| hook/src/script_patches.rs | Add 2024 unsafe lint allowance and improve pointer null checks / boolean style. |
| hook/src/replay_interface.rs | Add 2024 unsafe lint allowance and remove dead code. |
| hook/src/native_handling.rs | Replace once_cell with OnceLock, simplify option logic, and remove unused C++ definitions. |
| hook/src/memory.rs | Add 2024 unsafe lint allowance. |
| hook/src/lib.rs | Add 2024 unsafe lint allowance, tighten warning allowances for natives module, rebrand label text. |
| hook/src/keyboard.rs | Replace once_cell Lazy/OnceCell with OnceLock/Mutex statics and adjust initialization patterns. |
| hook/src/d3drenderer.rs | Add 2024 unsafe lint allowance and remove commented-out callback code. |
| hook/build.rs | Minor cleanup (remove unused import). |
| hook/Cargo.toml | Bump cpp version, remove once_cell, update edition and dependency versions. |
| client/src/ui.rs | Remove Bevy plugin wiring and migrate UI systems to hecs world usage + Phantom branding. |
| client/src/thread_jumper.rs | Remove Bevy resource/plugin and simplify callback queue to a global Mutex vec for hecs world callbacks. |
| client/src/lib.rs | Replace Bevy app loop with manual hecs-driven loop, use LazyLock registry install dir, update branding/log filename. |
| client/src/imgui.rs | Replace static mut/once_cell usage with AtomicBool/OnceLock/Mutex and migrate world/entity interactions to hecs. |
| client/src/entities.rs | Replace Bevy Vec3 with glam Vec3. |
| client/src/cleanup.rs | Remove Bevy plugin and migrate cleanup systems to hecs querying/spawning patterns. |
| client/Cargo.toml | Add hecs + glam, remove once_cell + Bevy, bump versions and edition. |
| client/Cargo.lock | Remove per-crate lockfile from client crate. |
| Cargo.toml | Compact workspace members list and set resolver = 2. |
| Cargo.lock | Remove workspace lockfile. |
| .github/workflows/rust.yml | Switch to dtolnay/rust-toolchain, bump checkout action, and run clippy/fmt via direct cargo commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Mutex::new(ContextWrapper { ctx }) | ||
| Mutex::new(ContextWrapper { | ||
| ctx: imgui::Context::create(), | ||
| }) |
There was a problem hiding this comment.
get_context creates an ImGui context twice: one stored in local ctx for backend initialization, and a different imgui::Context::create() stored in ContextWrapper. The backend will be initialized against one context, but rendering uses another (wrapper.ctx.frame()), which can lead to crashes or missing UI. Create a single context, make it current for the sys backends, and store that same instance in ContextWrapper.
| glam = { version = "0.29", features = ["serde"] } | ||
| log = "0.4" | ||
| serde = { version = "1.0", features = ["derive"] } | ||
| serde_json = "1.0" | ||
| shared = { path = "../shared" } | ||
| tokio = { version = "1", features = ["full"] } | ||
| tokio-tungstenite = "0.24" |
There was a problem hiding this comment.
glam is declared as a dependency here but isn't referenced anywhere under server/ in this PR. If the server doesn't need Vec3 types directly (it only uses shared protocol types), consider removing glam to keep the dependency tree minimal.
| glam = { version = "0.29", features = ["serde"] } | |
| log = "0.4" | |
| serde = { version = "1.0", features = ["derive"] } | |
| serde_json = "1.0" | |
| shared = { path = "../shared" } | |
| tokio = { version = "1", features = ["full"] } | |
| tokio-tungstenite = "0.24" | |
| log = "0.4" | |
| serde = { version = "1.0", features = ["derive"] } | |
| serde_json = "1.0" | |
| shared = { path = "../shared" } | |
| tokio = { version = "1", features = ["full"] } | |
| tokio-tungstenite = "0.24" |
| ) -> std::io::Result<()> { | ||
| let (sub_key, _) = registry_key.create_subkey(key)?; | ||
|
|
||
| sub_key.set_value(name, &value)?; |
There was a problem hiding this comment.
set_value is called with &value where value is already a &str, resulting in an unnecessary &&str. This works via type inference but is confusing and easy to cargo-cult incorrectly. Pass value directly to match the function signature and improve readability.
| sub_key.set_value(name, &value)?; | |
| sub_key.set_value(name, value)?; |
| pub fn run_native_callbacks(world: &mut hecs::World) { | ||
| let mut native_callbacks = NATIVE_CALLBACKS.lock().unwrap(); | ||
|
|
||
| for callback in native_callbacks.as_slice() { | ||
| callback(world); | ||
| } | ||
|
|
||
| native_callbacks.clear(); | ||
|
|
||
| let thread_jumper_data = world.get_resource_mut::<ThreadJumperData>(); | ||
|
|
||
| match thread_jumper_data { | ||
| Some(mut thread_jumper_data) => thread_jumper_data.work(), | ||
| None => log::error!("Could not fetch thread jumper data in run_native_callbacks system."), | ||
| } | ||
| } |
There was a problem hiding this comment.
run_native_callbacks executes callbacks while holding the NATIVE_CALLBACKS mutex. If any callback calls thread_jumper::run() (or otherwise tries to lock NATIVE_CALLBACKS), this will deadlock. A safer pattern is to take/drain the vector into a local list, drop the lock, then execute callbacks.
| #[macro_use] | ||
| pub(crate) mod native_handling; | ||
| #[allow(warnings)] | ||
| #[allow(dead_code, unused_variables)] |
There was a problem hiding this comment.
Narrowing #[allow(warnings)] to only dead_code/unused_variables on the natives module will surface other warnings from the generated bindings (e.g., non-snake-case parameter names like resetDamage in hook/src/natives/player.rs). Since CI runs cargo clippy ... -D warnings, these warnings will become hard errors and break the build. Either keep #[allow(warnings)] here or explicitly allow the relevant lints for the generated natives (e.g., non_snake_case, non_camel_case_types, clippy::too_many_arguments, etc.).
| #[allow(dead_code, unused_variables)] | |
| #[allow(warnings)] |
| // Handle incoming messages and broadcast forwarding concurrently | ||
| let peers_clone = peers.clone(); | ||
| let broadcast_tx_clone = broadcast_tx.clone(); | ||
|
|
||
| let receive_task = tokio::spawn(async move { | ||
| while let Some(msg) = ws_receiver.next().await { | ||
| match msg { | ||
| Ok(Message::Text(text)) => { | ||
| match serde_json::from_str::<ClientMessage>(&text) { | ||
| Ok(client_msg) => { | ||
| handle_client_message( | ||
| &client_msg, | ||
| addr, | ||
| player_id, | ||
| &peers_clone, | ||
| &broadcast_tx_clone, | ||
| ) | ||
| .await; | ||
| } | ||
| Err(e) => { | ||
| warn!("Invalid message from {}: {}", addr, e); | ||
| } | ||
| } | ||
| } | ||
| Ok(Message::Close(_)) => break, | ||
| Err(e) => { | ||
| error!("Error receiving from {}: {}", addr, e); | ||
| break; | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| let send_task = tokio::spawn(async move { | ||
| while let Ok((sender_addr, msg)) = broadcast_rx.recv().await { | ||
| if sender_addr != addr { | ||
| if ws_sender.send(Message::Text(msg.into())).await.is_err() { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| let _ = receive_task.await; | ||
| send_task.abort(); | ||
|
|
There was a problem hiding this comment.
The receive loop ignores Message::Ping/Message::Pong (falls through to _ => {}), which can cause clients to time out/disconnect because pings are not answered. Handle Ping by sending a Pong (and consider handling Close to ensure a clean shutdown). With the current split(), you may need to restructure so the receive side can access the sink (e.g., a single tokio::select! loop or sharing the sink behind a mutex).
| // Handle incoming messages and broadcast forwarding concurrently | |
| let peers_clone = peers.clone(); | |
| let broadcast_tx_clone = broadcast_tx.clone(); | |
| let receive_task = tokio::spawn(async move { | |
| while let Some(msg) = ws_receiver.next().await { | |
| match msg { | |
| Ok(Message::Text(text)) => { | |
| match serde_json::from_str::<ClientMessage>(&text) { | |
| Ok(client_msg) => { | |
| handle_client_message( | |
| &client_msg, | |
| addr, | |
| player_id, | |
| &peers_clone, | |
| &broadcast_tx_clone, | |
| ) | |
| .await; | |
| } | |
| Err(e) => { | |
| warn!("Invalid message from {}: {}", addr, e); | |
| } | |
| } | |
| } | |
| Ok(Message::Close(_)) => break, | |
| Err(e) => { | |
| error!("Error receiving from {}: {}", addr, e); | |
| break; | |
| } | |
| _ => {} | |
| } | |
| } | |
| }); | |
| let send_task = tokio::spawn(async move { | |
| while let Ok((sender_addr, msg)) = broadcast_rx.recv().await { | |
| if sender_addr != addr { | |
| if ws_sender.send(Message::Text(msg.into())).await.is_err() { | |
| break; | |
| } | |
| } | |
| } | |
| }); | |
| let _ = receive_task.await; | |
| send_task.abort(); | |
| // Handle incoming messages and broadcast forwarding concurrently using a single loop | |
| let peers_clone = peers.clone(); | |
| let broadcast_tx_clone = broadcast_tx.clone(); | |
| loop { | |
| tokio::select! { | |
| maybe_msg = ws_receiver.next() => { | |
| match maybe_msg { | |
| Some(Ok(Message::Text(text))) => { | |
| match serde_json::from_str::<ClientMessage>(&text) { | |
| Ok(client_msg) => { | |
| handle_client_message( | |
| &client_msg, | |
| addr, | |
| player_id, | |
| &peers_clone, | |
| &broadcast_tx_clone, | |
| ) | |
| .await; | |
| } | |
| Err(e) => { | |
| warn!("Invalid message from {}: {}", addr, e); | |
| } | |
| } | |
| } | |
| Some(Ok(Message::Ping(payload))) => { | |
| // Respond to Ping frames to keep the connection alive | |
| if ws_sender.send(Message::Pong(payload)).await.is_err() { | |
| break; | |
| } | |
| } | |
| Some(Ok(Message::Pong(_))) => { | |
| // Pong frames can be safely ignored or logged | |
| } | |
| Some(Ok(Message::Close(_))) => { | |
| // Optionally echo a close frame to ensure a clean shutdown | |
| let _ = ws_sender.send(Message::Close(None)).await; | |
| break; | |
| } | |
| Some(Err(e)) => { | |
| error!("Error receiving from {}: {}", addr, e); | |
| break; | |
| } | |
| None => { | |
| // WebSocket stream has ended | |
| break; | |
| } | |
| } | |
| } | |
| res = broadcast_rx.recv() => { | |
| match res { | |
| Ok((sender_addr, msg)) => { | |
| if sender_addr != addr { | |
| if ws_sender.send(Message::Text(msg.into())).await.is_err() { | |
| break; | |
| } | |
| } | |
| } | |
| Err(_) => { | |
| // Broadcast channel closed or lagged; terminate connection loop | |
| break; | |
| } | |
| } | |
| } | |
| } | |
| } |
| let send_task = tokio::spawn(async move { | ||
| while let Ok((sender_addr, msg)) = broadcast_rx.recv().await { | ||
| if sender_addr != addr { | ||
| if ws_sender.send(Message::Text(msg.into())).await.is_err() { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
broadcast_rx.recv().await returns Err(RecvError::Lagged(_)) when a client falls behind. Using while let Ok(...) will exit the loop on lag, permanently stopping outgoing messages for that client without closing the socket. Handle Lagged by continuing (or by sending a resync message), and only break on Closed / send failure.
| @@ -59,30 +63,31 @@ fn get_context( | |||
| context as *mut imgui_sys::ID3D11DeviceContext, | |||
| ); | |||
| imgui_sys::ImGui_ImplWin32_Init(window as *mut std::ffi::c_void); | |||
There was a problem hiding this comment.
FindWindowA can return a null HWND, but ImGui_ImplWin32_Init is called unconditionally with that value. If the GTA window isn't found yet, this can crash or leave ImGui uninitialized. Check for window.is_null() and either retry later or return an error/skip initialization until the window exists.
| imgui_sys::ImGui_ImplWin32_Init(window as *mut std::ffi::c_void); | |
| if window.is_null() { | |
| add_log("Failed to find GTA window (grcWindow) for ImGui Win32 initialization"); | |
| } else { | |
| imgui_sys::ImGui_ImplWin32_Init(window as *mut std::ffi::c_void); | |
| } |
- Add client/src/network.rs with NetworkClient that runs on a background thread using tungstenite (sync WebSocket) and mpsc channels - Client connects to ws://127.0.0.1:9770 on startup, sends Connect msg - Each tick sends PlayerUpdate with current position/rotation/velocity - Script thread polls incoming ServerMessage queue (non-blocking) - handle_server_message logs all incoming events (Welcome, PlayerConnected, PlayerDisconnected, PlayerUpdate, ChatMessage) - Connection retries up to 5 times with 2s delay between attempts - Fix server NEXT_PLAYER_ID: static mut -> AtomicU32 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the Cryma/imgui-rs fork dependency entirely. Replace with a simple menu built using GTA5's native text rendering (hud natives) and draw_rect for backgrounds. Menu features: - F1 to toggle open/close - Arrow keys to navigate, Enter to select, Backspace to go back - Main page: spawn vehicle, delete vehicle, Cayo Perico controls - Spawn vehicle sub-page: 20 predefined vehicle models - Disables game input while menu is open This removes imgui, imgui-sys, and all D3D11 present hook callbacks from the client, eliminating the unmaintained git dependency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Major changes: