hugosaintemarie / magic-maze

Online version of board game "Magic Maze"
https://magicmaze.herokuapp.com
MIT License
20 stars 11 forks source link

Sometimes clock is not inverted #16

Open timendum opened 3 years ago

timendum commented 3 years ago

Hi, first of all thanks for the app, it's fun to play.

I have one mayor issue: somtimes the item time (hourglass) is used (it get crossed) but the time is not inverted.

There are no visual clue of what is happening.

Can you advice?

hugosaintemarie commented 3 years ago

Hello @timendum and thank you for your feedback! I'm glad you're having fun even though this project is not really finished 🙂

I'm not really sure regarding the clock issue, I'd need to look into it. I will definitely reply to this issue when I sort it out!

crob1140 commented 3 years ago

Firstly, I want to say that I'm also a big fan of this project and I've been having a lot of fun with it as well! I have a theory around how this particular issue can happen but unfortunately I don't have a good way of reproducing it...so I'll just dump my thoughts here. I think it might be the result of a race condition caused by this line.

Here's how I see it (potentially) playing out:

Player 1: 1) There is a mouse-up event to set hero A's destination to cell X,Y, which contains an unused timer 2) The 'hero' event is emitted notify other players that hero A is moving to cell X,Y 3) After a bit of movement, the display loop catches that hero A is very close to cell X,Y, so it calls hero.move(true) 4) Within that method, it will call events.checkForEvents which will see that there is an unused timer in the cell X,Y 5) Player 1's local clock is inverted 6) The 'invertClock' event is emitted to notify other players that their clock should be inverted 4) Cell X,Y is marked as used on player 1's board 5) The 'used' event is emitted to notify other players that cell X,Y is now used

Player 2: 1) Receives the 'hero' event from player 1 notifying that hero A is moving to cell X,Y 2) After a bit of movement, the display loop catches that hero A is very close to cell X,Y, so it calls hero.move(true) 3) Within that method, it will call events.checkForEvents which will see that there is an unused timer in the cell X,Y 4) Player 2's local clock is inverted 5) The 'invertClock' event is emitted to notify other players that their clock should be inverted 6) Cell X,Y is marked as used on player 2's board 7) The 'used' event is emitted to notify other players that cell X,Y is now used 8) Player 2 now receives the invertClock event that was emitted by player 1 when they saw hero A entering cell X,Y 9) Player 2's local clock is inverted again, putting it back to the original value

That last step would happen to player 1 as well with the 'invertClock' event emitted by player 2, but I left that out to make it easier to follow the process.

This will only happen if a player happens to process the move(true) call before receiving the 'used' event from the other player, which is why you don't see it all the time. I think a potential fix could be to only check the cell for events if that heroes action was originally triggered by that player; if that was the case, then steps 3-7 of player 2's sequence would not exist. I'm more than happy to play around with a fix and raise a PR if you also think this might be the cause....not sure how to go about testing it though.

hugosaintemarie commented 3 years ago

Thank you so much for your warm feedback and detailed explanation @crob1140!

I think you're very much right with your diagnosis. A PR would be very much welcome if you find the time to make one, I must admit I'm focusing on other projects right now but would still love to wrap this up someday!

Here's another idea I didn't really take time yet to reflect on: what if hero.move() wasn't responsible for the animation, but use CSS transitions instead? I could update a hero's X/Y (top/left SVG-wise) coordinates and let a CSS transition smooth it out? What do you think? Could it fix the issue or should I still store who's been triggering the hero action?

Also: maybe only the host should handle the events.checkForEvents method and dispatch events to other players?

rcjsuen commented 3 years ago

I just hit this problem also after stopping on a timer spot. It didn't look like the clock inverted itself.