SimulaVR / Simula

Linux VR Desktop
MIT License
2.91k stars 87 forks source link

Mouse wheel scrolling should be customizable in `./config/config.dhall` #149

Closed georgewsinger closed 2 years ago

georgewsinger commented 2 years ago

The calls to

         (Just gsvs, G.BUTTON_WHEEL_UP) -> G.pointer_notify_axis_continuous wlrSeat 0 (0.05)
         (Just gsvs, G.BUTTON_WHEEL_DOWN) -> G.pointer_notify_axis_continuous wlrSeat 0 (-0.05)

in with hard-coded 0.05 values in SimulaServer.hs should be replaced with something like

  , _axisScrollSpeed    = 0.05 : Double

in our ./config/config.dhall.

CaydenPierce commented 2 years ago

I will try my hand at getting this working with a hard coded value in ./config/config.dhall start.

This should work fine because users can play with the value in config.dhall and hot reload settings with Super + r

The next step would be a GUI editor with sliders, toggles, etc. that would directly edit the config.dhall. This may be more than is needed now.

georgewsinger commented 2 years ago

It might be a good idea to add a config.dhall flag for mouse sensitivity also: _mouseSensitivity : Double? There might be a flag to adjust mouse sensitivity in Godot that we can route this factor to.

CaydenPierce commented 2 years ago

It might be a good idea to add a config.dhall flag for mouse sensitivity also: _mouseSensitivity : Double? There might be a flag to adjust mouse sensitivity in Godot that we can route this factor to.

Giving the option for mouse sensitivity makes sense.

There is no a preset flag in Godot to set this value. What the docs outline is multiplying mouse inputs by a (linear) scaling factor before passing them as a mouse move event.

This is demonstrated here: [https://docs.godotengine.org/en/stable/tutorials/3d/fps_tutorial/part_one.html] Specifically the sensitivity is set in line:

var MOUSE_SENSITIVITY = 0.05
CaydenPierce commented 2 years ago

So let's add a scale _mouseSensitivity : Double to the config and scale dx and dy by that value in SimulaServer.hs here:

 whenM (event `isClass` "InputEventMouseMotion") $ do
     mouseRelativeGV2 <- G.get_relative event :: IO GodotVector2
     mouseRelative@(V2 dx dy) <- fromLowLevel mouseRelativeGV2
     case maybeActiveGSVS of
         Nothing -> return ()
         (Just gsvs) -> do updateCursorStateRelative gsvs dx dy
                           sendWlrootsMotion gsvs

@georgewsinger

CaydenPierce commented 2 years ago

@KaneTW @georgewsinger

I understand the path to get this working:

** This is 90% done. Just don't have the Haskell know how to wrap it up:

When adding a new element to the GodotSimulaServer thing that looks like an object (axisScrollSpeed), would I use just a Double, or should everything be wrapped in this TVar, so TVar Double?

How do I access that Double properly in whenM (eventisClass"InputEventMouseButton") $ do? **

Current attempt:

In initGodotSimulaServer:

let axisScrollSpeedVal = (configuration ^. axisScrollSpeed)
gssAxisScrollSpeed' <- newTVarIO axisScrollSpeedVal
...
, _gssAxisScrollSpeed       = gssAxisScrollSpeed'       :: TVar Double -- in gss definition

In whenM (eventisClass"InputEventMouseButton") $ do:

axisScrollSpeed <- readTVarIO (gss ^. gssAxisScrollSpeed)
print axisScrollSpeed -- this properly returns the value saved in config.dhall
...
(Just gsvs, G.BUTTON_WHEEL_UP) -> G.pointer_notify_axis_continuous wlrSeat 0 (axisScrollSpeed)

This fails with:

Couldn't find method "pointer_notify_axis_continuous" with signature `t0
                                                                            -> Double -> IO ()'.

I understand I need to pass a Double as third argument... but how to get axisScrollSpeed as just a double in this context?

CaydenPierce commented 2 years ago

Alternatively, should pointer_notify_axis_continuous be modified to accept IO as third arg?

georgewsinger commented 2 years ago
CaydenPierce commented 2 years ago
  • Your approach across the board looks sound.
  • Probably use TVar Float instead of TVar Double. The Configuration datastructure can still use Double, since I believe that's what dhall expects when it parses decimal numbers. But I think pointer_notify_axis_continuous expects a Float.
  • You might run into number casting issues (something Haskell is notoriously finnicky about). You can use realToFrac to go from Double to Float.

Thanks. Will do. Where I am stuck is the Monad/functor/IO problem of accessing that number in the do-block here:

(Just gsvs, G.BUTTON_WHEEL_UP) -> G.pointer_notify_axis_continuous wlrSeat 0 (axisScrollSpeed)

I don't now how to access gssAxisScrollSpeed such that it's a Double or Float and not IO () inside that do-block

georgewsinger commented 2 years ago

Does

axisScrollSpeed <- readTVarIO (gss ^. gssAxisScrollSpeed)
G.pointer_notify_axis_continuous wlrSeat 0 axisScrollSpeed

not work?

CaydenPierce commented 2 years ago

Does

axisScrollSpeed <- readTVarIO (gss ^. gssAxisScrollSpeed)
G.pointer_notify_axis_continuous wlrSeat 0 axisScrollSpeed

not work?

Correct, this is not working because axisScrollSpeed <- readTVarIO (gss ^. gssAxisScrollSpeed) makes axisScrollSpeed an IO, and so building throws the error:

src/Plugin/SimulaServer.hs:953:44: error:
    • Couldn't find method "pointer_notify_axis_continuous" with signature `t0
                                                                            -> Double -> IO ()'.
    • In the expression:
        G.pointer_notify_axis_continuous wlrSeat 0 axisScrollSpeed
      In a case alternative:
          (Just gsvs, BUTTON_WHEEL_UP)
            -> G.pointer_notify_axis_continuous wlrSeat 0 axisScrollSpeed
      In a stmt of a 'do' block:
        case (maybeActiveGSVS, button) of
          (Just gsvs, BUTTON_WHEEL_UP)
            -> G.pointer_notify_axis_continuous wlrSeat 0 axisScrollSpeed
          (Just gsvs, BUTTON_WHEEL_DOWN)
            -> G.pointer_notify_axis_continuous wlrSeat 0 (- 1.0 * 0.05)
          (Just gsvs, _)
            -> do screenshotMode <- readTVarIO (gsvs ^. gsvsScreenshotMode)
                  case screenshotMode of
                    False -> ...
                    True -> ...
          (Nothing, _) -> return ()
    |
953 |          (Just gsvs, G.BUTTON_WHEEL_UP) -> G.pointer_notify_axis_continuous wlrSeat 0 axisScrollSpeed
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Here is that function I modified:

  whenM (event `isClass` "InputEventMouseButton") $ do
    let event' = GodotInputEventMouseButton (coerce event)
    pressed <- G.is_pressed event'
    button <- G.get_button_index event'
    wlrSeat <- readTVarIO (gss ^. gssWlrSeat)
    axisScrollSpeed <- readTVarIO (gss ^. gssAxisScrollSpeed)
    case (maybeActiveGSVS, button) of
         (Just gsvs, G.BUTTON_WHEEL_UP) -> G.pointer_notify_axis_continuous wlrSeat 0 axisScrollSpeed
         (Just gsvs, G.BUTTON_WHEEL_DOWN) -> G.pointer_notify_axis_continuous wlrSeat 0 (-1.0 * 0.05)
         (Just gsvs, _) -> do
           screenshotMode <- readTVarIO (gsvs ^. gsvsScreenshotMode)
           case screenshotMode of
             False -> do activeGSVSCursorPos@(SurfaceLocalCoordinates (sx, sy)) <- readTVarIO (gsvs ^. gsvsCursorCoordinates)
                         processClickEvent' gsvs (Button pressed button) activeGSVSCursorPos
             True -> do activeGSVSCursorPos@(SurfaceLocalCoordinates (sx, sy)) <- readTVarIO (gsvs ^. gsvsCursorCoordinates)
                        case pressed of
                          True -> do atomically $ writeTVar (gsvs ^. gsvsScreenshotCoords) (Just activeGSVSCursorPos, Nothing)
                          False -> do screenshotCoords@(origin, end) <- readTVarIO (gsvs ^. gsvsScreenshotCoords)
                                      atomically $ writeTVar (gsvs ^. gsvsScreenshotCoords) (origin, Just activeGSVSCursorPos)
         (Nothing, _) -> return ()

Not sure how Monads+IO work here so not sure how to just get a Double or Float as opposed to IO

georgewsinger commented 2 years ago

axisScrollSpeed is a Double, not an IO Double. I believe it should be cast into a Float when passed into G.pointer_notify_axis_continuous. You can cast it via (realToFrac axisScrollSpeed).

CaydenPierce commented 2 years ago

axisScrollSpeed is a Double, not an IO Double. I believe it should be cast into a Float when passed into G.pointer_notify_axis_continuous. You can cast it via (realToFrac axisScrollSpeed).

Yes that was it. Mouse scroll speed control in Simula is working now and editable in ./config/config.dhall upon restart.

Hot reload of the setting is not yet working. Will need to change the following function to include an update of axisScrollSpeed:

        reloadConfig _ True = do
          putStrLn "Reloading Simula config.."
          configuration <- parseConfiguration
          atomically $ writeTVar (gss ^. gssConfiguration) configuration
          let keyboardShortcutsVal = getKeyboardShortcuts gss (configuration ^. keyBindings)
          atomically $ writeTVar (gss ^. gssKeyboardShortcuts) keyboardShortcutsVal
          let keyboardRemappingsVal = getKeyboardRemappings gss (configuration ^. keyRemappings)
          atomically $ writeTVar (gss ^. gssKeyboardRemappings) keyboardRemappingsVal
          worldEnv@(worldEnvironment, _) <- readTVarIO (gss ^. gssWorldEnvironment)
          textures <- loadEnvironmentTextures configuration worldEnvironment
          atomically $ writeTVar (gss ^. gssEnvironmentTextures) textures
        reloadConfig _ _ = return ()

Which I believe will require making axisScrollSpeed a TVar

CaydenPierce commented 2 years ago

Conversion of axisScrollSpeed to TVar Double complete. reloadConfig function now updating axisScrollSpeed. Hot reload working.

CaydenPierce commented 2 years ago

Here's that work: https://github.com/CaydenPierce/Simula/commit/ad054b8088acddc542cf2fc4fc257a9d8aa131e1

Will add mouse speed customization now.

CaydenPierce commented 2 years ago

Mouse movement speed is done too.

PR here: https://github.com/SimulaVR/Simula/pull/151

georgewsinger commented 2 years ago

Nice work.