Make the UNO button functioning #166

Abhishek-Punhani · 2024-06-25T12:59:01Z

Making the uno button functioning
Set uno button to trigger Announce_Uno Event and added a handler for this event" -m
Fixes : #159

Checklist

I have tested these changes locally.
I have reviewed the code and ensured it follows the project's coding guidelines.
I have updated the documentation, if necessary.
I have assigned reviewers to this pull request.

Screenshots (if applicable)

[If your changes include any visual updates, provide screenshots here.]

vercel · 2024-06-25T12:59:04Z

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

Name	Status	Preview	Comments	Updated (UTC)
multiplayer-uno	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 26, 2024 6:34am

kuv2707

The second commit is alright, left suggestions for the first.

Also I notice that we aren't updating vulnerableToUno on throw_card, as we are in the backend.

See throwCard.ts
You can add a commit adding this, or make a separate PR.

kuv2707 · 2024-06-25T13:19:18Z

frontend/src/pages/Game.tsx

+        if (gameState?.players) {
            modal.show(<GamePropertiesModal />, 'large', [], false);
        }
        // eslint-disable-next-line
-    }, [gameState]);
+    }, [gameState?.players.length, gameState.id]);


It would be better to make the dependency array empty. So that the modal pops up only once at the start of the game.
In a different PR, we are introducing a button to show the modal in the middle of the game, and are also adding more options to the modal.

So this modal will be automatically shown once, when the user joins. The if statement is therefore not required too

But we need to show the number of players joined so it will update only if the modal is updated following a change in length of players array and also for the one who joins the game game id also changes(from null) so to show the game id we need to reload the modal .
We can incorporate a creator or admin in game and instead of showing Start Game , can show Username's Room Created ! , and the players that join will see Join Username's Room?

Good points.

We do want the modal content to change based on some changes to game state. Also, we only need to change the content if the modal is in display in the first place. I think we'd want to update the content on JOIN_GAME (since player count increases) and START_GAME (we'd want to close the modal once the admin starts the game).

The change of gameId from null to a value won't be visible to the modal, since i have added a commit which mounts the Game component (of which the modal is a part) only after we have a non-null game id. So we dont need to worry about re-rendering the modal then.

A room is created the moment a user clicks on Create Game. We can show a toast saying "Room Created!"

Once the joining players have opened a room, it wouldn't make much sense asking them if they want to join. If they didn't, they wouldn't have entered the game code / clicked on the url.

I think we should improve the modal system to be able to efficiently handle dynamic content.
The current implementation is such that if the user has opened some other modal (like rules etc) and someone joins the game, the GamePropertiesModal will pop up abruptly. That is just bad UX

Here's something we can do -

Remove the dependencies from this useEffect, so that it only appears once when the game page loads (with valid game data)

Make the following changes to the modal context -

Add a field id to uniquely identify each modal. We pass the id when calling modal.show

Add a function to only change the content of the modal if the modal of given id is rendering.
So sth like modal.setContent("abc",<p>hii</p> will only set the content to hii if a modal with id abc is already visible on the screen.

Then here, in this modal.show call, we pass in the id for this modal (say "gameProperties")

Add another useEffect which calls modal.setContent("gameProperties",<...>) and include in its dependency array gameState.players etc

What are your thoughts on this?

Yeah I agree that joining player will see different content in this way

kuv2707 · 2024-06-25T17:55:30Z

frontend/src/pages/Game.tsx

            modal.show(<GamePropertiesModal />, 'large', [], false);
        }
        // eslint-disable-next-line
-    }, [gameState]);
+    }, []); // todo :Fix the dependency array
+    const announceGame = () => {


This can better be called as announceUNO

kuv2707

Left a minor comment. Also please amend the commit message to match the format

There's also a merge conflict to be resolved

Set uno button to trigger Announce_Uno Event Also added a handler for this event Fixes : shivansh-bhatnagar18#159

kuv2707

LGTM

vercel bot deployed to Preview June 25, 2024 12:59 View deployment

kuv2707 reviewed Jun 25, 2024

View reviewed changes

Abhishek-Punhani force-pushed the FixIssue#159 branch from 75992ee to b8dd895 Compare June 25, 2024 16:52

vercel bot deployed to Preview June 25, 2024 16:52 View deployment

kuv2707 reviewed Jun 25, 2024

View reviewed changes

Abhishek-Punhani force-pushed the FixIssue#159 branch from b8dd895 to 074dacc Compare June 25, 2024 17:57

vercel bot deployed to Preview June 25, 2024 17:57 View deployment

Abhishek-Punhani force-pushed the FixIssue#159 branch from 074dacc to 8a7e8e6 Compare June 26, 2024 06:29

vercel bot had a problem deploying to Preview June 26, 2024 06:29 Failure

Abhishek-Punhani force-pushed the FixIssue#159 branch from 8a7e8e6 to 9ae54fb Compare June 26, 2024 06:31

vercel bot deployed to Preview June 26, 2024 06:32 View deployment

Frontend :Made Uno button functional

c49895b

Set uno button to trigger Announce_Uno Event Also added a handler for this event Fixes : shivansh-bhatnagar18#159

Abhishek-Punhani force-pushed the FixIssue#159 branch from 9ae54fb to c49895b Compare June 26, 2024 06:33

vercel bot deployed to Preview June 26, 2024 06:34 View deployment

kuv2707 approved these changes Jun 26, 2024

View reviewed changes

kuv2707 merged commit 72ddf1c into shivansh-bhatnagar18:master Jun 26, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the UNO button functioning #166

Make the UNO button functioning #166

Abhishek-Punhani commented Jun 25, 2024

vercel bot commented Jun 25, 2024 •

edited

Loading

kuv2707 left a comment

kuv2707 Jun 25, 2024

Abhishek-Punhani Jun 25, 2024 •

edited

Loading

kuv2707 Jun 25, 2024

kuv2707 Jun 25, 2024

kuv2707 Jun 25, 2024

kuv2707 Jun 25, 2024

Abhishek-Punhani Jun 25, 2024 •

edited

Loading

kuv2707 Jun 25, 2024

kuv2707 left a comment •

edited

Loading

kuv2707 left a comment

Make the UNO button functioning #166

Make the UNO button functioning #166

Conversation

Abhishek-Punhani commented Jun 25, 2024

Checklist

Screenshots (if applicable)

vercel bot commented Jun 25, 2024 • edited Loading

kuv2707 left a comment

Choose a reason for hiding this comment

kuv2707 Jun 25, 2024

Choose a reason for hiding this comment

Abhishek-Punhani Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

kuv2707 Jun 25, 2024

Choose a reason for hiding this comment

kuv2707 Jun 25, 2024

Choose a reason for hiding this comment

kuv2707 Jun 25, 2024

Choose a reason for hiding this comment

kuv2707 Jun 25, 2024

Choose a reason for hiding this comment

Abhishek-Punhani Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

kuv2707 Jun 25, 2024

Choose a reason for hiding this comment

kuv2707 left a comment • edited Loading

Choose a reason for hiding this comment

kuv2707 left a comment

Choose a reason for hiding this comment

vercel bot commented Jun 25, 2024 •

edited

Loading

Abhishek-Punhani Jun 25, 2024 •

edited

Loading

Abhishek-Punhani Jun 25, 2024 •

edited

Loading

kuv2707 left a comment •

edited

Loading