Fix some joystick problems #3

XuYicong · 2022-07-27T08:20:43Z

This PR focuses on the following problems with the joystick control:

After using the joystick, when pressing option to call the cursor out, a touch may be recognized at the center of the joystick.
For Genshin, after using the joystick, after openning the in-game map using a mapped key (the M key normally) and pressing option to call the cursor out, the map appears unable to be dragged.
When pressing and releasing a joystick key very quickly (in less than 0.04s, according to the code), the pressed key is not released correctly, and that key appears kept pressed.
When the placement position of the joystick is not perfectly aligned with the in-game joystick, at the start of a joystick touch, the in-game joystick may shortly (for 0.04s, according to the code) point to some certain direction before pointing to the user-pressed direction.
If multiple joystick keys are pressed, when releasing them simutaneously (which I can reproduce in Genshin), the joystick is not released correctly. The in-game joystick may keep pointing to some certain direction.

The causes of these problems are the following:

The touch phase of the joystick is never ended unless pressing option to switch visible mode. At which time, a touch action with "ended" touch phase is sent, which lets the game think there is a touch completing at that point.
Instead of ending the touch phase, the joystick "cancels" it in react to user's release of joystick keys. That touch phase was designed for cases such as when the device is too close to the face to track touch actions, and does not necessarily mean that touch point has completed normally. Genshin's in-game map always thinks there is a touch not released at that point even after an "ended" phase is sent later, preventing the user to drag the map.
The joystick starts its touch from the center, and sets a scheduled task to move to the edge 0.04 seconds later. However, it is possible that before the scheduled task is executed, the user had already released the key. In which case, the following touch move action would never end.
Some touch-screen game joysticks are not expected to be pressed at the center. For Genshin, there is actually no joystick center: it can always tell a direction from your touch point. That's why the starting "center" touch of joystick becomes some other direction.
The code checks for a "keyReleased" event only when it thinks some key is being pressed. However, when two keys are released simutaneously, there will be two "keyReleased" events. The latter one would start a new touch action at the joystick center.

How the problems are solved:

Change the "cancelled" phase to "ended".
The same as above
Before executing the scheduled task, check whether the touch action had ended
Move the start point of a joystick touch some distance away from the center, towards the user-pressed direction
Check for "keyReleased" events even no key is being pressed.

The modifications have been tested for Genshin. More tests are welcome for other games or applications.

FAQ:

Q: Why would the original code use the "cancelled" phase instead of an "ended" phase? How dare you change this?
A: The original author may aim to avoid triggering a "moved" phase after an "ended" phase. While a "moved" phase after a "cancelled" phase may cause less problems. In my modifications, the "moved" phase is sent only when it determines the touch had not ended. So the "cancelled" phase can be safely replaced with "ended" phase.

Q: You still got race problems. What if another thread is executed between the check and the following commands?
A: That is true. I have arranged the sequence of commands so that this situation is less possible to be met.

Q: Why don't you use locks or atomic actions to completely eliminate race conditions?
A: In consider of the performance. Moreover, similar race conditions exist everywhere in other classes or functions. Solely solve them here does not help a lot. And from the practical perspective, I never met cases like that in real playing.

Q: Why do you write so much? I have a headache reading this shit.
A: Sorry I'm not good at explaining things but I'm trying to express it more clear. You could just read the code, which is simpler, and only look for explanations when something does not make sense.

JoseMoreville · 2022-07-30T21:49:49Z

Hi, i'm going to review your changes, can you please create a release tag for your changes on your local repo?

XuYicong · 2022-07-31T12:27:36Z

@JoseMoreville
I have created a tag named "fix-joystick" at the last commit of this PR. Am I doing it as expected?

JoseMoreville · 2022-07-31T19:24:41Z

After using the joystick, when pressing option to call the cursor out, a touch may be recognized at the center of the joystick.

For Genshin, after using the joystick, after openning the in-game map using a mapped key (the M key normally) and pressing option to call the cursor out, the map appears unable to be dragged.

Hi, points one and two are still a problem.

After using the joystick, when pressing option to call the cursor out, a touch may be recognized at the center of the joystick.
For Genshin, after using the joystick, after openning the in-game map using a mapped key (the M key normally) and pressing option to call the cursor out, the map appears unable to be dragged.

I'm gonna tell more details and debug on discord

XuYicong · 2022-07-31T21:16:27Z

Sorry for not describing it clearly, my original description of the problems may be misleading. That two problems regarding the joystick are actually solved. But may be misunderstood as other problems not regarding the joystick. If you are running Genshin, I have the following specific steps to reproduce the two problems respectively:

Move around in the game. Then press the mapped key (usually B) to open the bag. Then press option to release cursor. On the "option" press, the item at the bottom left corner is selected.
Move around in the game. Then press the mapped key (usually M) to open the map. At this time, no matter what you do, you cannot drag the map. The map can be dragable only by closing and opening it again.

# This is the 1st commit message: Remove dead code # This is the commit message #2: Carthage didn’t like NSCursor # This is the commit message #3: add back some stuff # This is the commit message PlayCover#4: Add back more things # This is the commit message PlayCover#5: Add back more stuff

@objc

…s responsive fix on default (#77) * Fix for default screen * Code cleanup * setting @objc to scene control vars * PR to make default size work correctly (#1) * Reverting cleanup * Testing removing new scaling methods for default * Revert to new methods for testing * Setting orientation to 1 * 4 * commenting suspect stuff * 12 * sd * removing FBS * test * returning back to normal * Test * a * test * x * a * a * a * as * 1 * as * así * aa * as * Change names * testing something * testing 2 * more test * 12 * return * 123 * Testing defaults and auto fix * Testing c * is this ready? * test * 1 * returning adaptive * Delete PlayScreenDefault.swift * Update project.pbxproj Removing All references to PlayScreenDefault.swift * Adds fix for some apps with adaptive display enabled (#3) * testing changing to default bounds * Update project.pbxproj (#2) Removing All references to PlayScreenDefault.swift * test 2 * rest * test * as * asd * test * 123 * Adding inverseScreenValues to settings * Reduced code and added ternaries * added else when inverseScreenValues is false (might rework * Adds MacOSVersion to plist and a 'fix' for screen issue (#4) * testing changing to default bounds * Update project.pbxproj (#2) Removing All references to PlayScreenDefault.swift * test 2 * rest * test * as * asd * test * 123 * Adding inverseScreenValues to settings * Reduced code and added ternaries * added else when inverseScreenValues is false (might rework * Test to fix pre MacOS 13.2 * Check this out * Using float instead of string * v2 * test * lets see * adding print * NSLog(@"macOS version: %f", [[PlaySettings shared] macOSVersion]); * [[PlaySettings shared] macOSVersion] rounded * adding 00000 to every float * testing * return back to normal double * Forgot about floating point error * more than 13.1 * more * 13.199 * setting to latest macOS version * more floating error and macOS fixes * reduced lines and fix some errors * Update project.pbxproj * Update project.pbxproj * Update PlaySettings.swift * Enables responsiveness for some apps for default screen settings if toggle enabled (#5) * test disabling FBSDisplayMode * disabling FBS * Adding experimental fix toggle to fix responsiveness * adding an extra log * extra * moving the function to a better place * back to another place * added CheckResizabilityHasRun * Testing some stuff * name issue * added some comments to test * added fuction stop * True to yes * Changed yes to no * adding observer for resize * adding chess * test * back again * Removing extra check for a future commit * changed macos check to ios check * Remove macOSVersion

refactor: Replace bunch of State into ObservedObject

@objc

…s responsive fix on default (PlayCover#77) * Fix for default screen * Code cleanup * setting @objc to scene control vars * PR to make default size work correctly (#1) * Reverting cleanup * Testing removing new scaling methods for default * Revert to new methods for testing * Setting orientation to 1 * 4 * commenting suspect stuff * 12 * sd * removing FBS * test * returning back to normal * Test * a * test * x * a * a * a * as * 1 * as * así * aa * as * Change names * testing something * testing 2 * more test * 12 * return * 123 * Testing defaults and auto fix * Testing c * is this ready? * test * 1 * returning adaptive * Delete PlayScreenDefault.swift * Update project.pbxproj Removing All references to PlayScreenDefault.swift * Adds fix for some apps with adaptive display enabled (#3) * testing changing to default bounds * Update project.pbxproj (#2) Removing All references to PlayScreenDefault.swift * test 2 * rest * test * as * asd * test * 123 * Adding inverseScreenValues to settings * Reduced code and added ternaries * added else when inverseScreenValues is false (might rework * Adds MacOSVersion to plist and a 'fix' for screen issue (PlayCover#4) * testing changing to default bounds * Update project.pbxproj (#2) Removing All references to PlayScreenDefault.swift * test 2 * rest * test * as * asd * test * 123 * Adding inverseScreenValues to settings * Reduced code and added ternaries * added else when inverseScreenValues is false (might rework * Test to fix pre MacOS 13.2 * Check this out * Using float instead of string * v2 * test * lets see * adding print * NSLog(@"macOS version: %f", [[PlaySettings shared] macOSVersion]); * [[PlaySettings shared] macOSVersion] rounded * adding 00000 to every float * testing * return back to normal double * Forgot about floating point error * more than 13.1 * more * 13.199 * setting to latest macOS version * more floating error and macOS fixes * reduced lines and fix some errors * Update project.pbxproj * Update project.pbxproj * Update PlaySettings.swift * Enables responsiveness for some apps for default screen settings if toggle enabled (PlayCover#5) * test disabling FBSDisplayMode * disabling FBS * Adding experimental fix toggle to fix responsiveness * adding an extra log * extra * moving the function to a better place * back to another place * added CheckResizabilityHasRun * Testing some stuff * name issue * added some comments to test * added fuction stop * True to yes * Changed yes to no * adding observer for resize * adding chess * test * back again * Removing extra check for a future commit * changed macos check to ios check * Remove macOSVersion

@objc

…s responsive fix on default (PlayCover#77) * Fix for default screen * Code cleanup * setting @objc to scene control vars * PR to make default size work correctly (#1) * Reverting cleanup * Testing removing new scaling methods for default * Revert to new methods for testing * Setting orientation to 1 * 4 * commenting suspect stuff * 12 * sd * removing FBS * test * returning back to normal * Test * a * test * x * a * a * a * as * 1 * as * así * aa * as * Change names * testing something * testing 2 * more test * 12 * return * 123 * Testing defaults and auto fix * Testing c * is this ready? * test * 1 * returning adaptive * Delete PlayScreenDefault.swift * Update project.pbxproj Removing All references to PlayScreenDefault.swift * Adds fix for some apps with adaptive display enabled (#3) * testing changing to default bounds * Update project.pbxproj (#2) Removing All references to PlayScreenDefault.swift * test 2 * rest * test * as * asd * test * 123 * Adding inverseScreenValues to settings * Reduced code and added ternaries * added else when inverseScreenValues is false (might rework * Adds MacOSVersion to plist and a 'fix' for screen issue (PlayCover#4) * testing changing to default bounds * Update project.pbxproj (#2) Removing All references to PlayScreenDefault.swift * test 2 * rest * test * as * asd * test * 123 * Adding inverseScreenValues to settings * Reduced code and added ternaries * added else when inverseScreenValues is false (might rework * Test to fix pre MacOS 13.2 * Check this out * Using float instead of string * v2 * test * lets see * adding print * NSLog(@"macOS version: %f", [[PlaySettings shared] macOSVersion]); * [[PlaySettings shared] macOSVersion] rounded * adding 00000 to every float * testing * return back to normal double * Forgot about floating point error * more than 13.1 * more * 13.199 * setting to latest macOS version * more floating error and macOS fixes * reduced lines and fix some errors * Update project.pbxproj * Update project.pbxproj * Update PlaySettings.swift * Enables responsiveness for some apps for default screen settings if toggle enabled (PlayCover#5) * test disabling FBSDisplayMode * disabling FBS * Adding experimental fix toggle to fix responsiveness * adding an extra log * extra * moving the function to a better place * back to another place * added CheckResizabilityHasRun * Testing some stuff * name issue * added some comments to test * added fuction stop * True to yes * Changed yes to no * adding observer for resize * adding chess * test * back again * Removing extra check for a future commit * changed macos check to ios check * Remove macOSVersion

XuYicong added 2 commits July 27, 2022 14:07

fix some concurrent error in joystick

e779fc0

put moving status change closer to touch

c808069

Candygoblen123 approved these changes Jul 29, 2022

View reviewed changes

Candygoblen123 mentioned this pull request Aug 5, 2022

Release: 1.1.0 PlayCover/PlayCover#136

Closed

2 tasks

JoseMoreville merged commit 3af5920 into PlayCover:master Aug 6, 2022

XuYicong deleted the fix/joystick branch August 6, 2022 06:57

XuYicong referenced this pull request in XuYicong/PlayTools Mar 4, 2023

Merge pull request #3 from KhoraLee/refactor/appSetting

d648dfa

refactor: Replace bunch of State into ObservedObject

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix some joystick problems #3

Fix some joystick problems #3

Uh oh!

XuYicong commented Jul 27, 2022 •

edited

Loading

Uh oh!

JoseMoreville commented Jul 30, 2022 •

edited

Loading

Uh oh!

XuYicong commented Jul 31, 2022

Uh oh!

JoseMoreville commented Jul 31, 2022

Uh oh!

XuYicong commented Jul 31, 2022 •

edited

Loading

Uh oh!

Uh oh!

Fix some joystick problems #3

Fix some joystick problems #3

Uh oh!

Conversation

XuYicong commented Jul 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoseMoreville commented Jul 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XuYicong commented Jul 31, 2022

Uh oh!

JoseMoreville commented Jul 31, 2022

Uh oh!

XuYicong commented Jul 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

XuYicong commented Jul 27, 2022 •

edited

Loading

JoseMoreville commented Jul 30, 2022 •

edited

Loading

XuYicong commented Jul 31, 2022 •

edited

Loading