Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TripleAFrame.java - faster arrow scroll when meta keys are held down #56

Merged
merged 1 commit into from
Aug 25, 2015
Merged

TripleAFrame.java - faster arrow scroll when meta keys are held down #56

merged 1 commit into from
Aug 25, 2015

Conversation

DanVanAtta
Copy link
Member

For example, hold down shift while using arrow keys to scroll faster.

Review on Reviewable

@DanVanAtta
Copy link
Member Author

Feedback welcome, I'm starting to think it is better to have only control and shift as arrow key accelators and drop alt. Not sure about others, but using alt is not very natural, arrows keys and ctrl and or shift feel natural. I'm thinking of updating this so the old alt speed is control, control goes to shift, and the fast shift speed goes to when you're holding both shift and control.

@gaborbernat
Copy link
Member

don't have a strong opinion never had issue with scroll speed yet :| but what you describe can work, keep it as simple as possible I say :)

@DanVanAtta
Copy link
Member Author

To be clear this is for the arrow or a/s/d/w keys. Testing this out over some time, reaching over to the alt key was annoying when the other two are right next to each other. Happily the actual code change is easy. The values should be put into a settings the user can change, though at this point I'd like us to see a better system about user preferences before we tangle that web further.

@DanVanAtta DanVanAtta changed the title TripleAFrame.java - add multipliers to scroll speed if meta keys are held down TripleAFrame.java - faster arrow scroll when meta keys are held down Aug 6, 2015
@DanVanAtta
Copy link
Member Author

I updated this pull request so that only the control and shift keys will accelerate the arrow key scrolling and no longer the alt key. This PR is ready for review.

@ron-murhammer
Copy link
Member

Interesting. Here are my thoughts:

  1. I think it would be simpler and better to have just one accel key so either ctrl or shift
  2. I think you should update the 'Movement Help' window to add a note about using the functionality otherwise no one will ever know about it

@DanVanAtta
Copy link
Member Author

Thanks for the feedback. I'd prefer to update the doc notes in a follow-up PR leaving this as an incremental improvement.

I like the two keys compared to one for several reasons. I don't really agree that it is confusing, if it is then perhaps the scroll speeds are too similar. I'll skip the relatively long list I can think of why I prefer two buttons, but perhaps most notably with 2 buttons you get 4 different speed options. thinking of a map like NWO, or WaW, or even Big world, having the 3rd and 4th scroll speed setting is useful.

Ball is back in your court @ron-murhammer

@ron-murhammer
Copy link
Member

I'm ok leaving 2 keys if you think there is enough benefit. I think I just like simple :) Though if you want to leave 2 keys then IMO it would be better to adjust the speeds down a little. 15x is really fast and I didn't particularly find it useful. I really liked just the 4x and think it might be better to adjust them to be 2x, 4x, and 8x rather than 4x, 8x, and 15x. Thoughts?

I'm fine accepting the pull request without the 'movement help' update but in general I prefer if we try to update docs/tooltips/help/etc when we make changes otherwise they tend to get forgotten.

@DanVanAtta
Copy link
Member Author

TBH I found 4x and 8x to be useful. There needs to be enough of a speed difference for the keys to make sense, so the only 2-4x difference and 1x to 2x differences are not so great. 15x is not really intended to be used, and is supposed to be like a civ, get me half way round the map now! type of deal.

I'm thinking about playing around with 3x, 6x, and 9x. I'll also locate the notes and see about getting them included in this branch since there is more to do anyways.

Ball is still in my court...

@veqryn
Copy link
Member

veqryn commented Aug 14, 2015

We have soooo many hidden secret keyboard shortcuts & modifiers already as it is. People already think TripleA is too complex and non-user friendly.
How about just upping the default scroll speed by a bit and having only 1 modifier?

@DanVanAtta
Copy link
Member Author

On big world where I play tested for a few hours, I found 3 distinct useful speeds, slow, fast, faster (example, I'm looking at Japan, now I want to look asia, from japan looking at hawaii, and from japan looking at germany).

I do think 50 for the default is a bit slow and could be bumped up to 75 maybe. I do also see value in having a more fine grain control, nudge the map over type of speed. Since there are 3 useful speeds, we need at least 2 keys. That allows for a 4th speed, which is not very necessary, and certainly not more speeds are needed after that. So, I think we are at the sweet spot with 3 speeds and a 4th one to rule out what happens when someone holds both keys down.

Scroll speed increased from 50 to 100, control key multiplies by 5. Movement help notes updated so the line about arrow scrolling is slightly shorter and now mentions the control key multiplier.
@DanVanAtta
Copy link
Member Author

A new revision has been pushed:

  • Movement help notes now updated.
  • base scroll speed doubled from 50 to 100
  • only one multiplier key now, control, multiplies by 5

@ron-murhammer
Copy link
Member

Works well. Merging this.

ron-murhammer added a commit that referenced this pull request Aug 25, 2015
TripleAFrame.java - faster arrow scroll when meta keys are held down
@ron-murhammer ron-murhammer merged commit 6c2a029 into triplea-game:master Aug 25, 2015
@DanVanAtta DanVanAtta deleted the faster_arrow_scroll branch August 25, 2015 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants