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

[RDY] Touch support for pinch zoom and move map #1260

Merged
merged 2 commits into from
Nov 17, 2017

Conversation

mugmuggy
Copy link
Contributor

I have access to a convertable device that I have tested quickly at this stage addressing touch controls.

Both pinch zoom and move map which is working mostly okay with this code.

What I don't have is a device specifically like that mentioned in #593 which may have this behave entirely different or interfere generally with use so would be nice to get this tested elsewhere.

Copy link
Member

@TheCycoONE TheCycoONE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mugmuggy. I don't have a device to test on, but it reads well short of a few minor style issues.

if numfingers == 2 then
-- calculate magnitude of pinch
local mag = math.abs(dDist)
if mag > 0.002 then
Copy link
Member

@TheCycoONE TheCycoONE Oct 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make 0.002 (mulitgesture_sensitivity_cutoff?) a variable (constant) defined near the top of the class. Let's avoid magic numbers.

local mag = math.abs(dDist)
if mag > 0.002 then
-- pinch action - constant needs to be tweaked
self.current_momentum.z = self.current_momentum.z + dDist * 100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100 is another magic number, so put it in a variable too.

@@ -649,6 +682,10 @@ function GameUI:onTick()
self.current_momentum.z = self.current_momentum.z * self.momentum
self.app.world:adjustZoom(self.current_momentum.z)
end
if self.multigesturemove.x ~= 0.0 then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

condition isn't necessary, just set them to 0.

@@ -600,6 +601,38 @@ function GameUI:onMouseUp(code, x, y)
return UI.onMouseUp(self, code, x, y)
end

function GameUI:onMultiGesture(numfingers, dTheta, dDist, x, y)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add documentation to new functions.

@mugmuggy mugmuggy changed the title [WIP] Touch support for pinch zoom and move map [RDY] Touch support for pinch zoom and move map Oct 29, 2017
@TheCycoONE TheCycoONE changed the title [RDY] Touch support for pinch zoom and move map [RFC] Touch support for pinch zoom and move map Nov 3, 2017
@TheCycoONE
Copy link
Member

I'd still like to see this one tested more before it lands. I'll see if I can get my hands on any devices. I think @wolfy1339 has one?

@wolfy1339
Copy link
Contributor

wolfy1339 commented Nov 3, 2017 via email

@TheCycoONE
Copy link
Member

TheCycoONE commented Nov 3, 2017

I can't seem to generate those events on my linux chromebook, both the touchscreen and the touchpad only make normal mouse events.

@wolfy1339 testing even with the razer blade would be helpful.

@wolfy1339
Copy link
Contributor

wolfy1339 commented Nov 17, 2017

Just tested it
👍 📦

@TheCycoONE TheCycoONE changed the title [RFC] Touch support for pinch zoom and move map [RDY] Touch support for pinch zoom and move map Nov 17, 2017
@TheCycoONE TheCycoONE merged commit b68864a into CorsixTH:master Nov 17, 2017
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.

3 participants