realm-of-ra / mancala

https://meowing-anteater-cee.notion.site/Mancala-Game-MVP-7521e2f2e5294575b33b17601afde810
MIT License
9 stars 24 forks source link

feat: Implement Timer for Moves #63

Closed TAdev0 closed 1 week ago

TAdev0 commented 3 weeks ago

Closes #55

This is a first draft

vercel[bot] commented 3 weeks ago

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

Name Status Preview Comments Updated (UTC)
mancala ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 20, 2024 9:43pm
mancala-pkco ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 20, 2024 9:43pm
vercel[bot] commented 3 weeks ago

@TAdev0 is attempting to deploy a commit to the DAO Box Team on Vercel.

A member of the Team first needs to authorize it.

TAdev0 commented 3 weeks ago

@web3technologies it seems just formatting with the extension modified cairo files formatting

May i have additionnal informations? I havent done any test for now, i just need to understand what is the expected behaviour exactly What do you mean by ending the game? Simply declaring a winner?

TAdev0 commented 3 weeks ago

ok all Cairo files needed to be formatted actually

TAdev0 commented 3 weeks ago

Just improved imports (use core::starknet for example, for compatibility with >= 2.6.3 with edition 2023-11)

for now what i did is

     if player == self.player_one && self.last_move > block_info.block_timestamp + 100 {}
     if player == self.player_two && self.last_move > block_info.block_timestamp + 100 {}

Now i need to know what should i do exactly if the time has passed? how to end the game? simply declaring a winner? Also, the n limit to make a move should be hardcoded? or defined when creating a new game?

web3technologies commented 3 weeks ago

@web3technologies it seems just formatting with the extension modified cairo files formatting

May i have additionnal informations? I havent done any test for now, i just need to understand what is the expected behaviour exactly What do you mean by ending the game? Simply declaring a winner?

Ok yes I am thinking about this more and i think we need a new GameStatus::Timeout

After n number of blocks for now we can use 100 but later we will define this, then the game can be considered Timeout and then set the winner to != mancala_game.current_player and mancala_game.status = GameStatus::Timeout

I am interested to see your approach. I was thinking that the opponent player can call a function from the frontend that will initiate this Timeout. Then some validation to see if actually the current_block is > last_move_block + 100

If you want to discuss this some more you an reach me at: https://t.me/zachcook

TAdev0 commented 3 weeks ago

okay i'll do like you said, it seems fine.

Btw, in actions.cairo , for move

     assert!(mancala_game.status != GameStatus::Finished, "Game is already finished");
     assert!(mancala_game.status == GameStatus::InProgress, "Game is not in progress");

its redundant, i remove the first assert

TAdev0 commented 3 weeks ago

@web3technologies so i did it a bit differently : no new function to call and set the timeout enum but when the next player calls too late, the move is not executed, and the enum is set to timeout

i made a few changes in check to make it work

but this means the game is not finished with a winner. this would also be the case with an additional function

web3technologies commented 3 weeks ago

@web3technologies so i did it a bit differently : no new function to call and set the timeout enum but when the next player calls too late, the move is not executed, and the enum is set to timeout

i made a few changes in check to make it work

but this means the game is not finished with a winner. this would also be the case with an additional function

Ok so what happens if the player who calls too late never calls again? What if they leave the game and then never calls the move again? We need to allow the opposing player to say "my opponent hasnt made a move in 100 blocks, the game should be Timeout and I should win the game"

Does this make sense?

TAdev0 commented 3 weeks ago

Yeah sure this is a question I asked myself !

xpanvictor commented 3 weeks ago

@web3technologies has this information been synced with FE. Do I make an issue on this? I mean the timer call on the FE and has this functionality been completed on the contract?

web3technologies commented 3 weeks ago

@web3technologies has this information been synced with FE. Do I make an issue on this? I mean the timer call on the FE and has this functionality been completed on the contract?

You can make an issue however this is not critical for our two week timeline. I believe we will also need to add this in the design. We need a button on the frontend that appears to allow the user to call saying that the game should be timed out. @okhaimie-dev should we coordinate this with design first?

okhaimie-dev commented 3 weeks ago

@web3technologies has this information been synced with FE. Do I make an issue on this? I mean the timer call on the FE and has this functionality been completed on the contract?

You can make an issue however this is not critical for our two week timeline. I believe we will also need to add this in the design. We need a button on the frontend that appears to allow the user to call saying that the game should be timed out. @okhaimie-dev should we coordinate this with design first?

At Supreme's current pace on the frontend, I think it is okay to create an issue for the timer ui. I will speak with the design team today.

TAdev0 commented 3 weeks ago

hey @web3technologies sorry for the delay, took a few days off.

Just created a new public function, time_out , in addition to the previous mechanism. SO that both player can trigger the TimeOut statut.

TAdev0 commented 3 weeks ago

will add tests once we agree on the impl. I also added a new time_between_move variable in Mancala game storage, hardcoded to 100 in new function. We can probably change it in the future, passing the value as argument to the new function to allow personalisation

TAdev0 commented 2 weeks ago

Yep sure was waiting for doing it , will do that today!

TAdev0 commented 2 weeks ago

hey @web3technologies again sorry for the late. Just added 3 unit tests. Tel me if this is enough

I left 2 comments as well to make sur its ok

TAdev0 commented 1 week ago

@web3technologies just improved the test, checked that the game is indeed TimeOut and the winner is the other player. Just check my comment regarding whether we should keep the if statement that checks if the status is in progress (for the 2nd time, but it seems needed)

okhaimie-dev commented 1 week ago

@all-contributors please add @TAdev0 for code and tests

allcontributors[bot] commented 1 week ago

@okhaimie-dev

I've put up a pull request to add @TAdev0! :tada:

TAdev0 commented 1 week ago

@web3technologies just did the refactoring created a dedicated validate_time_out function