Conversation
|
Looks good – instructions were clear, everything installed correctly, and I was able to compile and run. |
Liquidscroll
left a comment
Liquidscroll
left a comment
There was a problem hiding this comment.
Overall this looks pretty good to me! Tested both setup.py and setup.js and they work as expected.
One small issue regarding the commands listed in README.md.
README.md
Outdated
| ``` | ||
| However, we also need to import the SplashKit library, as its not included in the repository by default. The Node server will import the necessary files automatically on start-up, but this can also be achieved with the included `setup.py` script. | ||
| ```bash | ||
| py setup.py # optional |
There was a problem hiding this comment.
This should probably be changed to python3 setup.py as py is not present on all OS's -- For example, this command does not work in WSL2.
It could also be explained that this command should be run from the root of the project, as the following command npm run server must be run from the Browser_IDE folder.
There was a problem hiding this comment.
Thank you for the note. I was unsure which tool name was the canonical one.
…o Browser_IDE to avoid complication
4 participants
Description
Introduces a Python script and a routine in the Node.js server script, both of which will download the files necessary to run SplashKit Online. The Node.js routine will only run if none of the files to be downloaded are present. The README file has also been updated to account for this simpler way in which to install SKO.
Type of change
expected)
How Has This Been Tested?
I have tested it by removing all relevant files and then either 1) running the Python script, or 2) starting the Node.js server, and then examining the file tree to ensure everything is present.
Testing Checklist
Checklist
Potential Issues
server.jsto work around async design are ugly at best.