adi-g15 / Ludo-The_Game

An implementation of the famous board game Ludo, using C++
MIT License
26 stars 16 forks source link

[ReactJS] Design a Ludo board #14

Closed adi-g15 closed 4 years ago

adi-g15 commented 4 years ago

This is #13 , but you can approach it with ReactJS, which i will likely use in the web version, and you can be the prime contributor for that πŸ˜„

Design a responsive webpage for a Ludo Board, that will be utilized for this project's web version.

You can start with this ->

===============================================================================
                                LUDO - THE GAME
===============================================================================
                                                           |
                                                           |
                                                           |
                                                           |    --scoreboard--
                                                           |
                    --Board inside--                       |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
-------------------------------------------------------------------------------

                                --log area--                                                                                   
tusharpandey13 commented 4 years ago

Hi I would like to take up this issue

adi-g15 commented 4 years ago

Yes, you can have a go on that πŸ˜ƒ

echo-sg commented 4 years ago

Can i contribute towards the issue?

adi-g15 commented 4 years ago

Yes, you can take up the issue πŸ‘πŸ½

Seems like @tusharpandey13 isn't active for some time.

Athi223 commented 4 years ago

I would like to contribute to this, could you possibly add an API endpoint at least for the Dice-rolling, move inputs, and its response part so it could help to check if the frontend works accordingly?

adi-g15 commented 4 years ago

I would like to contribute to this, could you possibly add an API endpoint at least for the Dice-rolling, move inputs, and its response part so it could help to check if the frontend works accordingly?

Okay, i will add api endpoints for those two, for now, hope that will be done by tomorrow evening. For now, you can follow these ->

  1. Dice Rolling -> Endpoint - https://adig15.herokuapp.com/ludo/roll [GET Method]

    request -
    response { "roll": [array of ints] }
  2. Move -> https://adig15.herokuapp.com/ludo/moveGoti [GET]

    • request -

      {
      "goti": {
          "colour": "_gotiColour_",
          "dir": "_currentDirection_",
          "coords": [_x_, _y_]
      },
      "dist": "distance_to_move_int_int" 
      }
    • response -

      {
      "bool": "boolean(signifying_move_valid_or_not)",
      "move": {
      "coord": "_final_coord_",
      "dir": "_final-direction_",
      "profit": "integer"
      }
      }

The move endpoint follows this -> https://github.com/AdityaGupta150/Ludo-The_Game/blob/4b38838fbd7e9526053502803f2f9e15bb83bb53/includes/game.hpp#L69

You can expect this request and response schemas @Athi223

But, since @echo-sg had also been assigned the issue, it's upon you if you want to work on it, in any case, since you have asked for it, the APIs will be added in either case.

Also, you can use javascript's random functions, or if you prefer some library, for the dice part, it's just an array of random numbers between 1 to 6.

adi-g15 commented 4 years ago

@Athi223 The apis have been setup now for those routes, if you like you can use them for testing now.

Athi223 commented 4 years ago

@AdityaGupta150 so here's the basic board until now, partially referred from https://codepen.io/thewayur/pen/WNNXJMO

ludo

Haven't committed the changes yet, hope it looks fine so i can commit and progress further maybe...

Athi223 commented 4 years ago

Also i had a few doubts:

  1. Is the move path https://adig15.herokuapp.com/ludo/move/goti instead of the one that you specified?
  2. Maybe the request type for 'move' could be changed to POST instead of GET since it has a complex object inside it?
  3. Are any of these request bodies for 'move' request correct: i. { "goti": { "colour": 0, "dir": 0, "coords": [6,9] }, "dist": 3 }

    ii. { "goti": { "colour": "LAAL", "dir": "EAST", "coords": [6,9] }, "dist": 3 }

because i tried these to based on whatever i could figure out from the code, but they gave response: 304 and no response data. Not sure if i gave wrong body or if it is because of the request type. I used this to test by the way => https://www.apirequest.io/

adi-g15 commented 4 years ago

@AdityaGupta150 so here's the basic board until now, partially referred from https://codepen.io/thewayur/pen/WNNXJMO

Haven't committed the changes yet, hope it looks fine so i can commit and progress further maybe...

That's quite good to say the least @Athi223 πŸ‘πŸ‘. You can also leave a block on right for the dice πŸ‘

And, if you want you can create many commits, if they are too much, then it can be simply be squash merged.

adi-g15 commented 4 years ago
  1. Is the move path https://adig15.herokuapp.com/ludo/move/goti instead of the one that you specified?

Yes, so as to include more moves like for the robot in /move later, in case it's needed.

  1. Maybe the request type for 'move' could be changed to POST instead of GET since it has a complex object inside it?

Yes, will be POST then, I let it be get since I typed that in a comment.

  1. Are any of these request bodies for 'move' request correct:

colour: 0 is not valid, nor is dir: 0 Coords should be in movable path on the board, dist: 0 will be accepted, but will return bool: false, that is move not possible.

colour can be 'R', 'G' , Y, B
dir can be U, D, L, R. (For up down left right respectively)

The status code for malformed requests will be 400 though, I saw it was 304 in code earlier. Though /move/goti route has been fixed, though if there seems to be a bug, do tell πŸ‘πŸ½

Athi223 commented 4 years ago

@AdityaGupta150 ludo

Here's the update till now

A small request, could you give me an api or a function to get current direction based on current position; cus i'm finding it a bit difficult to figure out current direction for move request: { "goti": { "colour": "_gotiColour_", "dir": "_currentDirection_", "coords": [_x_, _y_] }, "dist": "distance_to_move_int_int" }

adi-g15 commented 4 years ago

@Athi223 You should store the current direction of movement for the goti, which initially will be the start_dir of that colour, then can be updated from the direction received from move.

Though, I will create the api endpoint at adig15.herokuapp.com/ludo/util/getdir

The request will be like { 'coords': [x,y] }

Also, related C++ code is in the file -

https://www.github.com/AdityaGupta150/Ludo-The_Game/tree/master/src%2Fthinker.cpp#L54

Athi223 commented 4 years ago

@AdityaGupta150 i guess i can probably figure out the starting direction in the getDir function from https://www.github.com/AdityaGupta150/Ludo-The_Game/tree/master/src%2Fthinker.cpp#L54 as you suggested, so probably wont need the API

But there seems one issue with the /move/goti API, it is only working for "col": "R" and returning input not valid for `"col": "G" or "B" or "Y" so i cant check if the whole thing is working properly

adi-g15 commented 4 years ago

Also, for starting directions and coords, you can use these values ->

https://github.com/AdityaGupta150/Ludo-The_Game/blob/ade257c08eecba43ee42a64235cc72bdc5798001/src/ludo_coords.cpp#L24-L36

@Athi223 The problem was at https://github.com/AdityaGupta150/Ludo-The_Game/blob/ade257c08eecba43ee42a64235cc72bdc5798001/web/api/routes/move.js#L58

It will be fixed in some time, with detecting direction from coords added too, in case "dir" is passed in request, it will use that, else will use the direction according to the coords passed.

And also i think the "dir" is not required in request, as you said about getting direction of movement from coord, that can well be done server side, but for now, we will keep it as you prefer.

Athi223 commented 4 years ago

Also, for starting directions and coords, you can use these values ->

https://github.com/AdityaGupta150/Ludo-The_Game/blob/ade257c08eecba43ee42a64235cc72bdc5798001/src/ludo_coords.cpp#L24-L36

@Athi223 The problem was at

https://github.com/AdityaGupta150/Ludo-The_Game/blob/ade257c08eecba43ee42a64235cc72bdc5798001/web/api/routes/move.js#L58

It will be fixed in some time, with detecting direction from coords added too, in case "dir" is passed in request, it will use that, else will use the direction according to the coords passed.

And also i think the "dir" is not required in request, as you said about getting direction of movement from coord, that can well be done server side, but for now, we will keep it as you prefer.

yeah even i felt direction is not really required as all we gonna do is place the pieces according to our x,y positions, i just thought maybe the backend logic needs it and maybe to reduce some computation you're taking it from frontend. would be certainly better if we could just input colour and coords

adi-g15 commented 4 years ago

@Athi223 It's fixed, and from now on,

this request will also be valid, if any problem occurs (should not, but can't deny), inform then ⚑

{
    "goti": {
        "col": "G",
        "coords": [6, 3]
    },
    "dist": 12
}

And, do note that dist values, given to api, greater than 6 are also allowed, since in original C++ code, for example, if die output is 6,6,2 then the user can simply type 14 instead of typing 3 times, though this is likely not in web version, but just a point to note about the api.

Athi223 commented 4 years ago

@AdityaGupta150 Okay so finally the move/goti/ path is working almost fine, with probably one issue left. Whenever the goti enters the end zone(the last 5 blocks before the goti reaches its destination), in that region the API returns Bad Request, when the direction isn't specified like this (this is for the Green color by the way): end1

Whereas it works fine when the direction is specified, like this(Green color as well, tested using API test site): end2

So is it possible to fix this small issue? Because if not, then we'll probably have to add the direction parameter for every request, since adding it for only the last blocks would probably increase the complexity; also that extra parameter will increase the payload size unnecessarily.

The game is almost done by the way, only the goti killing logic is left. And also the safe place logic is remaining, which i was planning to add later when the basic game works perfectly.

Athi223 commented 4 years ago

@AdityaGupta150 Update: The move/goti path gives a false when it reaches the destination, like this(same Green color as above): win

Just wanted to confirm whether that was intended or if it is wrong. I guess that's intended since an invalid move is denoted by Bad request so i guess its completely fine to check piece reaching by a false.

Just confirm this so i'll add the logic accordingly.

adi-g15 commented 4 years ago

@Athi223

The move/goti path gives a false when it reaches the destination

The "bool" actually contains true or false, signifying whether the move is possible, and the move should be executed only if "bool" is true. So, since it correctly reaches end, "bool" should be true, that should not have been the problem, it was earlier so, due to a pre-decrement of dist, which was corrected, i have redeployed it, shouldn't be a problem. (it will only be false, when say you are 5 units from winning, and you pass 6 to the API, it will return bool: false, since the move isn't possible, so you can simply depend for this check on the api, just pass it whatever move value you get, and move if bool: true)

So is it possible to fix this small issue? Because if not, then we'll probably have to add the direction parameter for every request, since adding it for only the last blocks would probably increase the complexity; also that extra parameter will increase the payload size unnecessarily.

Added that, should be working now πŸ˜„ πŸŽ‰

The game is almost done by the way, only the goti killing logic is left. And also the safe place logic is remaining

Great friend, you really have put time into it, considering all trying the APIs too πŸŽ‰ πŸŽ‰

Also, for the kill logic, you may go like these ->

  1. If the board is a 2D array of boxes, then each box can be a class containing info on what gotis are present, either by i. an array of references to the gotis (if implemented so), ii. or in form of simple string like 'RRB' showing it has 2 laal gotis and 1 neela goti
  2. A map of colours mapped to an array of coords, though this might be less efficient than above, but possible

These finally can be used to see whether a goti of other colour (for the web version, just consider every other colour as opponent, though there is also the 'friend colours' implemented in the CPP code)

output The api is working now, as it should have earlier, for both inputs you gave πŸ‘πŸ½

adi-g15 commented 4 years ago

And for the, profit: null, it can simply be implemented, by adding another variable, but that is for ROBOT modes, it is beyond this issue, and likely not needed, but will be later, thats why it's in the api.

Athi223 commented 4 years ago

@Athi223

The move/goti path gives a false when it reaches the destination

The "bool" actually contains true or false, signifying whether the move is possible, and the move should be executed only if "bool" is true. So, since it correctly reaches end, "bool" should be true, that should not have been the problem, it was earlier so, due to a pre-decrement of dist, which was corrected, i have redeployed it, shouldn't be a problem. (it will only be false, when say you are 5 units from winning, and you pass 6 to the API, it will return bool: false, since the move isn't possible, so you can simply depend for this check on the api, just pass it whatever move value you get, and move if bool: true)

So is it possible to fix this small issue? Because if not, then we'll probably have to add the direction parameter for every request, since adding it for only the last blocks would probably increase the complexity; also that extra parameter will increase the payload size unnecessarily.

Added that, should be working now smile tada

The game is almost done by the way, only the goti killing logic is left. And also the safe place logic is remaining

Great friend, you really have put time into it, considering all trying the APIs too tada tada

Also, for the kill logic, you may go like these ->

1. If the board is a 2D array of boxes, then each box can be a class containing info on what gotis are present, either by
   i. an array of references to the gotis (if implemented so),
   ii. or in form of simple string like 'RRB' showing it has 2 laal gotis and 1 neela goti

2. A map of colours mapped to an array of coords, though this might be less efficient than above, but possible

These finally can be used to see whether a goti of other colour (for the web version, just consider every other colour as opponent, though there is also the 'friend colours' implemented in the CPP code)

output The api is working now, as it should have earlier, for both inputs you gave πŸ‘πŸ½

Looks like there's a small bug, at the coord and distances where it shouldve returned false, it seems that the operation times out; could you check it please?

for eg. [7,1] & dist: 6 as you mentioned

adi-g15 commented 4 years ago

Again, πŸ€” , it was a console.log instead of res.send, that caused the timeout, since the res.send never happened and server didn't respond back. Finally this too sorted out 😁

output2 Now, it's more dependendable. And, also it won't send 'profit' of move, that was for robot mode, and will require the information of the path too, no need of it for now.

Athi223 commented 4 years ago

@AdityaGupta150 One (hopefully) last request, could you make a small change in move/goti path, such that we can send multiple(preferably an array) of coords, for the same col & dist ? something like "coords": [ [7,1], [7,0], [7,3] ]; which preferably return array of bools for each case, and an array for coords containing valid next coords if the move is possible and null if it isnt. I needed this, since when (and incase) all of the remaining gotis are in the final path (last 6 blocks near the end), I wanted to check if any of those have a valid move available for the current value of the die; if yes,then player gets a choice to select the piece, else the chance moves on to the next player.

For eg.: I have 3 Green gotis at [7,1], [7,3] & [7,4] and i get a 6 on the die, in this case i dont have any possible moves so the next player shoudl get the chance.

I wanted the multiple coords API, since a single call takes about 350ms (from India atleast) and hence a maximum of 4 calls would take more than a second, so it was preferable if you could tweak the API a bit to take multiple coords.

Also the dir is not really required in the move/goti API

adi-g15 commented 4 years ago

Yes, that will be good considering the delay of the API calls. Will make it so, may require some more if statements to first tell whether it's a single coord, or an array of coords. Will let you know in this issue.

@Athi223 You may have to change some code, considering the changes in API, specially removing direction from client server interaction

Athi223 commented 4 years ago

@AdityaGupta150 I'd suggest it'd be easier if you just do it for an array, in that way even if the input is single coord, frontend can just send it as an array with one element. You could just traverse over the coords array by, lets say array.forEach() and put the relevant results(the bool and output coords i suppose) into 2 respective arrays and then send the result. That'll reduce your efforts of handling the type(single/multiple) and some computation time & resources as well.

adi-g15 commented 4 years ago

@AdityaGupta150 I'd suggest it'd be easier if you just do it for an array, in that way even if the input is single coord, frontend can just send it as an array with one element. You could just traverse over the coords array by, lets say array.forEach() and put the relevant results(the bool and output coords i suppose) into 2 respective arrays and then send the result. That'll reduce your efforts of handling the type(single/multiple) and some computation time & resources as well.

Yes, i have done a similar thing, with a single change- It accepts both single coord, or an array of coords, at server side, if it's a single coord, it will simply convert it to an array, then use .forEach to loop through the coords.

https://github.com/AdityaGupta150/Ludo-The_Game/blob/1ada7d3fca667d7c38e70eb74ae2eee29b3e40d7/web/api/routes/move.js#L105-L114

Direction have been completely removed from the interaction between client and server- Client requests is accepted as-

{
    "col": "R",
    "coords": [[13,6],[8,9]],
    "dist": "12"
}

{
    "col": "R",
    "coords": [8,9],
    "dist": "7"
}

{
    "col": "R",
    "coords": [[8,9]],
    "dist": "7"
}

And responds with :

{
  "bool": array_of_bools,
  "move": array_of_final_coords
}

For eg.

{
  "bool":[true,true],
  "move":[[6,0],[9,7]]}
adi-g15 commented 4 years ago

Lines you might like to change a bit -> https://github.com/Athi223/Ludo-The_Game/blob/3677803f2d69d89e1e18348565214f4d32be8b4e/web/ludo/src/App.js#L55-L61

https://github.com/Athi223/Ludo-The_Game/blob/3677803f2d69d89e1e18348565214f4d32be8b4e/web/ludo/src/App.js#L67 Maybe updated to -

          const updated_place = response.coords[0][0] + '-' + response.coords[0][1]

for a single coord request

As for this,

https://github.com/Athi223/Ludo-The_Game/blob/3677803f2d69d89e1e18348565214f4d32be8b4e/web/ludo/src/App.js#L58 This previous code, passing a single coord only will work with the api too, there should not be any changes to that.

As far as i saw your code, only these were the API usage, that needs to be updated

Also, it may be better to store the API url in an environment variable in .env , like API=https://adig15.herokuapp.com/ludo

It will be good if the url updates, probably into a cloud function or a lambda function (subroutes won't change, like /roll /move/goti)

Athi223 commented 4 years ago

@AdityaGupta150 Thanks a lot for those crucial changes and also for reducing the request size by removing those unnecessary elements. Is something wrong with the server right now? I'm getting Service Unavailable since 10-15min from now (I really hope I didn't mess up anything because i was testing with the old format just while you made the new changes live). Though i could check the new format successfully a couple of times after I saw your comments here, so probably my old-format requests won't be an issue.

I updated the changes as you suggested, can't test those since the server is having issues but i guess they're fine according to your documentation.

Just need one more change confirmation: https://github.com/Athi223/Ludo-The_Game/blob/3677803f2d69d89e1e18348565214f4d32be8b4e/web/ludo/src/App.js#L65 This line should change to if(response.bool[0]) { i suppose since i need to check if the bool of 0th coords (actually the only coords that have been sent), is it right? My only doubt was, since you're allowing both, single coord(1D array precisely) and multiple coords(2D array), so in both cases the result is still array of bools and array of move right?

Also if you want you can remove that extra check of whether we have a single coord or array, since all it needs is passing an array even incase of single coord like [ [6,1] ] instead of [6,1] if i'm not wrong. Ofcourse you could change it or keep it as it is according to your convinience, just let me know if you do make this change so in that case I'll have to make the subsequent change too.

adi-g15 commented 4 years ago

I'm getting Service Unavailable since 10-15min from now (I really hope I didn't mess up anything because i was testing with the old format just while you made the new changes live). Though i could check the new format successfully a couple of times after I saw your comments here, so probably my old-format requests won't be an issue.

It seems to be a problem with the server, i don't remember how much, but it won't be up for all 24 hours, and will simply sleep if the threshhold is reached. Trying to redeploy, if that makes a difference.

https://github.com/Athi223/Ludo-The_Game/blob/3677803f2d69d89e1e18348565214f4d32be8b4e/web/ludo/src/App.js#L65

This line should change to if(response.bool[0]) { i suppose since i need to check if the bool of 0th coords (actually the only coords that have been sent), is it right? My only doubt was, since you're allowing both, single coord(1D array precisely) and multiple coords(2D array), so in both cases the result is still array of bools and array of move right?

Yes, the result will be arrays, since whatever request you send, the server will actually change it to an array, as you pointer out in a comment, and then using .forEach. And response.bool can be changed to response.bools[0].

Also if you want you can remove that extra check of whether we have a single coord or array, since all it needs is passing an array even incase of single coord like [ [6,1] ] instead of [6,1] if i'm not wrong. Ofcourse you could change it or keep it as it is according to your convinience, just let me know if you do make this change so in that case I'll have to make the subsequent change too.

I think, i should leave it for now, by the way, the check is just this, this if should be enough to differentiate between an array of 2 ints (ie. a pair), or an array of pairs.

if (reqData.coords.length === 2 && all(reqData.coords, iter => typeof (iter) === 'number')) { // ie. is a single pair reqData.coords = [reqData.coords]; // convert to an array }

You can use it as if it takes arrays, that won't be an issue πŸ˜ƒ

adi-g15 commented 4 years ago

UPDATE - @Athi223 The apis are working again.

Some sample request json -

{
    "col": "R",
    "coords": [[13,6],[8,9]],
    "dist": "12"
}

{
    "col": "R",
    "coords": [8,9],
    "dist": "7"
}

{
    "col": "R",
    "coords": [[8,9]],
    "dist": "7"
}
Athi223 commented 4 years ago

I'm getting Service Unavailable since 10-15min from now (I really hope I didn't mess up anything because i was testing with the old format just while you made the new changes live). Though i could check the new format successfully a couple of times after I saw your comments here, so probably my old-format requests won't be an issue.

It seems to be a problem with the server, i don't remember how much, but it won't be up for all 24 hours, and will simply sleep if the threshhold is reached. Trying to redeploy, if that makes a difference.

Oh yes I remember, its free usage policy require it to be asleep for fixed amount of time i suppose, not an issue. I guess i should try to host it locally and change the API routes temporarily for testing.

For the .env part, seems like I need to refer the docs a bit to implement that, sorry for that.

In the meantime I also figured out a way to show multiple gotis on a single block, without they simply getting overlapped and appearing as one goti. This probably makes it possible to enable the "safe" zones on board, although i won't implement those unless we have the main game ready.

adi-g15 commented 4 years ago

The API is currently up, and seems will be so.

https://www.npmjs.com/package/dotenv

You maybe using this to work with the .env

It's just as simple as, create a file named .env

Put values in it in the form -

key1=value1
key2=value2

And in index.js, (can be in other files too, but then config() function needs the relative path too) Put the line require('dotenv').config()

You could read that itself though

Athi223 commented 4 years ago

The API is currently up, and seems will be so.

https://www.npmjs.com/package/dotenv

You maybe using this to work with the .env

It's just as simple as, create a file named .env

Put values in it in the form -

key1=value1
key2=value2

And in index.js, (can be in other files too, but then config() function needs the relative path too) Put the line require('dotenv').config()

You could read that itself though

Oh okay thanks. I read somewhere that react already uses dotenv so i directly tried to add the variable.

But by the way, i guess this will work as long as we use webpack(development) server or seperate serve the react app using some node-based server, but probably it wont work if we make a production build and add it to some other server i guess. Might work with express tho if we add the the .env variables to the express server folder.

adi-g15 commented 4 years ago

But by the way, i guess this will work as long as we use webpack(development) server or seperate serve the react app using some node-based server, but probably it wont work if we make a production build and add it to some other server i guess. Might work with express tho if we add the the .env variables to the express server folder.

In production build, we can specify the content of .env as environment variables on the hosting site, eg. netlify, github pages etc. So, it behaves the same way as if .env was available. Though, since the API url is not sensitive information, we may define it as a simple const variable in index.js itself, if the production server thing gives any problem ?

Athi223 commented 4 years ago

@AdityaGupta150 I have pushed the first working version on web-react branch.

As of now, the basic game logic is working fine.

Ps. 1 change i did in contrast to the c++ game was that, whenever 2+ gotis came at the same block, you had printed the count next to the goti color, which i understand is the obvious way to display it in terminal. So instead of the count on Goti component, i reduced the goti size to half when there are more than 1 gotis at same block. Do check it while testing and let me know if it looks fine that way.

Things that are not added as of now:

  1. "Safe" places logic(altho the safe places are marked on board without any business logic)
  2. No prompt for asking the number of players (I'll probably add this by night if time permits). Just wanted to commit and push a basic working version so didn't add it right now. This means that we compulsorily need to 4 players(technically) as of now.
  3. No movement animations for gotis. Couldnt manage this as of now since we are demounting and mounting the Goti component so for html/css they are completely different <figure> elements which probably can't be linked in any way. Maybe we could think of some transition animation from starting to end location, although that's just my thought and i haven't searched anything regarding it as of now.
  4. At the moment, we dont have any over-the-net multiplayer mode since in that case we need to shift the whole logic on server side and also implement websockets for that. So right now we only have a web-interface where we can play with 4 people on the same device.

I request you to clone the web-react branch somewhere and test it as much as possible.

Lastly, it was great working with you and also it helped me a lot to practice React and got to learn quite a few JS concepts as well. Ofcourse, having said that, I'm not stopping the progress here. Will try to add few of the features mentioned above in 1-2 days.

adi-g15 commented 4 years ago

I request you to clone the web-react branch somewhere and test it as much as possible.

Lastly, it was great working with you and also it helped me a lot to practice React and got to learn quite a few JS concepts as well. Ofcourse, having said that, I'm not stopping the progress here. Will try to add few of the features mentioned above in 1-2 days.

Will let you know by night about how it goesπŸ˜„ . Thanks to you also for putting time in it πŸŽ‰ As for online multiplayer, that surely is a target, will give a good go to implement it later, it's beyond this issue tbh.

adi-g15 commented 4 years ago

@Athi223 Just to let you know i am still trying it, almost completeting my first complete gameplay through it. Also your version is deployed on https://ludo1.netlify.app .

I am trying to fix some things myself, specially the responsiveness part, since on a 14 inch screen (my laptop), can't say about bigger screen, the dice is below the table, and i had to scroll to it.

Also, i would also ask you to add comments, so as for easier understanding for someone looking at the code.

Some more things i am also trying to fix, Another chance to roll die after kill, as for the safe place, not much a problem, can be fixed later, though it feels weird to get your gotis killed at starting box itself πŸ˜„

adi-g15 commented 4 years ago

Some glitches occured also, but i didn't recall what move i did, so i will be logging the moves in console from next time.

For example, i couldn't get why this happened, it's stuck for me to chose a blue goti, but i can't since none is unlocked now for blue, nor i can roll the die since it's expecting me to move. Screenshot (92)

Tried to see if it's due to a problem of server (maybe), but the server was still up and responding πŸ€”

Athi223 commented 4 years ago

I am trying to fix some things myself, specially the responsiveness part, since on a 14 inch screen (my laptop), can't say about bigger screen, the dice is below the table, and i had to scroll to it.

Yeah i guess it was responsive to some extent but not totally. Also as far as possible I didn't wanted to add extra burden of bootstrap or any framework for a single page app so didn't quite work on it tbh.

Also, i would also ask you to add comments, so as for easier understanding for someone looking at the code.

Yes I'll do that in the upcoming updates.

Some more things i am also trying to fix, Another chance to roll die after kill, as for the safe place, not much a problem, can be fixed later, though it feels weird to get your gotis killed at starting box itself πŸ˜„

Actually the issue was not with the safe place logic, the issue was rendering the different color gotis in the same place. According to my current logic I'm sending particular color as a prop to the Goti component and the it is rendered by looping length times, for one or more gotis of same color. So inorder to render different color gotis i need to make some (probably big) change in the logic, so I guess i need to think a bit on it before implementing so as to not break the whole thing.

adi-g15 commented 4 years ago

Also, i would also ask you to add comments, so as for easier understanding for someone looking at the code.

Yes I'll do that in the upcoming updates.

Yes comments are actually very helpful, i had a few more glitches, were i wasn't able to proceed maybe due to network or something, as any code, it may also need refactoring and fixes later on, it will be of great help.

As for responsiveness, great for a beginning anyways, since the point of the issue is done I think, but yes comments will greatly help.

Athi223 commented 4 years ago

Yes comments are actually very helpful, i had a few more glitches, were i wasn't able to proceed maybe due to network or something, as any code, it may also need refactoring and fixes later on, it will be of great help.

While testing i'd suggest you to keep developer tools open and on Network tab. That helped me a lot to check whether there's any issue on the client side logic or there exists some connectivity issue.

Athi223 commented 4 years ago

@AdityaGupta150 I've added the Safe places logic to the game. Merge it with my master. Try it when possible. Also I had mistakenly created a PR to master branch but I've closed it for now. I'll hopefully test the game a few times and add some exception handling logic for the fetch calls too, and then will send a PR.

Athi223 commented 4 years ago

@AdityaGupta150 Hey I've added comments to the App and Goti components, hope that'll help in understanding the code. Added it to my web-react branch and also merged it to my master. Have a look when you get time

adi-g15 commented 4 years ago

@echo-sg The issue is being closed, if you want to contribute to it or anything you may comment in this issue itself.

@Athi223 has already done a lot and has completed more than what this issue was originally for πŸ˜ƒ