diff --git a/.gitignore b/.gitignore index 93d01bfd..64206f48 100644 --- a/.gitignore +++ b/.gitignore @@ -85,4 +85,6 @@ SnapshotProfiles/TestProfile2.yaml MMManaged.Engine/obj MMManaged.Engine/bin obj/fsac.cache -MMTricks/obj \ No newline at end of file +MMTricks/obj +.paket/Paket.Restore.targets +testmodload*.txt diff --git a/DEVNOTES.md b/DEVNOTES.md index e7d4a352..342b28f9 100644 --- a/DEVNOTES.md +++ b/DEVNOTES.md @@ -46,6 +46,18 @@ Might be worth trying again with local claude code someday, but the web version ## Code Stuff +### Static mut + +The rust code makes heavy use of obtaining references to a global static mut (GLOBAL_STATE). For years this was not warned +about by the compiler at all, but is now considered a very serious problem. These days you can get an LLM to generate a +sample that will show the failure in rust playground - often only shows up in release build there, since it stems from LLVM +optimization. + +I haven't seen any cases where this causes problems in modelmod, and I do try to lock around GLOBAL_STATE with GLOBAL_STATE_LOCK, except in the draw indexed path where I thought it would be too slow. However I'm in the process of slowly untangling and fixing this. + +For now the warning is disabled in most places, but it still comes up now and again. As I fix areas in the code that have +this problem I plan to remove the warning. + ### MeshRelation building This was slow for a long time but I had claude put in an optimization which speeds it up pretty dramatically (c9a8fd2). diff --git a/Native/global_state/src/global_state.rs b/Native/global_state/src/global_state.rs index 02277668..f2d58750 100644 --- a/Native/global_state/src/global_state.rs +++ b/Native/global_state/src/global_state.rs @@ -115,7 +115,6 @@ pub struct HookState { pub clr: ClrState, pub interop_state: Option, //pub is_global: bool, - pub loaded_mods: Option, /// List of mod names that should have the d3d resources loaded on the next frame. /// Mods are added to this by `hook_draw_indexed_primitive` when it discovers that is /// trying to render a mod that hasn't been loaded yet. @@ -218,7 +217,6 @@ pub static mut GLOBAL_STATE: HookState = HookState { interop_state: None, //is_global: true, load_on_next_frame: None, - loaded_mods: None, active_texture_set: None, active_texture_list: None, dx9_update_texture_map: None, @@ -259,6 +257,9 @@ pub static mut GLOBAL_STATE: HookState = HookState { }; pub static mut ANIM_SNAP_STATE:UnsafeCell> = UnsafeCell::new(None); +/// Loaded mod database +pub static mut LOADED_MODS: Option = None; + const TRACK_GS_PTR:bool = true; /// Container structure providing access to the global state pointer. diff --git a/Native/hook_core/src/hook_render.rs b/Native/hook_core/src/hook_render.rs index 5b220780..51c3544b 100644 --- a/Native/hook_core/src/hook_render.rs +++ b/Native/hook_core/src/hook_render.rs @@ -25,7 +25,7 @@ use crate::debug_spam; use crate::input_commands; use crate::mod_render; use mod_stats::mod_stats; -use global_state::{GLOBAL_STATE, GLOBAL_STATE_LOCK}; +use global_state::{GLOBAL_STATE, GLOBAL_STATE_LOCK, LOADED_MODS}; use device_state::dev_state; use hook_snapshot; use types::native_mod; @@ -802,7 +802,7 @@ where ptr: GLOBAL_STATE.bound_vertex_buffer, checksums: GLOBAL_STATE.vb_checksums.as_ref(), }; - let res = GLOBAL_STATE.loaded_mods.as_mut() + let res = LOADED_MODS.as_mut() .and_then(|mods| { profile_start!(hdip, mod_select); diff --git a/Native/hook_core/src/hook_render_d3d11.rs b/Native/hook_core/src/hook_render_d3d11.rs index e9369e68..4292f54b 100644 --- a/Native/hook_core/src/hook_render_d3d11.rs +++ b/Native/hook_core/src/hook_render_d3d11.rs @@ -4,7 +4,7 @@ use std::ptr::{null_mut, null}; use std::sync::atomic::Ordering; use std::time::{SystemTime, Duration}; -use global_state::{GLOBAL_STATE, METRICS_TRACK_MOD_PRIMS, HWND}; +use global_state::{GLOBAL_STATE, LOADED_MODS, METRICS_TRACK_MOD_PRIMS, HWND}; use mod_stats::mod_stats; use shared_dx::dx11rs::{DX11RenderState}; use shared_dx::types::{HookDeviceState, DevicePointer, DX11Metrics, D3D11Tex}; @@ -642,7 +642,7 @@ pub unsafe extern "system" fn hook_draw_indexed( // if there is a matching mod, render it profile_start!(hdi, mod_precheck); - let quickcheck = GLOBAL_STATE.loaded_mods.as_mut().map( + let quickcheck = LOADED_MODS.as_mut().map( |mods| mod_render::preselect(mods, prim_count, vert_count)) .unwrap_or(false); let mod_status = if !quickcheck { @@ -679,7 +679,7 @@ pub unsafe extern "system" fn hook_draw_indexed( CheckRenderModResult::Deleted => false, CheckRenderModResult::NotRenderedButLoadRequested(ref name) => { // setup data to begin mod load - let nmod = mod_load::get_mod_by_name(name, &mut GLOBAL_STATE.loaded_mods); + let nmod = mod_load::get_mod_by_name(name, &mut LOADED_MODS); if let Some(nmod) = nmod { // need to store current input layout in the d3d data if let ModD3DState::Unloaded = nmod.d3d_data { diff --git a/Native/hook_core/src/input_commands.rs b/Native/hook_core/src/input_commands.rs index be214a65..baec7540 100644 --- a/Native/hook_core/src/input_commands.rs +++ b/Native/hook_core/src/input_commands.rs @@ -9,7 +9,7 @@ use fnv::FnvHashSet; use std::ptr::null_mut; use shared_dx::util::*; -use global_state::GLOBAL_STATE; +use global_state::{GLOBAL_STATE, LOADED_MODS}; use device_state::dev_state; use crate::hook_device_d3d11::apply_device_hook; use crate::hook_device_d3d11::query_and_set_runconf_in_globalstate; @@ -331,7 +331,7 @@ fn select_next_variant() { let hookstate = unsafe { &mut GLOBAL_STATE }; let lastframe = hookstate.metrics.total_frames; - hookstate.loaded_mods.as_mut().map(|mstate| { + unsafe { LOADED_MODS.as_mut() }.map(|mstate| { mod_render::select_next_variant(mstate, lastframe); mod_prefs::save_variant_selections(&mstate.mods, &mstate.selected_variant); }); @@ -340,7 +340,7 @@ fn select_prev_variant() { let hookstate = unsafe { &mut GLOBAL_STATE }; let lastframe = hookstate.metrics.total_frames; - hookstate.loaded_mods.as_mut().map(|mstate| { + unsafe { LOADED_MODS.as_mut() }.map(|mstate| { mod_render::select_prev_variant(mstate, lastframe); mod_prefs::save_variant_selections(&mstate.mods, &mstate.selected_variant); }); diff --git a/Native/mod_load/src/load_thread.rs b/Native/mod_load/src/load_thread.rs index 486abeeb..9c9b9eb1 100644 --- a/Native/mod_load/src/load_thread.rs +++ b/Native/mod_load/src/load_thread.rs @@ -25,7 +25,7 @@ use std::{ thread, }; -use global_state::GLOBAL_STATE; +use global_state::LOADED_MODS; use shared_dx::{types::DevicePointer, util::write_log_file}; use types::{interop::ManagedCallbacks, native_mod::NativeModData}; @@ -189,7 +189,7 @@ fn load_resource(mut msg: LoadMsg) -> Result<(), String> { let (diff,new_rc) = update_ref_count(msg.device, pre_rc); // i don't think its necessary to lock here because this is just a read of the hash table, though // we'll overwrite the value if we find it - if let Some(ref mut oldnmod) = get_mod_by_index(msg.nmod.midx, &mut GLOBAL_STATE.loaded_mods) { + if let Some(ref mut oldnmod) = get_mod_by_index(msg.nmod.midx, &mut LOADED_MODS) { // TODO(safety): I haven't seen a problem with this, but it may be a possible "torn write" where the render thread could observe a partially loaded struct, // if d3d_data with Loaded is copied over before the remaining stuff (copy ordering not guaranteed). // not clear on fix but maybe use atomics or else don't reuse the old hash slot at all (complicated because the mod is actually diff --git a/Native/mod_load/src/mod_load.rs b/Native/mod_load/src/mod_load.rs index 2b67eb36..94e329f0 100644 --- a/Native/mod_load/src/mod_load.rs +++ b/Native/mod_load/src/mod_load.rs @@ -27,7 +27,7 @@ use std; use std::ptr::null_mut; use shared_dx::util::*; use device_state::*; -use global_state::{GLOBAL_STATE, GLOBAL_STATE_LOCK, LoadedModState}; +use global_state::{GLOBAL_STATE, GLOBAL_STATE_LOCK, LOADED_MODS, LoadedModState}; use types::interop; use types::native_mod; @@ -67,7 +67,7 @@ pub unsafe fn clear_loaded_mods(device: DevicePointer) { // get device ref count prior to adding everything let pre_rc = device.get_ref_count(); - let mods = GLOBAL_STATE.loaded_mods.take(); + let mods = LOADED_MODS.take(); let mut cnt = 0; mods.map(|mods| { for (_key, modvec) in mods.mods.into_iter() { @@ -77,7 +77,7 @@ pub unsafe fn clear_loaded_mods(device: DevicePointer) { } } }); - GLOBAL_STATE.loaded_mods = None; + LOADED_MODS = None; GLOBAL_STATE.load_on_next_frame.as_mut().map(|hs| hs.clear()); let post_rc = (device).get_ref_count(); @@ -719,7 +719,7 @@ pub unsafe fn setup_mod_data(device: DevicePointer, callbacks: interop::ManagedC let mut selected_variant = global_state::new_fnv_map(16); mod_prefs::load_and_apply_variants(&loaded_mods, &mut selected_variant); - GLOBAL_STATE.loaded_mods = Some(LoadedModState { + LOADED_MODS = Some(LoadedModState { mods: loaded_mods, mods_by_name: mods_by_name, selected_variant, @@ -831,7 +831,7 @@ pub unsafe fn load_deferred_mods(device: DevicePointer, callbacks: interop::Mana for nmd in to_load.iter() { let mut nmod = - get_mod_by_name(&nmd, &mut GLOBAL_STATE.loaded_mods); + get_mod_by_name(&nmd, &mut LOADED_MODS); if let Some(ref mut nmod) = nmod { if nmod.fill_attempts > MAX_FILL_ATTEMPTS { if nmod.fill_attempts == MAX_FILL_ATTEMPTS + 1 { diff --git a/Native/mod_stats/src/mod_stats.rs b/Native/mod_stats/src/mod_stats.rs index c2f341ae..d96e7855 100644 --- a/Native/mod_stats/src/mod_stats.rs +++ b/Native/mod_stats/src/mod_stats.rs @@ -19,7 +19,7 @@ use std::thread::JoinHandle; use std::time::{SystemTime, Duration}; use std::collections::{HashMap, HashSet}; -use global_state::GLOBAL_STATE; +use global_state::{GLOBAL_STATE, LOADED_MODS}; use shared_dx::util::write_log_file; use util::mm_verify_load; @@ -361,12 +361,6 @@ pub fn update(now:&SystemTime) -> Option<(u32,u32)> { let elapsed = elapsed.unwrap_or_else(|| Duration::from_secs(0)); - let (total_frames, loaded_mods) = unsafe { - // this is the one place where I currently allow this warning, but it is pervasive in the code until I do - // something about it - unfortunately seems to be not an easy thing to fix. - (GLOBAL_STATE.metrics.total_frames, GLOBAL_STATE.loaded_mods.as_ref()) - }; - let send_thread_cmd = |lt:&Option,cmd:ThreadCommand| { if let Some(log_thread) = lt { if log_thread.thread.is_finished() { @@ -381,7 +375,8 @@ pub fn update(now:&SystemTime) -> Option<(u32,u32)> { // descend into the insane loaded mods structure to find active mods let mut new_active = 0_u32; let mut total_active = 0_u32; - loaded_mods + let total_frames = unsafe { GLOBAL_STATE.metrics.total_frames }; + unsafe { LOADED_MODS.as_ref() } .map(|m| m.mods.values() ) .map(|mlist| mlist.map(|m| m.iter().filter(|nmd| { @@ -732,7 +727,7 @@ mod tests { let set_mod_rendered = |name:&str, idx:usize| { let frame = unsafe { GLOBAL_STATE.metrics.total_frames }; - let lms = unsafe { GLOBAL_STATE.loaded_mods.as_mut().unwrap() }; + let lms = unsafe { LOADED_MODS.as_mut().unwrap() }; let mod_key = lms.mods_by_name.get(name).expect("mod not found"); let nmd = lms.mods.get_mut(mod_key).expect("mod not found").get_mut(idx).expect("mod not found"); nmd.d3d_data = ModD3DState::Loaded(ModD3DData::D3D11(ModD3DData11::new())); @@ -755,7 +750,7 @@ mod tests { mods_by_name: mods_by_name, selected_variant: global_state::new_fnv_map(16), }; - unsafe { GLOBAL_STATE.loaded_mods = Some(lms); }; + unsafe { LOADED_MODS = Some(lms); }; set_update_interval_ms(0); assert_eq!(update(&SystemTime::now()), Some((0,0))); set_mod_rendered("mod_100_200_2", 1); @@ -781,7 +776,7 @@ mod tests { std::thread::sleep(Duration::from_secs(1)); - unsafe { GLOBAL_STATE.loaded_mods = None }; + unsafe { LOADED_MODS = None }; super::reset(); } } \ No newline at end of file