Conversation
The Extract-7Zip command is not supported anymore by GitHub actions. It has been replaced with the Expand-7ZipArchive. Source: actions/runner-images#9361 (comment)
The distutils package has been removed with Python 3.12 (that is the Python version run by the GitHub actions at the moment). We were using it for distutils.dir_util.copy_tree, which has been replaced with shutil.copytree. Source: https://docs.python.org/3.11/whatsnew/3.10.html#distutils-deprecated
ax6's RTC tests do not actually need CGB; DMG is fully capable of run these roms. Therefore, the CGB requirement has been removed and the screenshots of such roms have been updated with the DMG's one.
|
I'm going to work on this in a bit once I get the other emulators updated. |
avivace
left a comment
There was a problem hiding this comment.
The changes related to DocBoy seem ok to me but:
- ax6 test roms results are updated (?)
- there are some unrelated changes in the ares emulator and in the main pipeline
.github/workflows/main.yml
Outdated
| Start-Service audio* | ||
| Invoke-WebRequest https://github.com/duncanthrax/scream/releases/download/3.6/Scream3.6.zip -OutFile C:\Scream3.6.zip | ||
| Extract-7Zip -Path C:\Scream3.6.zip -DestinationPath C:\Scream | ||
| Expand-7ZipArchive -Path C:\Scream3.6.zip -DestinationPath C:\Scream |
emulators/ares.py
Outdated
| import PIL.Image | ||
| import PIL.ImageOps | ||
| from distutils.dir_util import copy_tree | ||
| from shutil import copytree |
There was a problem hiding this comment.
At the time of my PR this was making the pipeline fail, as distutils has been removed starting from Python 3.12 release note.
I'll revert this and the other changes, as they have already been fixed in the main branch.
emulators/ares.py
Outdated
|
|
||
| if not os.path.exists("emu/ares/ares.exe"): | ||
| copy_tree(os.path.join("emu/ares", os.listdir("emu/ares")[0]), "emu/ares") | ||
| copytree(os.path.join("emu/ares", os.listdir("emu/ares")[0]), "emu/ares") |
|
Hi everyone. I've just noticed that the original repo has been moved and has been updated recently, great. I've noticed that there a few conflicts now: I'll update my branch so that we can make the merge smoother. Furthermore, now DocBoy also CGB in addition to DMG, so I'll consider add it as well. |
testroms/ax6.py
Outdated
| Test("ax6/rtc3test-1.gb", runtime=9.5, model=CGB), | ||
| Test("ax6/rtc3test-2.gb", runtime=7.5, model=CGB), | ||
| Test("ax6/rtc3test-3.gb", runtime=20.0, model=CGB), | ||
| Test("ax6/rtc3test-1.gb", runtime=9.5), | ||
| Test("ax6/rtc3test-2.gb", runtime=7.5), | ||
| Test("ax6/rtc3test-3.gb", runtime=20.0), |
There was a problem hiding this comment.
Please revert the ax6 test changes for now. This will be changed, but not in this PR. Thanks!
Can we merge without the CI part at the moment? That shouldn't need any other fix, right? Then we can work on the CI part after those other things you mentioned are fixed. |
|
This needs a CI workflow like the other emulators. See the GitHub CI workflow files |
All right, didn't know about the CI files, I'll add it later then. |
I know, but merging what we have won't hurt (we simply won't have the results just yet in the gh-pages branch) and CI can be added later - I just don't want to have this blocked nor blocking if there's not a real reason for it. |
The main blocker was that other PR that just got merged because I expected a merge conflict. My full intent was to fix the merge conflict here and add the CI myself then merge this PR after. Next on my to-do list. But I'm about to see a movie with the wife now so it would be after. If you would like to fix the merge conflict and merge without the CI that is fine. I'll add the CI later. No worries. 😊 |
|
Although also, it's not really "blocking" anything to leave this open, since whether or not it gets merged as-is, Vulcandth's next intended task is to add CI for it. Unless somebody else's intended tasks need this merged first too, and they don't need CI to work for it. |
|
In the latest commits I've solved the conflicts with upstream, added the ci-docboy.yml to the workflows and added support for DocBoy in CGB as well. Let me know if that's fine. |
Changes introduced by this PR:
As a consequence, this PR also update usages of Windows commands and Python packages that are not supported yet by the GitHub actions.
Commits:
Closes #24.
Closes #25.