Skip to content

Conversation

@jasonz6688
Copy link
Contributor

@jasonz6688 jasonz6688 commented Jun 18, 2021

List of Changes:

  • Added slider to the lobby page to set the number of problems for a game
  • Added numProblems value to the room, lobby, and updateSettingsRequest
  • Frontend now accepts and stores multiple problems on the PlayerGameView.tsx page, PlayerGameView.tsx and Editor.tsx also now accept a 2D array for code and an array for languages, PlayerGameView.tsx page stores the languages and code as two arrays, whereas the Editor stores the code and languages as is.
  • PlayerGameView.tsx stores currentProblemIndex, which determines which problem is being worked on, the code and language which are displayed and modified in the Editor depend on this value as well as the displayed description and console results/testcases.
  • currentProblemIndex is added as a field to submission request to tell the backend for which problem the submission is made.
  • currentProblemIndex can be incrememted or decremented by the small buttons added to PlayerGameView.tsx, it will wrap around from 0 and problems.length (a small number needs to be added somewhere to display the currentProblemIndex).
  • Code are submissions are now stored in a array of submissions in PlayerGameView.tsx, which is used to reload code after refreshes.
  • Removed code and language field in PlayerDTO and frontend player object
  • Score is now calculated as the number of problems which you solve completely correctly.
  • Game ends only after everyone solves every problem correctly.

Notes:

Alan has patched all the bugs that we know of, but there may be more bugs, specifically I think that there might be weird edge cases with reloading the page and loading saved code. Also you can technically set the numProblems slider below the number of selected problems, but the site will set the error and not progress when you try to start the game; this works but differs from how we do errors for other setting requirements. Also, I think that we should reconsider the choose problem modal to show up before the user selects the number of problem, as the user will first select a problem, close the modal, select a number, and re-open the modal, and choose problems.

Additionally, a UI PR will probably be needed to make the buttons more in place (especially because the time limit modal appears over the next and previous buttons) and to fix the font issue (if Alan hasn't already fixed that). Additionally, additional PRs can deal with viewing the winning code. Another UI change that we may want to add later is some text that displays which problem is being worked on (i.e. 1/2, 2/4, ...).

Screencast and Images:

Selecting multiple problems:
Shot1

Shot8

Shot9

Shot10

Selecting to many problems:
Shot11

Selecting problems then putting the slider down and pressing start game:
Shot12

Selecting one problem:
Shot2

Starting the game:
Shot3

Submitting correctly the first of three problems:
SHot4

Moving to the second problem:
Shot5

Submitting a partially correct (some test cases are wrong) the second problem:
Shot6

After submitting third problem correctly:
Shot7

@jasonz6688 jasonz6688 marked this pull request as draft June 18, 2021 19:54
@zpChris zpChris changed the title Staging Num Problems, Staging Num Problems Jun 21, 2021
@jasonz6688 jasonz6688 marked this pull request as ready for review June 23, 2021 03:15
@alankbi alankbi requested review from alankbi and zpChris June 23, 2021 04:36
Copy link
Contributor

@alankbi alankbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, glad this is finally getting merged! Since I also worked on this branch a bit, I'll hold off on approving or requesting changes for now, in the meantime just left a few comments/questions

@alankbi
Copy link
Contributor

alankbi commented Jun 24, 2021

Also, I think that we should reconsider the choose problem modal to show up before the user selects the number of problem, as the user will first select a problem, close the modal, select a number, and re-open the modal, and choose problems.

If I understand correctly, yeah this should be updated in #174 - basically numProblems will only be relevant if the user doesn't select a specific problem (if they do, then numProblems will automatically be incremented to match the count)

Additionally, a UI PR will probably be needed to make the buttons more in place (especially because the time limit modal appears over the next and previous buttons) and to fix the font issue (if Alan hasn't already fixed that). Additionally, additional PRs can deal with viewing the winning code. Another UI change that we may want to add later is some text that displays which problem is being worked on (i.e. 1/2, 2/4, ...).

+1 on the UI changes (should be handled in the mentioned PR or if not, the one after that). The font issue I pushed a change to hopefully address it, but since I'm not using Windows maybe @jasonz6688 you can run that branch locally and verify?

The one thing I didn't address is the scrollbar design on Windows, which to me looks a bit offputting and am wondering if we should customize that at all. However, idk if that's a standard thing to see on Windows, so maybe it isn't a huge deal - thoughts @jasonz6688 @zpChris?

@zpChris
Copy link
Contributor

zpChris commented Jun 25, 2021

+1 on the UI changes (should be handled in the mentioned PR or if not, the one after that). The font issue I pushed a change to hopefully address it, but since I'm not using Windows maybe @jasonz6688 you can run that branch locally and verify?

The one thing I didn't address is the scrollbar design on Windows, which to me looks a bit offputting and am wondering if we should customize that at all. However, idk if that's a standard thing to see on Windows, so maybe it isn't a huge deal - thoughts @jasonz6688 @zpChris?

My thoughts could change on this in the future, but in terms of our current goals I don't think a scrollbar Windows design fix is an important enough issue to put on our current docket to address.

@zpChris
Copy link
Contributor

zpChris commented Jun 25, 2021

I found a small bug relating to multiple problems output for which I've made a screencast; when the problem is ran or submitted, and then before completing the load the problem is switched, the output and/or results will pop up under the wrong problem.

After going back and forth the problem results will correct themselves, but they are initially off and that's likely a bug we should address, imo.

Copy link
Contributor

@zpChris zpChris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ✅

I left a good number of comments, but since @alankbi is working on #174 which will update this PR's work, I'm ok if some of those fixes or updates fall through to the next PR.

So excited to merge this in soon (long time coming) - let's gooo @jasonz6688!!! 🎉 🥳

@zpChris
Copy link
Contributor

zpChris commented Jul 4, 2021

@jasonz6688 Is this PR ready to merge?

@jasonz6688 jasonz6688 merged commit aeef174 into main Jul 6, 2021
@zpChris zpChris deleted the stagingNumProblems branch July 6, 2021 00:59
@jasonz6688
Copy link
Contributor Author

1 similar comment
@jasonz6688
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants