Kruptein / PlanarAlly

A companion tool for when you travel into the planes.
https://www.planarally.io/
MIT License
402 stars 73 forks source link

Support Touch devices/Touch interface #144

Closed schemen closed 4 years ago

schemen commented 5 years ago

I would love to be able to use this app on my iPad to cleanly use everything on a single device. Any chance to have a look at that?

Cheers!

Kruptein commented 5 years ago

Hey, I understand the usecase and would like to get this working but I don't have any tablet device myself. I do believe I can emulate it with chrome devtools, I'll see how that goes.

schemen commented 5 years ago

No worries! I'll also be testing it all the time when you need help - I can deploy a testing branch onto dndbox if you want :)

ZachMyers3 commented 4 years ago

I'm using PA on a touch device, so I'd like to look at implementing it.

It looks like there's a vue2-touch-events library that would make this possible. I'll play around with it on my fork and see what I can do.

Kruptein commented 4 years ago

That would be great!

You could also look into the general touch events.(https://developer.mozilla.org/en-US/docs/Web/API/TouchEvent). These should probably also be attached to the current mouse event functions

ZachMyers3 commented 4 years ago

I took your advise and dug into the TouchEvents.

It looks like every component that has a MouseEvent event handler should have a TouchEvent handler as well. I'm still getting familiar with the codebase but I think this is the work list I was able to come up with.

I don't think this list is exhaustive, and my limited testing shows that mouseup and mousedown event works, but mousemove and other events either don't work or work sub-optimally.

The only other way I could think of this is to handle the incoming TouchEvent within game.vue and convert it to a MouseEvent somehow, but that seems like a hack rather than an implementation. Let me know what you think.

ZachMyers3 commented 4 years ago

I've created a proof of concept for touch on my fork, see the changes here.

Currently only implemented panning on TouchMove, didn't want to get any further before I got your thoughts.

Thanks!

Kruptein commented 4 years ago

I did a quick test with the chrome device emulation and the panning behaves kinda odd. The root cause of this is that there is no touch equivalent of the onMouseDown in the pan tool currently. That function sets the panStart point, which now stays at 0,0.

ZachMyers3 commented 4 years ago

I'll work on getting touchDown to work in an equivalent way to mouseDown.

I noticed the error as well and couldn't quite get my finger on it.

As far as formatting and code style, does the writing style seem okay? Don't want to muddy the codebase.

Thanks

ZachMyers3 commented 4 years ago

Added touchstart and touchend events into the game canvas, propagated the event all the way to pan to mirror mouseup/mousedown. On my latest commit here.

Let me know what you think!

Kruptein commented 4 years ago

Currently at work, but I'll check it out later! Don't worry to much about 'muddy' code, if something stands out I'll give some suggestions and it's not like my current code is the paradigm of clean code anyway :)

ZachMyers3 commented 4 years ago

I've expanded the pan implementation to all the other tools, everything except shapes appear to be functioning well.

I think I'd like to add some more complex gestures (pinch to zoom, three fingers to auto pan, etc) to make it a little cleaner.

I also refactored the utility getMouse() because touch events don't have the same syntax to get the pointer location. I created two additional functions getTouch() and getLocalPointFromEvent().

export function getLocalPointFromEvent(e: MouseEvent | TouchEvent): LocalPoint {
    if (e instanceof MouseEvent) {
        return getMouse(e);
    } else {
        return getTouch(e);
    }
}

export function getMouse(e: MouseEvent): LocalPoint {
    return new LocalPoint(e.pageX, e.pageY);
}

// takes a given touch event and converts to LocalPoint
export function getTouch(e: TouchEvent): LocalPoint {
    // touches is a TouchList, which is a list of touches (for each finger)
    // default to first touch (first index) to get x/y
    return new LocalPoint(e.touches[0].pageX, e.touches[0].pageY);
}

You can look in client/src/game/ui/tools/ for the builk of the changes.

Kruptein commented 4 years ago

I gave it another test and it seems to behave nice! The other tools also seem to be working.

The pinch to zoom and three fingers auto pan are indeed something that would be nice. I'm unsure how I'm going to test that, but I'll find some tablet somewhere :)

I would just ditch the onMouseDown, onTouchDown etc and just immediately call the onDown from game.vue and this for all these functions that do nothing extra. I'm still deciding whether I would rename it to something akin to onInputDown or keep it as is.

Good work so far!

ZachMyers3 commented 4 years ago

My only concern with doing something like that is if there ever was a tool that needed to act different depending on a mouse or touch event. In that case onUp/onDown wouldn't account for that if it were to be called directly from game.vue.

There would need to be more refactoring in tool.vue and tools.vue to handle MouseEvent and TouchEvent inside of the new onUp, onDown and onMove events.

Kruptein commented 4 years ago

If you feel like the chances are realistic that the separate functions are needed, I'm fine with leaving them as is.

ZachMyers3 commented 4 years ago

Looking at the more complex events, I was looking at modifying tool.vue and firing different events based on the TouchEvent input. This is what I have in concept rights now.

        this.$parent.$on("touchstart", (event: TouchEvent, tool: string) => {
            // a different tool cant trigger anothers event
            if (tool !== this.name) {
                return;
            }
            if (event.touches.length === 2) {
                this.scaling = true;
                this.onPinchStart(event);
            } else {
                this.onTouchStart(event);
            }
        });
        this.$parent.$on("touchend", (event: TouchEvent, tool: string) => {
            // a different tool cant trigger anothers event
            if (tool !== this.name) {
                return;
            }

            if (this.scaling) {
                this.onPinchEnd(event);
                this.scaling = false;
            } else {
                this.onTouchEnd(event);
            }
        });
        this.$parent.$on("touchmove", (event: TouchEvent, tool: string) => {
            // a different tool cant trigger anothers event
            if (tool !== this.name) {
                return;
            }

            if (this.scaling) {
                this.onPinchMove(event);
            }

            // determine the number of fingers on screen to trigger different events
            if (event.touches.length >= 3) {
                this.onThreeTouchMove(event);
            } else {
                this.onTouchMove(event);
            }
        });

I know in other situations (like mouse button to pan) you implanted it within the pan tool, instead of triggering a different event. Just curious on your thoughts on doing it this way.

Kruptein commented 4 years ago

For now this is fine to further test with. I do foresee some refactor in the future as this begins to clutter up in both the tool.vue file as well as the event bus itself. It's probably better if in the future the tools.vue file could call functions on the tools immediately instead of having to go through the eventbus.

ZachMyers3 commented 4 years ago

Could you elaborate on this a little more? A lot of my background is backend so the event bus and TypeScript in general are all very new to me.

Kruptein commented 4 years ago

The way the tools.vue file now communicates to tool.vue is using a vue event-bus (the .emit and .on stuff).

It feels kinda hacky, so I'm not 100% happy with it and thus might look at refactoring it in the future. That said, it's fine to just continue with the existing approach.

ZachMyers3 commented 4 years ago

I'll keep going with the way I'm going then. If you determine a more "clean" way to go about this and provide an example I wouldn't be against trying that out as well, though it might be served best with a different issue/PR.

ZachMyers3 commented 4 years ago

I was able to fix the bug for creating shapes, it was using the circle tool as the starting point reference for both events, so I adjusted touchEvents to use the touch start location.

I ended up using vue2-hammer to pull the pinch event inside of game.vue and was able scale off of this event. Here's the snippet

    pinch(e: any): void {
        if (!e.target || !(<HTMLElement>e.target).tagName || (<HTMLElement>e.target).tagName !== "CANVAS") return;
        const delta = Math.sign(e.deltaY) * -1;
        gameStore.updateZoom({ newZoomDisplay: gameStore.zoomDisplay - 0.1 * delta, zoomLocation: l2g(getLocalPointFromEvent(e.srcEvent)) });
    }

You can see the changes in my latest commit.

ZachMyers3 commented 4 years ago

I think if all of this is acceptable I've got a few more commits to clean up the codebase and I'm ready to submit a PR

Kruptein commented 4 years ago

Going to close this as base interaction should be possible since PR #213 has been merged.

More exotic things like pinch and other multi gesture combos are for a later pr.