veeenu / hudhook

A videogame overlay framework written in Rust, supporting DirectX and OpenGL
MIT License
207 stars 30 forks source link

I encountered a new issue related to input #212

Closed vsylva closed 1 week ago

vsylva commented 1 week ago

This may not be related to hudhook. The game itself cannot obtain any valid input from hudhook's inputs.rs and can only use the code below. Unfortunately, after playing for a while, the IMGUI panel and its components do not respond to mouse clicks. After removing inputs.rs, it runs well.

    unsafe fn before_render<'a>(
        &'a mut self,
        _ctx: &mut hudhook::imgui::Context,
        _render_context: &'a mut dyn hudhook::RenderContext,
    ) {
        static mut IS_KEY_OPEN_MENU_DOWN: bool = false;

        if GetAsyncKeyState(0xC0) & 0x8000u16 as i16 != 0 {
            if !IS_KEY_OPEN_MENU_DOWN {
                IS_KEY_OPEN_MENU_DOWN = true;
                self.is_menu_on = !self.is_menu_on;
            }
        } else if IS_KEY_OPEN_MENU_DOWN {
            IS_KEY_OPEN_MENU_DOWN = false;
        }

        if !self.is_menu_on {
            _ctx.io_mut().mouse_draw_cursor = false;
            return;
        }

        let mut mouse_pos: hudhook::windows::Win32::Foundation::POINT =
            hudhook::windows::Win32::Foundation::POINT {
                x: 0,
                y: 0,
            };

        let _ = GetCursorPos(&mut mouse_pos);
        let _ = ScreenToClient(self.game_window, &mut mouse_pos);

        _ctx.io_mut().mouse_draw_cursor = true;
        _ctx.io_mut().mouse_pos[0] = mouse_pos.x as f32;
        _ctx.io_mut().mouse_pos[1] = mouse_pos.y as f32;

        static mut IS_MOUSE_LEFT_DOWN: bool = false;

        if GetAsyncKeyState(0x1) & 0x8000u16 as i16 != 0 {
            IS_MOUSE_LEFT_DOWN = true;

            _ctx.io_mut().mouse_down[0] = true;
        } else {
            IS_MOUSE_LEFT_DOWN = false;

            _ctx.io_mut().mouse_down[0] = false;
        }
    }
veeenu commented 1 week ago

It sounds like the window procedure is not pumping messages. Maybe it's happening from a different thread, can't really say without debugging the game itself.

A couple observations about your code: you're using static muts in the render loop without any synchronization. You'd be better off putting IS_MOUSE_LEFT_DOWN and IS_KEY_OPEN_MENU_DOWN as fields in your RenderLoop struct to ensure soundness. I wouldn't be surprised if doing that would solve your issue entirely.

When using static muts in this manner, the Rust compiler is going to assume you have respected the borrow checker rules, which you haven't due to absence of synchronization, and decide accordingly on which optimized code paths to generate. This could very easily lead to a situation where the compiler says: "assuming that the borrow checker rules have been respected, I'm going to reorder these instructions in this way" which leads to logic errors you can't see from the code. That's why we make a massive use of interior mutability in hudhook's static muts (eg Mutexes).

Also, I noticed you changed before_render's signature to be unsafe -- why is that? It is supposed to be safe from the standpoint of its callsites. The right thing to do would be to add unsafe inline in your before_render implementation, but there is nothing unsafe there besides the static mut accesses.

Also note that static mutables are going to be a hard error in Rust 2024. We'll have to find workarounds in hudhook as well for that.

vsylva commented 1 week ago

It sounds like the window procedure is not pumping messages. Maybe it's happening from a different thread, can't really say without debugging the game itself.

A couple observations about your code: you're using static muts in the render loop without any synchronization. You'd be better off putting IS_MOUSE_LEFT_DOWN and IS_KEY_OPEN_MENU_DOWN as fields in your RenderLoop struct to ensure soundness. I wouldn't be surprised if doing that would solve your issue entirely.

When using static muts in this manner, the Rust compiler is going to assume you have respected the borrow checker rules, which you haven't due to absence of synchronization, and decide accordingly on which optimized code paths to generate. This could very easily lead to a situation where the compiler says: "assuming that the borrow checker rules have been respected, I'm going to reorder these instructions in this way" which leads to logic errors you can't see from the code. That's why we make a massive use of interior mutability in hudhook's static muts (eg Mutexes).

Also, I noticed you changed before_render's signature to be unsafe -- why is that? It is supposed to be safe from the standpoint of its callsites. The right thing to do would be to add unsafe inline in your before_render implementation, but there is nothing unsafe there besides the static mut accesses.

Also note that static mutables are going to be a hard error in Rust 2024. We'll have to find workarounds in hudhook as well for that.

Yeah, that makes sense! However, debugging is too troublesome, so i decided to use a custom version of hudhook and integrate it directly into the project, in order to reduce the death of brain cells. haha.

For hard error in Rust 2024 perhaps we could use RwLock or something else. There may be some performance loss, but for increasingly powerful hardware, it's negligible.

veeenu commented 1 week ago

For hard error in Rust 2024 perhaps we could use RwLock or something else. There may be some performance loss, but for increasingly powerful hardware, it's negligible.

parking_lot (which we already use) has mutexes and rwlocks that are basically free in the uncontended case, which is probably most of our use cases -- in most games we are only ever going to stay on a single rendering thread.