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

Core: Touch event xDelta and yDelta Fixes #525

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

scottrules44
Copy link
Contributor

Addresses #269

@Shchvova
Copy link
Contributor

Shchvova commented Mar 1, 2023

I am worried about this one... It would break existing games, wouldn't it?

@scottrules44
Copy link
Contributor Author

scottrules44 commented Mar 1, 2023

Well, due to the fact that event.xDelta and event.yDelta were implemented wrong, it is most likely they were ignored.
It is important to note the relatively new addition (xDelta and yDelta were added back in 2019)

A handful of people may have used the delta value and worked around the wrong values.

Either way, the values are wrong; I would argue that this fix does way more good than harm.

@scottrules44
Copy link
Contributor Author

scottrules44 commented Mar 1, 2023

Also I scanned all 44 cases publicly available games on Github and non of them should be affected by the change. It will mainly affect games that have implement a crazy workaround get xDelta/yDelta
https://github.com/search?l=Lua&p=5&q=event.xDelta&type=Code

@XeduR
Copy link
Contributor

XeduR commented Mar 1, 2023

I would agree with Scott on this one.

event.xDelta and event.yDelta were added to Solar2D via 68e90cb.

I would be genuinely surprised if anyone was using these properties. As the initial implementation was flawed, these properties gave correct values if and only if the content area perfectly matched the device/window resolution. Given what these properties are mostly used for, any developer would have run into issues with them and they would have either had to fall back to calculating the deltas themselves or writing a workaround.

The current way of calculating delta x and delta y in Lua that devs are likely using:

local function touchEvent( event )
  local dx = event.x - event.xStart
  local dy = event.y - event.yStart
  print( dx, dy )
end

And here's a workaround for using event.xDelta and event.yDelta, I think, I've never actually used this because the approach above is much easier, cleaner and faster:

local function touchEvent( event )
  local scaleFactor
  if string.find( system.orientation, "portrait" ) then
    scaleFactor = display.pixelWidth / display.actualContentWidth
  else
    scaleFactor = display.pixelWidth / display.actualContentHeight
  end

  local dx = event.xDelta / scaleFactor
  local dy = event.yDelta / scaleFactor
  print( dx, dy )
end

@Shchvova
Copy link
Contributor

Shchvova commented Mar 1, 2023

Ok. I'll test an merge then

@scottrules44
Copy link
Contributor Author

^Figured out there is a much simpler way to implement, also here attached a demo project you can test with, should compare the values
assets2.zip

print("Correct Delta x and y values:")
local dx = event.x - event.xStart
local dy = event.y - event.yStart
print( dx )
print( dy )
print("Delta x and y values:")
print(event.xDelta)
print(event.yDelta)

librtt/Rtt_Event.h Outdated Show resolved Hide resolved
@scottrules44
Copy link
Contributor Author

Any updates on this pull @Shchvova :) ?

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