Changed camera controls to a more traditional RTS style #53

Merged
merged 8 commits into from Jan 9, 2017

Projects

None yet

4 participants

@kingoflolz
Collaborator

Sorry if I'm treading on your toes, but I saw some comments on Reddit complaining about the UI and controls, so I changed the camera controls to feel more natural for me, to be more of an RTS style. New controls are:

  • WASD or shift + mouse move to pan
  • middle click + drag to rotate
  • Scroll to zoom (also, scroll events were not being fired before for me in Linux because I think Linux uses MouseScrollDelta::LineDelta instead of MouseScrollDelta::PixelDelta, so I updated the code handling that so both should work now, untested in Windows or Mac)

Also, as theoretically what keyboard and mouse events do can be changed by the current keys and buttons held down, I updated the input handling so the Key and Mouse events mutate the InputState struct in UserInterface, and the actual update is performed when an UIUpdate message is received, after all the input is stored into InputState.

Ben Wang

@MiniDigger

mmh. I think some like the current control scheme and some like a more traditional one.
How about a config option to switch between those two control schemes?

@aeickhoff
Member

Let me just say that I'm super impressed that you managed to add to and even improve the camera control code like this. From my first look at it it seems to even already go in a direction where control meanings are nicely separated from the actual bindings! I hope I get a chance to properly review and merge soon!

@pfrenssen

Works great! Much easier than how it was before. Any chance to get the rotation on the right mouse button instead of the middle one?

@kingoflolz
Collaborator

I'll work on getting the ability to rebind keys from a config file, so everyone can have their favourite controls :P

@kingoflolz
Collaborator
kingoflolz commented Dec 27, 2016 edited

Settings implemented, although the serialization of VirtualKeyCode and MouseButton is a massive hack because Glutin does not have a serialize or deserialize traits for their enums. If we can live with an in tree Glutin, we should have much cleaner code for that, and have human readable key names.

FYI I created an issue on Glutin to maybe add support for that in the future.

@pfrenssen you can change the rotation key binding now by changing "rotate_modifier_key":[1002] in config.json to "rotate_modifier_key":[1003]

@kingoflolz
Collaborator
kingoflolz commented Dec 27, 2016 edited

Key codes list is at https://docs.google.com/spreadsheets/d/1DbPnkO-FlBCZVBkUsFAFPnxQREReBKM5chDN7-cE6Mc/edit?usp=sharing

Also, I have never played with invert Y before, does it invert when panning?

@kingoflolz kingoflolz referenced this pull request Dec 27, 2016
Open

Create Options Menu #51

0 of 41 tasks complete
@pfrenssen

Config settings work great! I also tried if it was possible to bind the panning on a mouse button, and that works very smoothly too.

I saw a few compiler warnings now though, I'm guessing you intend to clean these up when you finish the PR.

warning: returning the result of a let binding from a block. Consider returning the expression directly., #[warn(let_and_return)] on by default
  0 {"rotation_speed":1.0,"move_speed":1.0,"zoom_speed":1.0,"invert_y":false,"mouse_main":[1001],"forward_key":[32,62],"backwar  --> src/core/settings.rs:80:9
   |
80 |         ret
   |         ^^^
   |
note: this expression can be directly returned
  --> src/core/settings.rs:79:29
   |
79 |         let ret: Settings = serde_json::from_str(&s).unwrap();
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#let_and_return

warning: unused variable: `msg`, #[warn(unused_variables)] on by default
   --> src/core/ui.rs:318:27
    |
318 |     fn receive(&mut self, msg: &UIUpdate) -> Fate {
    |                           ^^^
@kingoflolz
Collaborator

Fixed compiler warnings. I architectured it so it is possible to bind any function either the mouse or keyboard, so if you want, you can bind a mouse button to go forwards as well :P

@aeickhoff
Member

First, @kingoflolz, since this pull request was started before I added CLAHub integration, could you please sign the Contributor License Agreement: https://www.clahub.com/agreements/citybound/citybound

Thank you!

@kingoflolz
Collaborator

Done!

@aeickhoff
Member
aeickhoff commented Jan 9, 2017 edited

So although this is quite hacky and has some limitations, I'll merge it, since it's better for more people than my hardcoded controls and already a little configurable. I'll create new issues regarding further goals for the controls.

๐ŸŽ‰ THIS IS THE FIRST SERIOUS PULL-REQUEST TO CITYBOUND TO EVER BE MERGED! ๐ŸŽ‰

@aeickhoff aeickhoff merged commit 9d22a6e into citybound:master Jan 9, 2017

1 check passed

clahub All contributors have signed the Contributor License Agreement.
Details
@kingoflolz
Collaborator

Haha I am new to rust and did not know how to shut it up except for #allow

@aeickhoff
Member

Even more impressive if you're new! In my experience of also still being quite new to Rust, it's really worthwhile to follow what Clippy says and in many cases trying to follow it even teaches you better Rust!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment