gilesknap / mciwb

Minecraft Interactive world builder
Apache License 2.0
298 stars 6 forks source link

Fixes for signs in 1.20, dependency/lintings updates #132

Closed N6UDP closed 8 months ago

N6UDP commented 8 months ago
gilesknap commented 8 months ago

This is great, thanks! Inspires me to get back into this project.

Do you want to take a look at why the CI is failing? I could take a look this weekend if not.

gilesknap commented 8 months ago

I have invited you as a collaborator so you can look at the CI if you wish.

I'm OK with only supporting the later versions of Minecraft - the goal of this project is to have fun with python and it would be pretty hard to support all versions of MC.

BTW, it would be interesting to look into what else has changed re NBT. I found that signs were about the most useful thing for creating an interactive interface because they had the most rich NBT but perhaps there are better choices now. Also, I think I recall that I have to constantly poll a range of blocks for the sign, it would be better if a search were available through the API but I never found such a thing (witness the clunky workaround for get_block https://github.com/mcipc-tools/mcwb/blob/96c16003d6365a4d017d82a62ce06b4677f95fe6/mcwb/api.py#L78-L96).

N6UDP commented 8 months ago

"Fixed" some of the CI by pinning the python version (seems broken due to disttools deprecation in 3.12 see https://docs.python.org/3.10/whatsnew/3.10.html#distutils-deprecated ). Not entirely sure what's up with the permission denied error coming from the test_backup which works locally but not on actions. I did try moving from /tmp to $HOME but that didn't make a difference. FWIW speaking of updates/changes in MC: https://github.com/conqp/mcipc/commit/c231c63cf1611a7f324799cf3949485d27956b94

gilesknap commented 8 months ago

Regarding issues with distutils. This project is based on https://pypi.org/project/python3-pip-skeleton/ and usually merging in the latest version of that will fix many of the dependency issues.

However, some of my colleagues are replacing the skeleton with a copier project to make pulling in updates easier. https://github.com/DiamondLightSource/python-copier-template/tree/ci-split. So, this weekend I'll have a go at switching this project to use that with the hope that it will fix CI issues and make it easier to fix future breaking dependencies.

Yes we should get your new blocks added, I look forward to a new release of conqp.

I have had loads of trouble with that backup test. Every time I have it running nicely I find it fails in CI. Maybe we should disable it during CI and keep it around as a local test only.

codecov[bot] commented 8 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (b6e0ecc) 70.71% compared to head (a97d744) 64.23%. Report is 2 commits behind head on main.

Files Patch % Lines
src/mciwb/__main__.py 87.50% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #132 +/- ## ========================================== - Coverage 70.71% 64.23% -6.49% ========================================== Files 16 16 Lines 898 903 +5 ========================================== - Hits 635 580 -55 - Misses 263 323 +60 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

N6UDP commented 8 months ago

Makes sense. For now let's disable the test in CI - I might have some time this weekend myself to see about getting it to run in actions. Other than the coverage report (expected) it now passes the other CI tests. The reason for the new param etc is because the @v4 actions actually have a breaking change apparently see https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md for details.

gilesknap commented 8 months ago

Looks Good. Merging.

re the v4 artifact action. This is fixed in the skeleton so is just the kind of thing that using skeleton is good for. I also think we are dropping the whole upload publishing of requirements.txt thing as too cumbersome. To be replaced by the use of a constraints file and dependabot.

gilesknap commented 7 months ago

just a headsup: @N6UDP don't do any more work on the CI - I'm pulling in our latest copier project which will rewrite all of the CI.