Support for pitching the camera #79

Merged
merged 9 commits into from Jan 10, 2017

Projects

None yet

3 participants

@Herbstein
Collaborator

This was a fairly simple implementation. Right now it uses the rotation modifier keys and speed, but I can add separate settings if that is needed.

Do note that I, unfortunately, ran rustfmt on src/core/ui.rs. I can revert and re-implement, if you'd prefer that.

@aeickhoff
Member

Already getting the cinematic stuff going, I see. Can't you just revert all the unnecessarily changed lines that don't have anything to do?

Also would be nice if you could either not add any default keybindings at all or make them pretty obscure, so you don't accidentally tilt the camera.

@Herbstein
Collaborator

Just to be clear, this is for looking up and down with the camera, not tilting it sideways. Tilt might have been a bad descriptor. I decided to implement this mostly because I felt that the default angle was too big.

I can definitely remove any usage from the game code itself, but I would personally find it an incredibly useful feature to have.

@aeickhoff
Member
aeickhoff commented Jan 9, 2017 edited

Oh I see, confusing indeed. Maybe "pitch" like for planes?

...and yes, that is much more useful.

@Herbstein
Collaborator

That is a far better name for it. I'll change the naming scheme in the source right away.

We also currently have yaw implemented, but it's simply called 'rotation'. Should I rename that too?

@aeickhoff
Member

Yeah that would be cool and consistent. Thank you!

@Herbstein Herbstein changed the title from Support for tiliting the camera to Support for pitching the camera Jan 9, 2017
@Herbstein
Collaborator

This wasn't the place for it, but I do think we should run rustfmt over the source soon-ish. Most editors should be configurable to run the command every time a file is saved; I personally know that Visual Studio Code + Rusty Code supports this natively. Making sure to adhere to rustfmt would result in way cleaner, and more consistent, code.

@Herbstein
Collaborator
Herbstein commented Jan 10, 2017 edited

Because of the way camera translation works, panning to almost-horizontal makes the camera move very slowly. I don't think there's much we can do right now, but we should be aware of the issue.

Maybe movement speed should be based on the cameras distance to the target instead of the cameras height, since the reason for the original feature was different speeds at different zoom-levels.

@Herbstein
Collaborator

It seems to work fairly well, but I'm not sure if the camera is a tad bit too slow. At least that is easily changeable.

@kingoflolz
Collaborator

Maybe a separate modifier key can be added, and bound by default to the same as the rotational modifier keys, for people who want them separated.

@Herbstein
Collaborator
Herbstein commented Jan 10, 2017 edited

That is easily added. The only problem with that is that we check for mouse input in an if ... else if ... else expression. Only one of the modifiers can used at a time. Do we rely on that fact, or is that just baggage?

After testing: Removing the else clauses definitely wasn't the solution. I think we need a slightly more robust way of handling movement modifiers.

@kingoflolz
Collaborator
kingoflolz commented Jan 10, 2017 edited

I think the problem is that this clause https://github.com/citybound/citybound/blob/master/src/core/ui.rs#L335 needs to be triggered if and only if none of the other branches execute.
Maybe set a flag in each if branch?

@Herbstein
Collaborator

I decided to do it slightly differently, but the result is ultimately the same. Any other comments/ideas?

@kingoflolz
Collaborator

Looks great to me!

@aeickhoff
Member

Just tried it, works great, thank you!

@aeickhoff aeickhoff merged commit 8109336 into citybound:master Jan 10, 2017

1 check passed

clahub All contributors have signed the Contributor License Agreement.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment