ghost-cursor icon indicating copy to clipboard operation
ghost-cursor copied to clipboard

feat: ability to associate timestamps with movements

Open aw1875 opened this issue 1 year ago • 3 comments

Take two on a PR I did a few years ago (https://github.com/Xetera/ghost-cursor/pull/42).

This version does essentially the same thing as the original PR but with a more elegant approach by utilizing the options that can be passed to the path function and calculating the time to take using the Trapezoidal rule.

I'm open to any suggestions/changes so please let me know.

aw1875 avatar Mar 31 '24 02:03 aw1875

Thanks for the PR!

If I understand correctly, this is only showing the timestamps, not actually using it, right?

Maybe we should change the puppeteer mouse.move() event to use CDP Input.dispatchMouseEvent() directly, which supports a timestamp: https://chromedevtools.github.io/devtools-protocol/tot/Input/#method-dispatchMouseEvent

Niek avatar Apr 03 '24 08:04 Niek

That's correct it's just for show. However, I wouldn't be against digging more into the Input.dispatchMouseEvent docs to use the generated timestamps in puppeteer if you think it would make sense for me to do.

aw1875 avatar Apr 03 '24 18:04 aw1875

@Niek just checking back in so I don't lose track of this pr. Did you want me to make any changes?

aw1875 avatar Apr 11 '24 02:04 aw1875

In my opinion this only makes sense if the timestamps are used, meaning we need to switch to manual Input.dispatchMouseEvent() calls.

Niek avatar May 08 '24 18:05 Niek

In my opinion this only makes sense if the timestamps are used, meaning we need to switch to manual Input.dispatchMouseEvent() calls.

Honestly, I don't really see any scenario where this would work unless I'm also making changes to functions like move, moveTo, etc. However, something like this could technically take that approach (this is kinda hacky but you get the idea):

   const tracePath = async (
-    vectors: Iterable<Vector>,
+    vectors: Iterable<Vector | TimedVector>,
     abortOnMove: boolean = false
   ): Promise<void> => {
...
-        await page.mouse.move(v.x, v.y)
+        if ('timestamp' in v) {
+          await getCDPClient(page).send('Input.dispatchMouseEvent', {
+            type: 'mouseMoved',
+            x: v.x,
+            y: v.y,
+            timestamp: v.timestamp
+          })
+        } else {
+          await page.mouse.move(v.x, v.y)
+        }
...

Or, we could always use the dispatchMouseEvent with something more along the lines of:

   const tracePath = async (
-    vectors: Iterable<Vector>,
+    vectors: Iterable<Vector | TimedVector>,
     abortOnMove: boolean = false
   ): Promise<void> => {
...
-        await page.mouse.move(v.x, v.y)
+        const dispatchParams: Protocol.Input.DispatchMouseEventRequest = {
+          type: 'mouseMoved',
+          x: v.x,
+          y: v.y,
+        }
+
+        if ('timestamp' in v) dispatchParams.timestamp = v.timestamp
+
+        await getCDPClient(page).send('Input.dispatchMouseEvent', dispatchParams)
...

aw1875 avatar May 11 '24 07:05 aw1875

Yes, this is what I was thinking of. Although it's probably better to have the CDP client initialised once in order to avoid creating thousands of clients.

Niek avatar May 11 '24 08:05 Niek

Good call, my brain didn't even process that I was calling that in a loop when I sent that code block

aw1875 avatar May 13 '24 01:05 aw1875

Awesome, thanks! This LGTM, but @bvandercar-vt can you review this as well?

Niek avatar May 13 '24 08:05 Niek

Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.

aw1875 avatar May 15 '24 04:05 aw1875

Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.

Did you run yarn lint? Should fix the formatting

bvandercar-vt avatar May 15 '24 14:05 bvandercar-vt

Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.

Did you run yarn lint? Should fix the formatting

Yeah that was post yarn lint (plus husky should be running the linter automatically anyway)

aw1875 avatar May 15 '24 14:05 aw1875

Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.

Did you run yarn lint? Should fix the formatting

Yeah that was post yarn lint (plus husky should be running the linter automatically anyway)

Ah yeah yarn lint doesn't fix this whitespace issue on my end either:

image

Definitely a downside of ts-standard then. We could add prettier as a dep if the repo owners are interested.

Update: Not sure if the repo owners will go for it, but I wanted to see how it looked-- made a prettier PR: https://github.com/Xetera/ghost-cursor/pull/142

bvandercar-vt avatar May 15 '24 15:05 bvandercar-vt

Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.

Did you run yarn lint? Should fix the formatting

Yeah that was post yarn lint (plus husky should be running the linter automatically anyway)

Ah yeah yarn lint doesn't fix this whitespace issue on my end either:

image

Definitely a downside of ts-standard then. We could add prettier as a dep if the repo owners are interested.

Update: Not sure if the repo owners will go for it, but I wanted to see how it looked-- made a prettier PR: #142

super annoying, still a little what caused the whitespace to begin with as I turned off any formatters I have when working in this repo but not sure what else I can do besides manually fix it

aw1875 avatar May 16 '24 01:05 aw1875

Should be good now, really odd but oh well

aw1875 avatar May 16 '24 02:05 aw1875

@aw1875 PR looks good! Thanks for implementing those changes!

bvandercar-vt avatar May 16 '24 16:05 bvandercar-vt

Thanks a lot, merged now!

Niek avatar May 16 '24 16:05 Niek