pong command by mikesmith-23 · Pull Request #92 · USF-OS/FogOS · GitHub
Skip to content

pong command#92

Open
mikesmith-23 wants to merge 10 commits into
USF-OS:mainfrom
mikesmith-23:main
Open

pong command#92
mikesmith-23 wants to merge 10 commits into
USF-OS:mainfrom
mikesmith-23:main

Conversation

@mikesmith-23

@mikesmith-23 mikesmith-23 commented Sep 30, 2024

Copy link
Copy Markdown

Project 1: Pong Command Integration for FogOS

For Project 1, I developed a terminal-based version of Pong for our FogOS by implementing custom system calls to handle real-time input and terminal display. In the game, the player controls a paddle using the up and down arrow keys, while the ball bounces between the paddle and the wall. Although the game does run, there is a significant issue with how input is tracked. The non-blocking input isn't working as intended, which means the arrow keys don’t effectively move the paddle during gameplay. After the game ends, any untracked input gets processed all at once, causing the terminal to flood with menu text.

Menu:

pong_menu

All things considered, I want to advise anyone who might use this command—to be prepared for some odd behavior due to the input-blocking issue. Despite these issues, I’m proud of what I accomplished and the amount I learned about system calls and OS integration. In retrospect, I bit off more than I could chew for this first assignment, but it was a great learning experience. Moving forward, future improvements will focus on resolving the input issues and adding the missing features.

Game Play:

pong_gameplay.mov

@mikesmith-23 mikesmith-23 changed the title setup repo for pull pong command Oct 1, 2024
@AbelA72

AbelA72 commented Oct 1, 2024

Copy link
Copy Markdown

@alach2

alach2 commented Oct 2, 2024

Copy link
Copy Markdown

I can code review this!

@mvvillarrealblozis

Copy link
Copy Markdown

I can code review this

@mvvillarrealblozis

Copy link
Copy Markdown

I think this is a really cool and fun addition. Like you mentioned in the PR, I did experience some strange behavior in the terminal while playing the game. I am a huge fan of how thorough your documentation is, I just wish it had followed the Java-doc style documentation a little closer. There is some big brain code in here which is really cool to see.

High Level Checks:

  • PR fully resolves the task/issue
  • Does the PR have tests?
  • Does the PR follow project code formatting standards?
  • Does the OS still compile and run?
  • Is documentation included?

Code Checks:

  • Does the code achieve its intended purpose?
  • Were all edge cases considered?
  • Is the code organized and maintainable?
  • Does the code maintain the same level of performance? (no performance regressions)

@alach2

alach2 commented Oct 7, 2024

Copy link
Copy Markdown

High-Level Checks:
[] PR fully resolves the task/issue.
[✔️] Does the PR have tests?
[✔️] Does the PR follow project code formatting standards?
[✔️] Does the OS still compile and run?
[✔️] Is documentation included?
Code Checks:
[] Does the code achieve its intended purpose?
[] Were all edge cases considered?
[✔️] Is the code organized and maintainable?
[✔️] Does the code maintain the same level of performance? (No performance regressions)

This code looks really good and professional, but I also experienced the input issues you referred to in your PR. When I play the game, I do not see the paddle move, and after a few seconds the game crashes and prints a few Invalid Choice statements. This is really strong code that is well formatted, it just needs to properly account for the keyboard choices. I think it would be helpful to add some more documentation to your code too, especially in pong.c. The comments are very helpful and straight forward, but I get lost with a few of the variables and parameters and what they represent. Overall, I think you did an amazing job and added some really impressive code to your system, successfully printed the game console, accounted for errors, and formatted your code really nicely.

@AbelA72

AbelA72 commented Oct 16, 2024

Copy link
Copy Markdown

High-Level Checks:
[] PR fully resolves the task/issue.
[✔️] Does the PR have tests?
[✔️] Does the PR follow project code formatting standards?
[✔️] Does the OS still compile and run?
[✔️] Is documentation included?
Code Checks:
[ X ] Does the code achieve its intended purpose?
[✔️] Were all edge cases considered?
[✔️] Is the code organized and maintainable?
[✔️] Does the code maintain the same level of performance? (No performance regressions)

This seemed like a very hard project but you did it pretty well, the quality is great and I can tell you spent a lotta time on this. I did notice the bug you mentioned and it kinda made my terminal go crazy a few times and I haven't really been able to play it. I would've loved to play this cause I was looking forward to it, maybe another day.

Pong.c

  • I saw that you had a switch for the game menu and when I entered a random number it kinda spammed the game menu at me with the invalid choice after each one, I think that's probably the same issue the game has but just in case it's not I wanted to let you know.
  • The code looks pretty organized and documented well, it is also pretty sophisticated, so props to you for getting it done.

I don't really have any other changes, overall this was a great shot at this and I think you did great for the time you had.

@malensek

Copy link
Copy Markdown
Contributor

This is a cool idea for a project. There are a lot of very advanced things happening here, but I also am curious about some of the decisions you made:

  • Why did you create a generic syscall function (complete with support for variadic arguments) and then use that to call your system calls?
  • What is delay used for if you are busy waiting instead?
  • Why are many of the terminal escape sequences implemented as system calls instead of being done in user space?
  • There seem to be references to the x86 version of xv6 in here, so maybe you were using some of the resources for that while developing this project?
  • Seems like some work was done on potentially support VGA instead but that didn't end up happening?

It seems like the template program was mostly supplied by ChatGPT, right? It has all the usual signs including comments for every line in the original file you checked in (then the comments were removed / reworded later).

In my mind, being ambitious is a great thing for these projects, but this is a great example of a situation that warrants writing a quick test program to make sure it's actually possible to do and then adjusting expectations accordingly. There are also a LOT of changes to the xv6 codebase that I'm not sure are necessary (changing types for many user space function inputs, adding includes from the host machine that we're not using in xv6) so this is difficult to merge back into the main branch without breaking the rest of the programs.

@snarayan57982

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants