Premoves create widespread bugs

[Naviary2]

Several of the bugs after releasing this latest update have come from premoves. When premoves are queued, and some other part of the application reads the list of pieces, or modify them, if we don't explicitly unapply and reapply premoves, it just creates bugs when pieces aren't in expected locations.

I wonder if there's some way we can automatically prevent all parts of the application from reading from the pieces, or rewinding/making moves, if the premove application isn't what's expected...

Open to ideas.

[shirsakm]

I wonder if there's some way we can automatically prevent all parts of the application from reading from the pieces, or rewinding/making moves, if the premove application isn't what's expected...

I don't think that's possible, but it might be feasible to create a proxy layer that will handle the checking for us and then return the real state of the piece. It would then take refactoring to update all the places that asks for the piece with the new function. Though it doesn't have to be done all at the same time, that's a saving grace.

Another option is to update the premove lifecycle to emit events when they are applied / rewound. This is technically the simplest way to turn off all aspects of the app from reading from the pieces, etc. But, this will also take the same amount of refactoring since this doesn't actually force the reading of the current state, just that you can react to the event, and do something about it. This is good for invalidating stale data in the cache, but I don't think it's good for much else. All that without the advantage of allowing new usage to have a clean way of working with premoves. Moreover, this feels like, and in fact is a hack solution.

[Naviary2]

I want to cross my fingers and hope there's no other locations we've missed where we should be unapplying and reapplying premoves. Anything else sounds far complicated.

[galoomba1]

Why do you need to "unapply" premoves in the first place? Aren't premoves just a queue of moves that get made when it's your turn and they're legal?

[Naviary2]

Why do you need to "unapply" premoves in the first place? Aren't premoves just a queue of moves that get made when it's your turn and they're legal?

Yes, but having queued premoves changes where you see pieces on the board, as they make board and mesh changes. So the game has to for example, before making the board & mesh changes from your opponents move, rewind all the changes of the premoves, apply the opponents move, then reapply the changes of the premoves, so that all changes are in the correct chronological order. Otherwise there would be missing pieces where they are expected.

[galoomba1]

Weird to modify the actual board that game logic uses, sounds like there should be a separate visual-only board in that case

[Naviary2]

Even still, this visual-only board would experience the same bugs, until they are patched. If the logic is faulty, the crashes would still occur.

[trichoplax]

I haven't looked into the code, but I'm wondering if rewinding (applying the premoves in reverse, in reverse order) is necessary. If both the current board state and the premove list are stored, would the following alternative approach work when the opponent makes a move:

• Update the current board state to reflect the opponent's move.
• Update the current board state to reflect the first move on the premove list.
• Discard the first move from the premove list.
• Apply the remaining moves on the premove list to the current board state to give the display board state.

There's no reason that the rewinding and reapplying approach couldn't be fixed to not have bugs, but it seems like twice the work, and twice the opportunities for introducing bugs. Is storing a separate current board state and display board state (the same idea as @galoomba1 suggests) too expensive in memory?

I understand that fitting in memory doesn't mean it would be a small amount of work to change the implementation. I'm just curious if it's an option in principle.

[Naviary2]

You just described its current behavior, except before it updates the current board state to reflect the opponent's move, it first updates the board state, applying the premoves list in reverse.

There's no reason that the rewinding and reapplying approach couldn't be fixed to not have bugs

I agree.

but it seems like twice the work, and twice the opportunities for introducing bugs. Is storing a separate current board state and display board state (the same idea as @galoomba1 suggests) too expensive in memory?

Well that's the thing, not only is keeping track of two separate board states complex and unnecessary, the one used for premove visuals would STILL have the same bugs the one we're using now has, because it would still have to follow the same steps to unapply premoves, apply the opponents move, then reapply premoves again...

[trichoplax]

The difference I was trying to describe is omitting the unapply step. When the opponent moves, throw away the display board state and construct a new one using only forward moves applied to the current board state.

I can imagine this being more expensive in terms of writes to memory (since it overwrites the entire display board state with the current board state). I guess if a game has huge numbers of pieces this could be a big problem.

[Naviary2]

Arguably more complex since in order to construct a new board state it has to reapply all moves since the start of the game, which is often more than how many premoves you have queued.