ise-ethereum / on-chain-chess

Play decentralised chess on the Ethereum blockchain!
MIT License
214 stars 39 forks source link

Refactor frontend for off-chain communication. Many bugs fixed #206

Closed graup closed 8 years ago

graup commented 8 years ago
kyroy commented 8 years ago
WARNING in ./app/javascript/gamesFactory.js
jshint results in errors
  Line is too long. @ line 475 char 103
        console.log("Sent move no", game.state[8] * 128 + game.state[9], 'from', fromIndex, 'to', toIndex);

  Strings must use singlequote. @ line 475 char 31
        console.log("Sent move no", game.state[8] * 128 + game.state[9], 'from', fromIndex, 'to', toIndex);

  Line is too long. @ line 844 char 101
        if ((game.chess.turn() === 'w' && game.self.color === 'white' && data.args.timeoutState !== 0) ||

  Line is too long. @ line 845 char 101
            (game.chess.turn() === 'b' && game.self.color === 'black' && data.args.timeoutState !== 0)) {

  'generateState' is defined but never used. @ line 3 char 9
    import {generateState, generateFen} from './utils/fen-conversion.js';

WARNING in ./app/javascript/playGame.js
jshint results in errors
  Line is too long. @ line 173 char 112
            gameStates.addSelfMove(game.gameId, algebraicToIndex(move.from), algebraicToIndex(move.to), game.state);
graup commented 8 years ago

There are some new bugs, but I wanted to get this PR up now because I won't have time to look at it further until tomorrow.

Sometimes I get an error that the move counts don't match after reloading. Also the notification for new moves in other games doesn't work 100% yet.

kyroy commented 8 years ago

Were these bugs also existing before he refactoring?

graup commented 8 years ago

Before refactoring lots of other things didn't work, so it wasn't really possible to see them. I would say these are new bugs. The basic move, claims / react to claim events and reading from local storage still works.

kyroy commented 8 years ago

games.update needs to be changed (see games.add) => done

kyroy commented 8 years ago
let lastMoveTime;
try {
  // Try to find out time of last move
  lastMoveTime = gameStates.getLastMoveTime(game);
  if (!lastMoveTime) {
    // get it from blockchain?
  }
} catch (e) {
}

The last move time is not stored in the contract. However, we could search the transaction where the last move was made / when the game was joined by the second player

kyroy commented 8 years ago

When pressing surrender or close, the message "waiting..." is present, but does not get updated. When reloading the page, the action is already completed.

kyroy commented 8 years ago

At some point, we should also remove the move listeners ;)

dennis-westphal commented 8 years ago

When starting a new game, I get this:

screen shot 2016-07-12 at 15 38 09

Move no. NaN is not really ideal ;-)

dennis-westphal commented 8 years ago

Offering a draw does not work for both sides:

screen shot 2016-07-12 at 15 41 18

dennis-westphal commented 8 years ago

When a game is won, the claimEther button is displayed. This should not be the cause; the frontend should automatically claim the ether (worked before).

kyroy commented 8 years ago

the frontend should automatically claim the ether (worked before).

Ye, this worked for onchain... not for offchain ;)

dennis-westphal commented 8 years ago

Now there seems to be a different problem:

screen shot 2016-07-12 at 19 17 26
dennis-westphal commented 8 years ago

Also, the Move no NaN issue still exists.

kyroy commented 8 years ago

Ye, i am working on some problems ;)
Atm i don't care about the move number... don't know why this was added ;)

graup commented 8 years ago

I encountered cases were the move couldn't be saved because the move number wasn't correct. If that's fixed we can remove the counter again :)

kyroy commented 8 years ago

The move that was send, was send with the old state, not with the new state. This may have caused the error ;)

graup commented 8 years ago

Proxy communication seems to work (better than before).

The last big bug is that moveFromState goes out-of-gas. Still haven't found a good way to debug that :(

lisasi commented 8 years ago

Oh, i always read "another" as "other". Weird. Of course you're right @dennis-westphal!

graup commented 8 years ago

Merging to clean up Issues. Going to create a new issue for the remaining bug(s).