Touch controls - #87
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pmndrs/racing-game/CGa3wYGcYyCpovMmzojxEx3v2Jj4 [Deployment for 0c37f76 failed] |
|
beautiful! can we make the minimap a little smaller? i also think the on screen hints have to go, it's too little space for that. |
|
I ain't sure, but this is targeted for portrait mode? I think the game is better played on landscape mode then portrait mode as the car itself will already take half of your screen. But good begin! |
I actually would prefer to disable the minimap on mobile as well.
One option would be to move the camera back a little more on mobile. |
Yeah that seems good to me. |
|
right, lets remove the minimap, makes no sense on a small screen. |
| } | ||
| .mobile-controls .left-turn, | ||
| .mobile-controls .right-turn { | ||
| -webkit-user-select: none; |
There was a problem hiding this comment.
These are all redundancy, user-select should cover all cases. It isn't like we should have to cover browsers from a decade ago(Luckily)
| } | ||
| } | ||
|
|
||
| @media (hover: hover) { |
There was a problem hiding this comment.
Hmm we are now going to rely into media queries to detect, touch devices? Well I have no experience with this specific media query and don't know the reliability of this. It should be hold in mind that it could break with edge cases.
There was a problem hiding this comment.
Thereby a detect if any keys are pressed(mobiles/tablets with keyboards) that we could add a non-touch class into the html and then as well hide these.
There was a problem hiding this comment.
I suggest we use a package to handle this: https://github.com/duskload/react-device-detect
so we don't have to worry about the edge cases
| useEffect(() => { | ||
| const downHandler = (e) => { | ||
| if (target.indexOf(e.target.value) !== -1) { | ||
| const isRepeating = !!pressed[e.target.value] |
There was a problem hiding this comment.
The value of this array is already a boolean or null no need to have the !! in check as in the if statement the browser will already check it for truthiness.
There was a problem hiding this comment.
The controls have changed since this PR has been opened
There was a problem hiding this comment.
Have all of the controls changed or specific controls that can get updated?
please list them.
How can we set this pr up for success? @njm222 , thanks
There was a problem hiding this comment.
From what I remember this is good to go, it just needs to be merged with main and resolve conflicts.
I would rebase onto main or force push a new branch since a lot has changed since this PR was opened.
Co-authored-by: Gusted <williamzijl7@hotmail.com>
|
is this ready to go? |
|
I don't think so |
Gusted
left a comment
Gusted
left a comment
There was a problem hiding this comment.
No more issues raised for me other then that little comment.
| <Help /> | ||
| <KeyboardControls /> | ||
| <HideMouse /> | ||
| {isMobile && <MobileControls />} |
There was a problem hiding this comment.
| {isMobile && <MobileControls />} | |
| {isMobile ? <MobileControls /> : <KeyboardControls />} |
| useKeys() | ||
| if (isMobile) { | ||
| useTouch() | ||
| } |
There was a problem hiding this comment.
| useKeys() | |
| if (isMobile) { | |
| useTouch() | |
| } | |
| if (isMobile) { | |
| useTouch() | |
| return | |
| } | |
| useKeys() |
There was a problem hiding this comment.
I would leave it like this so you can use both the touch and keyboard control on an iPad.
|
@motcodes is attempting to deploy a commit to the Poimandres Team on Vercel. A member of the Team first needs to authorize it. |
5 participants
I added some simple touch-based controls:
I moved the speedometer and controls menu to the top when using a touch-enabled device.
And after clicking "click to continue" I unmount the start menu to prevent text selection.