Dev to Master for MCprep 3.5 #479
Conversation
As we're dropping 2.7x support, this is no longer needed. Most of the compatability functions will remain though since they're more readable.
Replaced some instances of os.path with the Path object from pathlib for readability. Also added type annotations to MCprepEnv as we're moving towards that. This also means MCprep will not work at all in Blender 2.7x due to the use of new syntax. This will be the first in a long process of modernizing MCprep's code with 2.8 style code. Blender 2.7x users may not be happy, but it's for the better. If anything, 6 years worth of 2.7x support was a mistake (in my opinion), as it limited what we could do and opened MCprep to even more bugs (like in Blender 2.93 with make_annotations, which ironically now is deprecated).
Replaced the old = syntax with the new : syntax. This also removes the need for make_annotations, hence why they're removed.
As we're removing 2.7x support, it makes sense to remove Blender Internal compatability as the engine is not present in Blender 2.8 and above. Even UPBGE (a fork of the Blender game engine that's still maintained) uses EEVEE, so there's no reason to continue supporting Blender Internal.
The bang at the top meant Linux users couldn't run the test suite. This changes now allows Linux users to run tests
Deprecation warning function was moved in MCprepEnv since it made more sense to put it there
Originally matprep_cycles used to take multiple arguments, each one representing a seperate option. This was annoying as it meant the function has an unfathomable amount of arguments to keep track of. This has been replaced with a dataclass (called PrepOptions) that keeps track of all the options selected by the user. Not only does it make it slightly easier for developers, it also gives the following benefits: - More info on what types the different options are - Better organization Previously we couldn't do this due to 2.7x support, hence this being in mcprep-3_5-recore.
To avoid polluting output
To avoid polluting output
I'm honestly not sure how I missed these, but good thing they were caught. Now MCprep shouldn't be dealing with any errors related to env now...
While removing the last of the ENVs, I noticed some deprecation warnings regarding the use of this function, so I've went ahead and removed it. Since this function calls other deprecated functions as well, what should have been 1 warning was instead 3. Apologies to anyone who had to suffer that in their terminal ;-;
In all honesty I doubt we'll get around to actually removing these functions in 3.5.1, but since we're merging this with the 3.5 feature branch, I think it's important to warn developers and make them think about the eventual demise of these functions. Plus there's absolutely no reason to use deprecated functions for 3.5 when we'll have nice features like type annotations. Anyway, hopefully after this commit, we'll merge this with the MCprep 3.5 feature branch *fingers crossed*
For some reason the spawner was missing the use of env, causing errors. While this has been fixed, it does illustrate an issue with the current deprecation system in that deprecated stuff will error out. More investigation is needed.
Removed references to darker in CONTRIBUTING.md
|
I was able to run tests on Linux (using
With all of that, here are my results I do however get some intersting errors: Looks like we got to fix some texture stuff in Git LFS |
|
Based on #483, it does look like tests pass on Arch Linux. I'm not sure what are the 2 tests that are skipped, but I'll just mark Linux testing as done |
|
Alright, just for the purposes of keeping it up to date, I've updated the |
|
Yeah the skipped tests are fine, they're semi annoying ones that would open up folders or webrowser pages which we can set to include if we really want to be exhaustive, but not necessary for now. I'm surprised there aren't more errors actually, I'm not worried about having 100% of assets in the test file for git lfs, so long as tests pass. The obj files could be certainly made smaller or more specific for our needs to not have extras, but this is ok for now I'd say and not something to fret too hard over. I think we're very close, just need to align on that concept art - will chat over Discord! |
|
Work on releasing in progress - I've migrated a few more unit tests, which have thankfully highlighted a few more fixes to assets I'm working through right now. Mostly save versions, addressing a couple non-relative paths, etc. Giving this another day then expecting to cut the release. |
This includes QA checking scripts, which have already uncovered one issue with one of the updated rigs.
|
We are even closer with #487 but I did come across a few issues that remain:
|
|
Based on the error, it might be a case of having to reload assets in the test (users have to reload them anyway when creating a new file) |
|
Regarding that error, I realize now I actually had already addressed that long ago by changing how we apply materials during the creation stage, so it should be mitigated. These other issues I expect to be pre-existing, and we can see about addressing in a patch - not worth pushing the release out furhter. I got tied up tonight, but for real I plan to cut the release tomorrow. If you had any comments on this PR feel free to add them, but I won't wait for them as I'll focus on merging than and then doing final merging here to cut the release. |
|
We just need to add Git LFS instructions (I did notice we need to update documentation for tests too) in I won't be able to push any changes today or tomorrow, so someone else needs to do those changes to @zNightlord @MajesticalDiscomfort, any comments before we release MCprep 3.5 tomorrow? |
|
Removed this from “Draft” status since we plan to merge tomorrow |
|
Thanks - I'm doing some final sorting of things, and then I'll merge the other PR followed by this one. This is it for real - just a couple hours at most now! |
This was a weird one. Essentially, running the unittests would always result in the `mcprep_data_update.json` file being renamed to `mcprep_data.json` (and since that file isn't tracked in git, would look like the file was deleted wrt to local changes). This was happening, out of all things, due to this line in the unittest runner: unittest.TestLoader().discover(MODULE_DIR, "*_test.py") This is because the discover command clearly has to attempt loading of the modules it's scanning over, and at some point we import the env module and thus the __init__ code on module load would trigger, resulting in the file rename. By moving the rename into the register function, we ensure it only happens in the context of an addon, which is all we really need for it.
Unfortunately, we do still have the issue that the test runner renames the mcprep_data_update.json file, and modifies checked-in mtl files, but we can address this in the future.
The new test runner in python is straight forward enough to use that I feel there is no longer a need for any system-specific shell files.
…ions Migrating more critical tests ahead of release.
|
Here we go, 3.5! |
Development on new features has calmed down for the most part, so I think it's safe to say we're now in the final stage before release. While GitHub says only 1 approval is needed, this PR needs approval from all developers to be merged.
Checklist of things we need to do before merging
CONTRIBUTING.md(I haven't been able to pull assets yet)Wolf. #409Warden Rig Submission #420After release, if possible, we should publish the RC builds (after changing the commits they point to) just to make it easier for users to get. Future RC builds will be published, as MCprep 3.5 will ignore RCs.