quick fix for mobile crash #88

[pancelor]

• open a game with mouse controls in desktop firefox (e.g. export this minimal example and open the exported HTML file: https://auroriax.github.io/PuzzleScript/editor.html?hack=ff896cbe88c966435f5d7da328eaf7af)
• open the devtools, set the browser to pretend to be a mobile device
• reload the page, the mobile undo/restart/quit pullout menu should appear
• pull out the pullout menu and to play the game, toggling the pullout menu a few times and interacting with the game while the pullout menu is open. after a few clicks the game will freeze up the browser

root cause: setMouseCoord lets coords.x be NaN (faked keypress that was generated on mobile) which spreads contagiously, turning x1 into NaN, making getTilesTraversingPoints enter an infinite loop

fixing this should probably involve making the pullout menu "consume" input events and not pass them down into the game

fixes Auroriax#88

[Auroriax]

Sadly this still seems to get stuck in an infinite loop for me, following your repro steps. It seems to get stuck in one of the for loops of the IsInside function: the iterator becomes NaN which results in an infinite loop. (Apparently? JS can be so weird at times.)

Likely you want a similar safeguard against NaN coordinates in this function as well. (I think that for the IsInside function, if either coordinate is NaN then it should return as the cursor is likely not inside the canvas).

Screenshot of the debugger when pausing Firefox:

@pancelor Could you take a look at this? Otherwise, feel free to bounce it back to me and then I'll take a closer look.

[pancelor]

sure! I'll check it out sometime this weekend

nice catch, and ty for the screenshot

[pancelor]

wait, I bet this is because I only pushed the source code and didn't update standalone_inlined.txt; if I run a local server on this PR and export, the exported HTML doesn't have my changes, e.g. at the start of getTilesTraversingPoints (I bet that's what happened to you here)

can you run compile.js and try again? I should do it myself but it's a bit of a hassle

[Auroriax]

Hmm, I compiled the engine, but apparently I didn't include the fix for some reason! I've recompiled it and your fix now works as described. Apologies for the mistake!

Now that I've had a quick look, there's one issue: you can't press the Undo button repeatedly. E.g. do a couple of turns, press undo, but only the first turn will be undone. If you tap outside of the canvas and then undo it will work, which might indicate some pointer event weirdness. I'm not sure how to fix this, I've tried firing prevent() in this case but that doesn't seem to do the trick.

[pancelor]

Ah, that must be happening because tapping the "undo" option both undoes one turn and also creates a LMB object, which moves the game forward one turn. (could be prevented with nosave; e.g. in this updated example).

This is basically the same underlying issue again (touch events are not consumed by the pullout menu) so I think this quick fix should be merged as-is, and leave that problem for later on down the line.

(on a separate note: this is a lot of finicky code for mouse interaction! I wonder if there's some nicer-to-use way to add mouse controls to puzzlescript... maybe this: clicking with the mouse creates a LMB object, then dragging makes that existing object move, rather than creating a LMB_drag object. that would be simpler for my needs but maybe it prevent certain other kinds of games from working that I haven't considered)

[Auroriax]

So once you do a stack trace the underlying issue seems to pop up: after you press a button on the rollout menu, the game will trigger a fake mouse press to put the focus back on the canvas element. In doing so, it triggers the setMouseCoord function that sets the values to NaN. But if you make the function return during this step, you can't repeatedly press the undo button. And I'm not sure why that is?

For now I'll just use your fix though, since without the return statement you added, it already prevents the weird softlock.

[pancelor]

oh, that's very odd -- you tried it out on my second example? (and rebuilt the inlined html?)

I used the firefox devtools on desktop to fake mobile input, and iirc and my first example made it impossible to undo more than once, but my second example fixed that (for me). very strange

I wonder if something wild was happening where, e.g., the old code set the coords to NaN, which crashed some code later before it could fire a second game turn; now that the NaNs are gone, the undo press successfully fires an (unwanted) game event. (I haven't looked into it at all, just a theory)