Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,6 @@ SnapshotProfiles/TestProfile2.yaml
MMManaged.Engine/obj
MMManaged.Engine/bin
obj/fsac.cache
MMTricks/obj
MMTricks/obj
.paket/Paket.Restore.targets
testmodload*.txt
12 changes: 12 additions & 0 deletions DEVNOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
5 changes: 3 additions & 2 deletions Native/global_state/src/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ pub struct HookState {
pub clr: ClrState,
pub interop_state: Option<interop::InteropState>,
//pub is_global: bool,
pub loaded_mods: Option<LoadedModState>,
/// 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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -259,6 +257,9 @@ pub static mut GLOBAL_STATE: HookState = HookState {
};
pub static mut ANIM_SNAP_STATE:UnsafeCell<Option<AnimSnapState>> = UnsafeCell::new(None);

/// Loaded mod database
pub static mut LOADED_MODS: Option<LoadedModState> = None;

const TRACK_GS_PTR:bool = true;

/// Container structure providing access to the global state pointer.
Expand Down
4 changes: 2 additions & 2 deletions Native/hook_core/src/hook_render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions Native/hook_core/src/hook_render_d3d11.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions Native/hook_core/src/input_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
});
Expand All @@ -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);
});
Expand Down
4 changes: 2 additions & 2 deletions Native/mod_load/src/load_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions Native/mod_load/src/mod_load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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() {
Expand All @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 6 additions & 11 deletions Native/mod_stats/src/mod_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<LogThread>,cmd:ThreadCommand| {
if let Some(log_thread) = lt {
if log_thread.thread.is_finished() {
Expand All @@ -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| {
Expand Down Expand Up @@ -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()));
Expand All @@ -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);
Expand All @@ -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();
}
}
Loading