Moo-Ack-Productions / MCprep

Blender python addon to increase workflow for creating minecraft renders and animations
https://theduckcow.com/MCprep
GNU General Public License v3.0
280 stars 26 forks source link

Create a coding contributor guide #273

Closed TheDuckCow closed 2 years ago

TheDuckCow commented 2 years ago

Right now, essentially only I, Patrick, am able to fully make use of the local scripts and testing framework. The goal is to 1) generalize how those function, and 2) create a readme contributing guide to point to, to make it easier for others to jump in and contribute to MCprep.

StandingPadAnimations commented 2 years ago

The dev branch could be a good starting point for people wanting to contribute

TheDuckCow commented 2 years ago

Starting with this by porting over some reusable code I've developed before that will enable consistent building and installing of the addon on mac and linux devices. In a future commit, I'll copy over the equivalent windows code but will need to boot to test that further.

TheDuckCow commented 2 years ago

At this stage, I would say this contribution guide is in a good first draft state. I would love to have someone (particularly a Windows user) give this guide a test run. Specifically:

  1. Does the compile.bat script work as expected?
  2. Does the run_tests.bat script run tests? Are they all passing (was it clear that you needed to copy over the MCprep_resources folder first?)
  3. Are any of the instructions unclear? Any suggestions for improvement?

I don't want to mark this as completed until I get at least one person to validate, ideally with a screenshot of the output tests showing what has passed/failed.

StandingPadAnimations commented 2 years ago

Testing the compile.bat file, I get this error:

tar: Couldn't open ./MCprep_addon.zip: Permission denied

However, it does seem to also output:

Finished zipping, check for errors
Installing addon to all files in blender_installs.txt
Installing addon to: C:\Users\user\AppData\Roaming\Blender Foundation\Blender\3.1\scripts\addons
33 File(s) copied
Cleaning up files

So that's some good news

TheDuckCow commented 2 years ago

Thanks a bunch for testing this @StandingPadAnimations - I should have put this in the contributing guide, I actually am unsure why that error is occurring and what to do about it, as if you just ignore that error, to my understanding everything is working fine. You could spot check by deleting MCprep from preferences in the blender UI, shut blender down, run the compile script, and then open blender and fine MCprep is installed (maybe you need to enable first time though; wouldn't need to thereafter though).

Any luck with the automated tests?

StandingPadAnimations commented 2 years ago

The only issue is the lack of compatibility with the Windows store version of Blender (which can be launched without needing a filepath)

StandingPadAnimations commented 2 years ago

So I'll have to download the zip version of 3.1 for further testing

StandingPadAnimations commented 2 years ago

So I did a short test with a portable copy of 3.1. Seems to work fine and outputs the results:


(3, 1, 2)   spawn_mob   " not found in ()\n 
(3, 1, 2)   spawn_mob_linked    reloading mobs\n \n 
(3, 1, 2)   change_skin does not exist\n \n 
(3, 1, 2)   import_jmc2obj  ta yellow_terracotta
(3, 1, 2)   import_mineways_separated   rracotta yellow_wool
(3, 1, 2)   import_mineways_combined    unmapped than mapped
(3, 1, 2)   meshswap_spawner     loaded for spawning
(3, 1, 2)   meshswap_jmc2obj    lium:Error: Meshswap
(3, 1, 2)   meshswap_mineways_separated lium:Error: Meshswap
(3, 1, 2)   meshswap_mineways_combined  lium:Error: Meshswap
(3, 1, 2)   detect_desaturated_images   e or directory\n \n 
(3, 1, 2)   find_missing_images_cycles  e or directory\n \n 
(3, 1, 2)   qa_meshswap_file     local meshswap file
(3, 1, 2)   item_spawner    No items loaded
(3, 1, 2)   entity_spawner  No entities loaded
(3, 1, 2)   model_spawner   No models loaded
(3, 1, 2)   sync_materials  aterials.blend\n \n 
(3, 1, 2)   sync_materials_link aterials.blend\n \n 
(3, 1, 2)   load_material   text is incorrect\n 
(3, 1, 2)   world_tools moon_sun.blend\n \n ```
StandingPadAnimations commented 2 years ago

And here's the command line output:

COMPLETED, 16 passed and 20 failed
Passed tests:
        enable_mcprep, prep_materials, prep_materials_pbr, prep_missing_passes, openfolder, import_world_split, import_world_fail, name_generalize, canonical_name_no_none, canonical_test_mappings, detect_extra_passes, uv_transform_detection, uv_transform_no_alert, uv_transform_combined_alert, test_enable_obj_importer, test_generate_material_sequence
Failed tests:
        spawn_mob:  enum "hostile\mobs - Rymdnisse.blend:/:silverfish" not found in ()\n
        spawn_mob_linked: timeError: Error: Failed to parse mcmob_type  try reloading mobs\n \n
        change_skin:   None  kw)\n RuntimeError: Error: Skin directory does not exist\n \n
        import_jmc2obj: g_table_top yellow_concrete yellow_glazed_terracotta yellow_terracotta
        import_mineways_separated: w_glazed_terracotta yellow_stained_glass yellow_terracotta yellow_wool
        import_mineways_combined: More materials unmapped than mapped
        meshswap_spawner: No meshswap assets loaded for spawning
        meshswap_jmc2obj: e:Error: Meshswap  blue_orchid:Error: Meshswap  allium:Error: Meshswap
        meshswap_mineways_separated: p:Error: Meshswap  blue_orchid:Error: Meshswap  allium:Error: Meshswap
        meshswap_mineways_combined: p:Error: Meshswap  Blue_Orchid:Error: Meshswap  Allium:Error: Meshswap
        detect_desaturated_images: \textures\block\grass_block_side.png': No such file or directory\n \n
        find_missing_images_cycles: ecraft\textures\block\sugar_cane.png': No such file or directory\n \n
        qa_meshswap_file: resources\mcprep_meshSwap.blend: missing tests dir local meshswap file
        item_spawner: No items loaded
        entity_spawner: No entities loaded
        model_spawner: No models loaded
        sync_materials: on/MCprep_resources/resourcepacks/mcprep_default/materials.blend\n \n
        sync_materials_link: on/MCprep_resources/resourcepacks/mcprep_default/materials.blend\n \n
        load_material: or bpy.ops.mcprep.load_material.poll() failed  context is incorrect\n
        world_tools: ripts\addons\MCprep_addon\MCprep_resources\clouds_moon_sun.blend\n \n
StandingPadAnimations commented 2 years ago

Looks like it is possible to run tests using the Windows store version of Blender as long as blender_execs.txt has a line that just says blender.exe

StandingPadAnimations commented 2 years ago

So I've been testing how the system works when modifiying code, and it seems to work perfectly

TheDuckCow commented 2 years ago

With the steam install, is the challenge that it's just hard to identify the full windows path to the install?

And are you still having more than 1 test fail? I'm guessing the issue is the need to copy over the MCprep_resources folder, since those files aren't directly checked in (I added that later as an explicit step in the contributing guide). This would likely be a common source of confusion so I'm considering adding all the resources as part of git LFS, just need to be mindful of the grand total 10GB max size per repo (which I believe is true across branches, although branching on its own won't take up more space, only if the files are changed). It would make fetching quite a bit slower though for an initial commit, but then again also guards against unwanted changes.

StandingPadAnimations commented 2 years ago

I would assume for the steam version there would be issues identifying the path, although it would be easier compared to the Microsoft store.

TheDuckCow commented 2 years ago

Just a follow up, at the moment when you run tests, what is your output (similar to your post here)? If it's only the import_mineways_combined: More materials unmapped than mapped then I think I'll mark this complete for now, any further improvements can come later.

TheDuckCow commented 2 years ago

Feel free to reply @StandingPadAnimations whenever you have a chance, but I realize this is just a dev guide, so no real reason to keep delaying a release for this.