draw_circle usage examples (animation + interactive)#712
draw_circle usage examples (animation + interactive)#712jankiluitel wants to merge 5 commits intothoth-tech:mainfrom
Conversation
❌ Deploy Preview for splashkit failed.
|
There was a problem hiding this comment.
Code Review
Issues have been found in the submission, hence I cannot approve it. Please refer to the comments that I left in the files.
Problems
- C++ does not need to default the
splashkitnamespace, as that namespace does not exist (code runs well without it) - Python code contains multiple syntax errors, comments with descriptions and recommendations are given on the respective lines
Files that need to run before approval
-
draw_circle-2-animation.cpp -
draw_circle-2-animation.py -
draw_circle-3-interactive.cpp -
draw_circle-3-interactive.py
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| x = 0 | ||
|
|
||
| while not window_close_requested("Circle Animation"): |
There was a problem hiding this comment.
argument of window_close_requested should be the window object (returned from open_window earlier)
|
|
||
| while not window_close_requested("Circle Animation"): | ||
| process_events() | ||
| clear_screen(Color.WHITE) |
There was a problem hiding this comment.
Color.WHITE does not exist in SplashKit's Python; you might need to use color_white() or, instead of the entire function, use clear_screen_to_white()
| process_events() | ||
| clear_screen(Color.WHITE) | ||
|
|
||
| draw_circle(Color.RED, x, 300, 50) |
There was a problem hiding this comment.
First argument is invalid, as Color.RED does not exist; you might need to use color_red() instead
| x = 400 | ||
| y = 300 | ||
|
|
||
| while not window_close_requested("Interactive Circle"): |
There was a problem hiding this comment.
window_close_requested takes the result of open_window as an argument
|
|
||
| while not window_close_requested("Interactive Circle"): | ||
| process_events() | ||
| clear_screen(Color.WHITE) |
There was a problem hiding this comment.
Color.WHITE does not exist in Splashkit's Python
| process_events() | ||
| clear_screen(Color.WHITE) | ||
|
|
||
| if key_down(KeyCode.LEFT_KEY): |
There was a problem hiding this comment.
KeyCode.LEFT_KEY does not exist, you might need to use KeyCode.left_key instead; similar issue perists at lines 16, 18, 20
| if key_down(KeyCode.DOWN_KEY): | ||
| y += 5 | ||
|
|
||
| draw_circle(Color.BLUE, x, y, 50) |
There was a problem hiding this comment.
Color.BLUE does not exist in Python, you might need to use color_blue() instead
|
|
||
| draw_circle(Color.BLUE, x, y, 50) | ||
|
|
||
| refresh_screen(60) No newline at end of file |
There was a problem hiding this comment.
This function does not take arguments, use refresh_screen_with_target_fps() to set the speed of refresh
|
I reviewed the implementation of the usage examples for the draw_circle function. The overall structure of the contribution is clear and well-organised. The examples follow the correct naming conventions and folder structure, and the inclusion of both animation and interactive examples makes the usage more practical and easy to understand. A strong aspect of this work is that it demonstrates real use cases, such as moving a circle using arrow keys and animating it across the screen. This helps users understand how the function works in an interactive environment rather than just a static example. The integration with the usage example system also appears to be correctly handled. However, there are several issues that need to be addressed, especially in the Python examples. For example, Color.BLUE, Color.WHITE, and Color.RED are not valid in SplashKit Python and should be replaced with functions like color_blue(), color_white(), and color_red(). Similarly, KeyCode.LEFT_KEY and other key constants should be updated to the correct format such as KeyCode.left_key. Another issue is the use of refresh_screen(60) in Python, which is not valid. Instead, refresh_screen_with_target_fps() should be used. There is also a problem with window_close_requested() being used incorrectly, as it should take the window object returned by open_window() as an argument. In the C++ example, the use of using namespace splashkit; may cause issues depending on the environment, and the code appears to work without it, so it could be removed for consistency. Overall, the examples are well-designed and useful, but fixing these language-specific issues and ensuring consistency across all implementations would significantly improve the correctness and usability of the contribution. |
Osaid2993
left a comment
There was a problem hiding this comment.
The examples are clear and the interactive idea is nice, but I think a few style and completeness issues need to be fixed before approval.
-
The C++ files use using namespace splashkit;, which does not match the usual SplashKit example style. I also noticed both the C++ and Python versions use window_close_requested(...) instead of the more standard quit_requested() loop style used in other examples.
-
In the Python files, the colour usage also looks inconsistent with the usual SplashKit Python style. From what I’ve seen in other examples, this should probably use the normal lowercase colour functions for consistency.
The example idea is good, but I think these points should be addressed first.
kottochii
left a comment
There was a problem hiding this comment.
Peer review
One of the Python examples (respective comment is attached) crashes due to errors in the code.
Files to check
-
public/usage-examples/graphics/draw_circle-3-interactive.py
| process_events() | ||
| clear_screen_to_white() | ||
|
|
||
| if key_down(left_key()): |
There was a problem hiding this comment.
Method left_key() does not exist; instead, you should use KeyCode.left_key (a value, not a function). Same goes for the rest of the key identifiers in this file.
There was a problem hiding this comment.
Thanks for the detailed feedback!
I have fixed all issues:
- Updated Python API usage (colors, keys, refresh function)
- Replaced incorrect window loop with quit_requested()
- Removed unnecessary namespace usage in C++
- Ensured consistency across all examples
Please let me know if anything else needs improvement.
Thanks for the detailed feedback! I have fixed all issues:
Please let me know if anything else needs improvement. |
Thanks for the detailed feedback! I have fixed all issues:
Please let me know if anything else needs improvement. |
kottochii
left a comment
There was a problem hiding this comment.
All of the previously flagged issues have been addressed.
All examples are confirmed to run and to show up in the web version.
Osaid2993
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback. I checked the updated files and the issues I raised have been fixed. The examples now look more consistent and aligned with the project style.
222448082Ashen
left a comment
There was a problem hiding this comment.
Description
This PR adds advanced usage examples for the draw_circle function in the SplashKit library. It introduces two new categories of examples: Animation (a circle moving across the screen) and Interaction (controlling a circle with keyboard input). These examples are provided in both C++ and Python to ensure parity across supported languages and to help users understand how to move from static drawing to dynamic application logic.
The changes were refined through multiple review cycles to correct Python-specific API calls (colors, key codes, and refresh functions) and to align with SplashKit’s preferred C++ coding standards (removing global namespaces).
Type of change
- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
- Documentation (update or new)
How Has This Been Tested?
The examples were verified through the following methods:
- Manual Execution: Both Python and C++ scripts were compiled/run to ensure the logic works as intended (no crashes, correct input handling).
- Scraper Integration: Verified that the new files are correctly picked up by the documentation scraping system for display on the website.
- Deploy Preview: Checked via Netlify bot (initial failure was resolved in commit
18a94bb).
- Tested in latest Chrome
- npm run build
Checklist
If involving code
- My code follows the style guidelines of this project
- I have performed a self-review of my own code
- I have commented my code in hard-to-understand areas
- I have made corresponding changes to the documentation
- My changes generate no new warnings
If modified config files
N/A - No config files were modified.
Folders and Files Added/Modified
- Added:
-
public/usage-examples/graphics/draw_circle-2-animation.cpp -
public/usage-examples/graphics/draw_circle-2-animation.py -
public/usage-examples/graphics/draw_circle-3-interactive.cpp -
public/usage-examples/graphics/draw_circle-3-interactive.py
-
Additional Notes
The initial issues regarding Python syntax (e.g., Color.RED vs color_red()) and C++ namespaces were fully resolved. The loops now consistently use quit_requested() instead of window_close_requested() to match the project's idiomatic style. All examples are now confirmed to be working and approved by multiple peer reviewers.
This PR enhances the usage examples for draw_circle by adding advanced and interactive demonstrations.
Changes:
These examples improve understanding of dynamic and interactive usage of draw_circle.