-
Notifications
You must be signed in to change notification settings - Fork 49
Guy Graphics Library #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This reverts commit c9a8023.
|
I will code review this. |
|
Guy Graphics Library guy.c: formatting issues formatting issue with guy_move_head(), guy_run(), guy_celebrate(), user.h: no issues More comments on other methods and Javadoc documentation of what each method does. It would also be helpful to include more information on how to run certain parts of your project. It took a little bit of time to understand how to run it. High Level Checks:
Code Checks:
|
|
I can code review this |
|
High Level Checks:
Code Checks:
As a whole this is a fun and well implemented addition to XV6. It is well organized to add onto it as well as being easily used by other functions to add their own animations of guy. I also found no edge cases or bugs with the code with my testing. However, documentation could be improved with javadocs with every function to better explain their purposes and how they fit with one another. Additionally, formatting can be improved such as with the one line printf in guy_info() that can be broken up. But overall this is a solid project that can be added and added upon to in any os of XV6. |
|
I can code review this! |
|
High Level Checks:
Code Checks:
I love this project! It's very fun to see guy pop up when qemu is booted- I think in terms of high level performance, it's great but when I mistype a command, I think the error message should include some valid inputs to type in so that the user can follow that- so for example: "mistyped - try 'guy run', 'guy ___'," etc.... instead of just "mistyped try again". Code quality looks good! Maybe instead of a clear screen function since that wipes away everything, maybe create a separate function that just clears away the animation instead of the instructions to run the program. Your documentation is easy to follow, the instructions are great. Awesome job! |
|
This is definitely one of the more unique additions to user space! The other CRs already mentioned it, but this needs a formatting sweep to get in xv6 formatting and fix some of the spacing issues (tabs/spaces mixed). Is there any reason the system call is needed? I think that can be implemented by calling the regular I know this is intended to be used as a library or user space program, but the guy / guy2 naming is a bit confusing. Maybe guy and guy_lib? I like Esha's idea to print the guy and then move the cursor back so the screen doesn't need to be cleared (selectively clearing it instead) but I imagine that would be a lot of work and isn't a required update. you can get +0.5 back for fixing the formatting and issues outlined above. |
Description:
This pull request contains a guy graphics library that contains animations for the user's enjoyment. As they open and run commands in FogOS, the animations will run. This library gives the user directions on how to run the animations. Future developers can include any of the functions within the interface in any file. To see some examples, look in and run ls.c and rm.c. There is also a message function that is optional, but calling it almost takes more work than writing your own message so I just left it in.
This PR also implements another syscall function called sleep_ms, that allows programs to sleep for shorter durations than the current sleep functions in FogOS. To see more documentation, look in docs, guy.md.
Changed files:
Makefile - now includes guy.c
guy.c - main function that calls on the guy library
guy2.c - the actual guy library, provides an interface so functions can be accessed within user.h
user.h - now contains guy functions
docs/guy.md - for documentation
sysproc.c, syscall.c, syscall.h - all changes to implement ms_sleep
Limitations:
Never actually use the message function but left it in for fun.
Sleep_ms can occasionally interfere with animations if there is a lot running on your machine. Keep this in mind. If this is the case It's not my code that is trash its your CPU or wifi. :P
Occasionally exec fails, but I have also been informed that is not a result of my code and more what the system can handle.
Example:
The Professor let me know that I don't need to make a test script because of the kind of project that I did, so instead here are a few screen shots:
When running make qemu:

When user runs rm :

When user enters guy info:
