induktio / thinker

Improved game engine features for SMACX.
https://discord.gg/XdFuwWzzku
GNU General Public License v2.0
75 stars 11 forks source link

Mousewheel moves units #48

Closed DrazharLn closed 6 months ago

DrazharLn commented 1 year ago

Hi, scrolling the mouse wheel now sends keyboard up and keyboard down events, which is great for scrolling menus and stuff, but isn't what I expected on the main map view (where it orders units to move up or down).

I think the scrollwheel should either be disabled on the map screen or be remapped to zoom in and out.

I tried compiling Thinker myself and debugging it, but it exits immediately if I try to run it under gdb (but works fine otherwise). I was able to confirm that map_is_visible() is returning false when the map is visible, though.

induktio commented 1 year ago

Yeah, mousewheel is not supposed to work like that in the game. There's some leftover code causing this that should have been cleaned up earlier. It could be remapped to map zoom function which seems pretty useful.

DrazharLn commented 1 year ago

The original code in PRACX works fine and was always supposed to have mousewheel cause zooming.

Looks like the issue is that map_is_visible expects CState.TimersEnabled to be true if the map is visible, but it's not. In the current code the timers are only enabled in mod_blink_timer() and only if some complicated conditionals are met.

Editing map_is_visible so that it doesn't check CState.TimersEnabled makes zooming work in the map for me, but PRACX does check if the timers are enabled (m_fGlobalTimersEnabled in PRACX) and that seems to work fine. Perhaps your hook function mod_blink_timer is broken?

induktio commented 1 year ago

I'm not sure if you're running the game with smooth_scrolling enabled or not but this option (which is based on Pracx) was not enabled by default since I noticed it affects scrolling speed noticeably and can make it too slow to be satisfactory when there's lots of stuff to be rendered. The rendering in original engine for map scrolling seems much faster.

The hooks for any of the smooth_scrolling functions won't get activated in patch_setup unless the feature is enabled, so maybe that's why you're not seeing mod_blink_timer working. The hooks for WinProc do always get activated because it's needed for other stuff too, but it also affects mouse wheel scrolling here. I would probably modify the mouse wheel scrolling behaviour but keep smooth_scrolling still disabled by default.

DrazharLn commented 1 year ago

I'm just using the default thinker.ini. It sounds like a bug if zooming with the mousewheel doesn't work unless you have smooth scrolling turned on.

It also seems a bit dodgy to have a function that sets TimersEnabled to true, but not one that sets it to false. In the current code it defaults to false and then might be set to true at some point, which seems like a bad signal of whether or not the map is visible, but also maybe just a sign that the semantics from PRACX have been lost and this variable just doesn't mean the same thing it does in PRACX (in PRACX there are hooks for both starting and stopping timers, not just for starting timers).

I can do a PR that strips out the TimersEnabled code, if you like, seeing as it doesn't seem to work anyway. That would also fix the mousewheel zooming.

induktio commented 1 year ago

The issue here relates to the fact that some hooks (including mod_blink_timer) in patch_setup only get activated when smooth_scrolling is enabled but the code for mouse wheel events runs always regardless. With default config ini smooth_scrolling should be disabled. What would be needed is some way to decouple mouse wheel scrolling from the rest of smooth_scrolling features. Mouse wheel scrolling is still very useful so I'd rather keep it in the game but it could have a separate config option which could be even toggled from GUI menus.

DrazharLn commented 1 year ago

Yeah, I'm saying that mouse wheel scrolling and zooming both seem to work fine (scrolls in some menus, zooms on the map) if map_is_visible() is changed to ignore the status of CState.TimersEnabled. I'm also saying that I think that TimersEnabled is probably broken anyway because it is never turned off, only on.

induktio commented 1 year ago

If you pull the latest commit, it should fix mouse wheel zooming and refactor some code. Additionally zooming should not activate when there's other windows open on top the world map. Mouse wheel zooming is now always enabled even if smooth_scrolling is disabled. Let me know if you see any issues with the implementation.

induktio commented 1 year ago

Okay, uploaded a new build which fixes a small issue with file open/save dialogs. The mod is now able to check if any file dialog is open and allow scrolling the file list instead of zooming the map. With Pracx+vanilla it looks like the file list scrolling works only on the main menu, not when the world map is open because it just zooms the map there instead. So this could be an useful addition in any case.