downforacross / downforacross.com

Web frontend for downforacross.com -- continuation of stevenhao/crosswordsio
https://downforacrosscom.downforacross1.now.sh
MIT License
220 stars 92 forks source link

[fix] Autocheck intermittently doesn't work #308

Open justindhill opened 5 months ago

justindhill commented 5 months ago

Summary

Fixes an issue where autocheck intermittently doesn't work on a cell due to out of order processing of updateCell and check events.

Description

Currently, autocheck is implemented by the frontend adding two game events in close succession, an updateCell event and a check event whose scope is constrained to the cell coordinates of the updated cell. This makes sense conceptually, since socket.io guarantees ordering of messages regardless of the transport it's using. Problems can arise due to minute variations in how long it takes to process an event. Occasionally, the check event finishes processing before the updateCell event. Clients then receive the events out of order, causing the check to be performed on an unpopulated cell.

Solution

This problem really only arises because of how close the events are fired to one another. Other events don't follow this pattern, so are far less likely to experience this problem. Additionally, for other events, ordering may not be as important. Therefore, instead of trying to solve the general case of processing finishing in-order, we can solve this particular problem by adding an autocheck parameter to the updateCell event and not firing a check event at all. Processing of check events remains the same, so explicit checks of cells, words, and puzzles are unaffected.

Before / After

Untitled 2 Untitled

vercel[bot] commented 5 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
downforacross.com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 31, 2024 0:41am
flackr commented 3 weeks ago

The code looks good and this seems to work pretty well for me, any chance we can get this merged to get auto-check working flawlessly?

flackr commented 5 days ago

@justindhill can you merge master into the branch so I can play test this with all of the other changes that have landed since?