Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - Other rig properties, mob type, color variant #572

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from
Draft

WIP - Other rig properties, mob type, color variant #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7d35b1f
Layout things
zNightlord Jul 30, 2023
9ff1e4e
Add back the enum
zNightlord Sep 2, 2023
ea6956c
unf
zNightlord Sep 10, 2023
8f500da
some extra code
zNightlord Oct 12, 2023
e3f3cfe
swap mob variant
zNightlord Jan 25, 2024
75d9801
change
zNightlord Apr 15, 2024
a66bae9
init wip-prop-other-attributes
zNightlord May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions MCprep_addon/materials/prep.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,8 @@ def execute(self, context):
else:
self.report(
{"ERROR"},
"Nothing modified, be sure you selected objects with existing materials!"
)

"Nothing modified, be sure you selected objects with existing materials!")

addon_prefs = util.get_user_preferences(context)
self.track_param = context.scene.render.engine
self.track_exporter = addon_prefs.MCprep_exporter_type
Expand Down
104 changes: 103 additions & 1 deletion MCprep_addon/materials/skin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,17 @@
from . import generate
from .. import tracking
from .. import util
from ..spawner import spawn_util

from ..conf import env
from .prep import McprepMaterialProps


swap_all_imgs_desc = (
"Swap textures in all image nodes that exist on the selected \n"
"material; if off, will instead seek to only replace the images of \n"
"nodes (not image blocks) named MCPREP_SKIN_SWAP"
)
Comment on lines +40 to +44
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to use env._ for i18n

Suggested change
swap_all_imgs_desc = (
"Swap textures in all image nodes that exist on the selected \n"
"material; if off, will instead seek to only replace the images of \n"
"nodes (not image blocks) named MCPREP_SKIN_SWAP"
)
swap_all_imgs_desc = (
env._("Swap textures in all image nodes that exist on the selected \n")
env._("material; if off, will instead seek to only replace the images of \n")
env._("nodes (not image blocks) named MCPREP_SKIN_SWAP")
)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I agree with the call to use env._ here, I do think we should do it only once on the final constructed multi line string block. We have no way of knowing the number of lines or breaks for other locales.



swap_all_imgs_desc = (
Expand Down Expand Up @@ -335,7 +344,29 @@ def download_user(self, context: Context, username: str) -> Optional[Path]:
self.track_param = "username"
return saveloc

def check_entity_texture(texture_path: str) -> Optional[Path]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a docstring here so we can better understand this function.

context = bpy.context
addon_prefs = util.get_user_preferences(context)
active_pack = bpy.path.abspath(context.scene.mcprep_texturepack_path)
active_pack = os.path.join(
active_pack, "assets", "minecraft", "textures", "entity")

base_pack = bpy.path.abspath(addon_prefs.custom_texturepack_path)
base_pack = os.path.join(
base_pack, "assets", "minecraft", "textures", "entity")

# if not os.path.isdir(active_pack):
# env.log(f"No models found for active path {active_pack}")
# return None
base_has_textures = os.path.isdir(base_pack)
Comment on lines +351 to +361
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really the biggest fan of os.path for filepath operations, it's all string manipulation (vs pathlib.Path which is much nicer in abstraction). Not a blocker, but I'd advise using pathlib instead of os.path for new code since it's much better, and works with os.path functions anyway if needed.



if base_has_textures:
return generate.find_from_texturepack(texture_path)
else:
env.log(f"Base resource pack has no entity texture folder: {base_pack}")
return None

# -----------------------------------------------------------------------------
# Operators / UI classes
# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -395,7 +426,6 @@ def execute(self, context):

return {'FINISHED'}


Comment on lines 428 to -398
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: Good to keep 2 spaces between top-level entities

class MCPREP_OT_apply_skin(bpy.types.Operator):
"""Apply the active UIlist skin to select characters"""
bl_idname = "mcprep.applyskin"
Expand Down Expand Up @@ -703,6 +733,77 @@ def execute(self, context):
return {'FINISHED'}


class MCPREP_OT_swap_skin_variant(bpy.types.Operator, spawn_util.VariationProp):
"""Apply the active UIlist skin to select characters"""
bl_idname = "mcprep.swap_skin"
bl_label = "Swap mob skin"
bl_description = "Swap the mobs variant"
Comment on lines +739 to +740
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about i18n here too

Suggested change
bl_label = "Swap mob skin"
bl_description = "Swap the mobs variant"
bl_label = env._("Swap mob skin")
bl_description = env._("Swap the mobs variant")

bl_options = {'REGISTER', 'UNDO'}

# def invoke(self, context, event):
# return context.window_manager.invoke_props_dialog(
# self, width=300 * util.ui_scale())

# def draw(self, context: Context):
# layout = self.layout
# self.draw_variation_ui(context, layout)

@tracking.report_error
def execute(self, context: Context):
obj = context.object
if obj.type != 'ARMATURE':
self.report({'ERROR'}, "Please select an armature")
return {'CANCELLED'}
mats, skipped = getMatsFromSelected(obj, False)
mob_type = obj.get("MCPREP_mob_type", "Custom")

if mob_type == "Custom":
self.report({'ERROR'}, "You are not allowed to use on Custom rig")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight nitpick on spelling and grammar.

Suggested change
self.report({'ERROR'}, "You are not allowed to use on Custom rig")
self.report({'ERROR'}, "You are not allowed to use on a custom rig")

return {'CANCELLED'}

texture_paths = self.get_texture_paths(mob_type)
name = mob_type.lower().replace(" ", "_")
if mob_type == "Villager":
self.doVillager(mats, texture_paths)
elif mob_type == "Zombie":
check_entity_texture(context)
if self.zombie_variation in ["ZOMBIE", "HUSK"]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you've mentioned this before and I tend to agree: better to not hard code rig-specific data in our code files, but rather (if anywhere) into our mcprep json data file, or better yet onto properties or metadata of the rigs themselves.

Though if in this case, is it just about the player skin 1.8 format aspect ratio? If so, I actually have some code somewhere in MCprep that does this check in order to convert to 1.8 formats on the live, with the username download function.

# Using Player model so convert player skin to 1.8 format
# loadSkinFile()
pass
else:
# Assign textures materials for drown
pass
elif mob_type == "Skeleton":
pass

elif mob_type in ("Allay", "Vex"):
if mob_type == "Allay":
name += "/" + name
else:
name = "illager"
Comment on lines +765 to +784
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a good area to use an Enum. Perhaps something like MobType, where each value is rqual to a string. From there, it you could use MobType(whatever_string) to instantiate it.


else:
pass
return {'FINISHED'}

def doVillager(self, materials: List[Material], texture_paths: List[str]):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick about Python convention, use snake_case for functions.

Suggested change
def doVillager(self, materials: List[Material], texture_paths: List[str]):
def do_villager(self, materials: List[Material], texture_paths: List[str]):

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with above comment, but going further - is there some way that such a function like this could be generalized (and not specify villager in the name)?

Imagine that we somehow overhauled all our rigs, or provided different rigs for 3.0+ (for asset browser) vs 2.8/2.9. I would rather we avoid a situation where we have to change MCprep code just because rigs changed.

So, I think these specialized functions can still exist, I would just rather we try to make them generally useable and if anything rely on property assignments we check for to make them useable (I'm happy to change the rigs to match as needed, remember we want to save them in 2.80 specifically to be forwards compatible)

""" materials texture path order follows by profession, profession level, type """
prof_mat, biome_mat, level_mat = None, None, None
if mat[0].get("MCPREP_VILLAGER_PROFESSION"):
image = util.loadTexture(texture_paths[0])
proStat = generate.assert_textures_on_materials(image, [mat])
if mat.get("MCPREP_VILLAGER_BIOME"):
image = util.loadTexture(texture_paths[1])
biomeStat = generate.assert_textures_on_materials(image, [mat])
if mat.get("MCPREP_VILLAGER_LEVEL"):
Comment on lines +793 to +799
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like with all of these properties, we should create a new file (like mcprep_props.py or variables.py, leaning more towards the latter) of constants assigned to these variable names. That way, we can document them better and avoid typos.

Not a blocker for this PR, but just something I wanted to say before I forget.

image = util.loadTexture(texture_paths[2])
levelStat = generate.assert_textures_on_materials(image, [mat])
if not all([proStat, biomeStat, levelStat]):
self.report({'ERROR'}, "Something wrong happen during swap variant texture")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another nitpick on grammar, although it's pretty vague anyway.

Suggested change
self.report({'ERROR'}, "Something wrong happen during swap variant texture")
self.report({'ERROR'}, "Something wrong happen while swapping the variant texture")

else:
pass
Comment on lines +804 to +805
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else-pass isn't needed, I'd remove this (unless pass is just a placeholder at the moment.


class MCPREP_OT_download_username_list(bpy.types.Operator):
"""Apply the active UIlist skin to select characters"""
bl_idname = "mcprep.download_username_list"
Expand Down Expand Up @@ -788,6 +889,7 @@ def execute(self, context):
ListColl,
MCPREP_OT_swap_skin_from_file,
MCPREP_OT_apply_skin,
MCPREP_OT_swap_skin_variant,
MCPREP_OT_apply_username_skin,
MCPREP_OT_download_username_list,
# MCPREP_OT_skin_fix_eyes,
Expand Down
119 changes: 77 additions & 42 deletions MCprep_addon/mcprep_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,8 @@ class MCPREP_PT_skins(bpy.types.Panel):

def draw(self, context):
layout = self.layout
wm_props = context.window_manager.mcprep

if addon_just_updated():
restart_layout(layout)
return
Expand All @@ -951,56 +953,63 @@ def draw(self, context):
sind = context.scene.mcprep_skins_list_index
mob_ind = context.scene.mcprep_props.mob_list_index
skinname = None

row = layout.row()
row.label(text=env._("Select skin"))
row.operator(
"mcprep.open_help", text="", icon="QUESTION", emboss=False
).url = "https://theduckcow.com/dev/blender/mcprep/skin-swapping/"

# set size of UIlist
row = layout.row()
col = row.column()

is_sortable = len(env.skin_list) > 1
rows = 1
if (is_sortable):
rows = 4

# any other conditions for needing reloading?
if not env.skin_list:
col = layout.column()
col.label(text=env._("No skins found/loaded"))
p = col.operator(
"mcprep.reload_skins", text=env._("Press to reload"), icon="ERROR")
elif env.skin_list and len(env.skin_list) <= sind:
col = layout.column()
col.label(text=env._("Reload skins"))
p = col.operator(
"mcprep.reload_skins", text=env._("Press to reload"), icon="ERROR")
else:
col.template_list(
"MCPREP_UL_skins", "",
context.scene, "mcprep_skins_list",
context.scene, "mcprep_skins_list_index",
rows=rows)

col = layout.column(align=True)

row = col.row(align=True)
row.scale_y = 1.5
if env.skin_list:
skinname = bpy.path.basename(env.skin_list[sind][0])
p = row.operator("mcprep.applyskin", text=f"Apply {skinname}")
p.filepath = env.skin_list[sind][1]
row.prop(wm_props, "skin_modes",expand=True)
if wm_props.skin_modes == 'PLAYER':

# set size of UIlist
row = layout.row()
col = row.column()

is_sortable = len(env.skin_list) > 1
rows = 1
if (is_sortable):
rows = 4

# any other conditions for needing reloading?
if not env.skin_list:
col = layout.column()
col.label(text="No skins found/loaded")
p = col.operator(
"mcprep.reload_skins", text="Press to reload", icon="ERROR")
elif env.skin_list and len(env.skin_list) <= sind:
col = layout.column()
col.label(text="Reload skins")
p = col.operator(
"mcprep.reload_skins", text="Press to reload", icon="ERROR")
else:
row.enabled = False
p = row.operator("mcprep.skin_swapper", text=env._("No skins found"))
row = col.row(align=True)
row.operator("mcprep.skin_swapper", text=env._("Skin from file"))
row = col.row(align=True)
row.operator("mcprep.applyusernameskin", text=env._("Skin from username"))
col.template_list(
"MCPREP_UL_skins", "",
context.scene, "mcprep_skins_list",
context.scene, "mcprep_skins_list_index",
rows=rows)

col = layout.column(align=True)

row = col.row(align=True)
row.scale_y = 1.5
if env.skin_list:
skinname = bpy.path.basename(env.skin_list[sind][0])
p = row.operator("mcprep.applyskin", text=f"Apply {skinname}")
p.filepath = env.skin_list[sind][1]
else:
row.enabled = False
p = row.operator("mcprep.skin_swapper", text="No skins found")
row = col.row(align=True)
row.operator("mcprep.skin_swapper", text="Skin from file")
row = col.row(align=True)
row.operator("mcprep.applyusernameskin", text="Skin from username")
else:
row = layout.row()
col = row.column()

wm_props.draw_variation_ui(context, col)
col.operator("mcprep.swap_skin")
split = layout.split()
col = split.column(align=True)
row = col.row(align=True)
Expand Down Expand Up @@ -1044,6 +1053,12 @@ def draw(self, context):
# datapass = scn_props.mob_list[mob_ind].mcmob_type
tx = f"Spawn {name} with {skinname}"
row.operator("mcprep.spawn_with_skin", text=tx)
b_row.label(text="Resource pack")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same story about i18n

Suggested change
b_row.label(text="Resource pack")
b_row.label(text=env._("Resource pack"))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the code was originally written months ago. I hold this for awhile and stuck there so. So noted.

subrow = b_row.row(align=True)
subrow.prop(context.scene, "mcprep_texturepack_path", text="")
subrow.operator(
"mcprep.reset_texture_path", icon=LOAD_FACTORY, text="")



class MCPREP_PT_materials(bpy.types.Panel):
Expand Down Expand Up @@ -1969,6 +1984,25 @@ class McprepProps(bpy.types.PropertyGroup):
effects_list_index: bpy.props.IntProperty(default=0)


class MCprepWindowManager(spawn_util.VariationProp, bpy.types.PropertyGroup):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might also need some additional context, where and how is this manager window being used? Is this mean to be just open session property settings used to drive operator functions?

If so, might warrant some more discussion on how I'm thinking things will evolve, since we're moving towards things like resource pack layering

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since this is more of an UI properties that need only one, single to operate and doesn't need to save to the context scene, blend file so WindowManager would be the best place for it.

skin_modes : bpy.props.EnumProperty(
name="Modes",
description="Skinswap modes",
items=(
('PLAYER', "Player", ""),
('MOB', "Mob/Entity", "")
Comment on lines +1989 to +1993
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also need to use i18n functions

Suggested change
name="Modes",
description="Skinswap modes",
items=(
('PLAYER', "Player", ""),
('MOB', "Mob/Entity", "")
name=env._("Modes"),
description=env._("Skinswap modes"),
items=(
('PLAYER', env._("Player"), ""),
('MOB', env._("Mob/Entity"), "")

)
)

@classmethod
def register(cls):
bpy.types.WindowManager.mcprep = bpy.props.PointerProperty(type=cls)

@classmethod
def unregister(cls):
del bpy.types.WindowManager.mcprep


# -----------------------------------------------------------------------------
# Register functions
# -----------------------------------------------------------------------------
Expand All @@ -1977,6 +2011,7 @@ class McprepProps(bpy.types.PropertyGroup):
classes = (
McprepPreference,
McprepProps,
MCprepWindowManager,
MCPREP_MT_mob_spawner,
MCPREP_MT_meshswap_place,
MCPREP_MT_item_spawn,
Expand Down
5 changes: 4 additions & 1 deletion MCprep_addon/spawner/entities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@

import bpy

from bpy.types import Context
from bpy.types import (
Context,
Event
)
from ..conf import env, Entity
from .. import util
from .. import tracking
Expand Down
Loading