-
Notifications
You must be signed in to change notification settings - Fork 26
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.
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
Fix bug in spawn_util #405
Merged
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1cf3335
Fix bug in spawn_util
CFeeney333 f49fe67
Fix bug in spawn_util.py
CFeeney333 323028e
Remove commented out code
CFeeney333 a51f45b
Wrap long comments
CFeeney333 1f6c957
Fixing check_blend_eligible and adding more tests
TheDuckCow 8cb4d75
Merge remote-tracking branch 'origin/dev' into dev
TheDuckCow 3270d08
Creating a more real integration test for the warden rig selection.
TheDuckCow 09bbc5f
Fixed typo in test list.
TheDuckCow File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,21 +98,29 @@ def filter_collections(data_from): | |
|
||
|
||
def check_blend_eligible(this_file, all_files): | ||
"""Returns true if the path blend file is ok for this blender version. | ||
"""Returns true if this_file is the BEST blend file variant for this rig. | ||
|
||
Created to better support older blender versions without having to | ||
compeltely remake new rig contirbutions from scratch. See for more details: | ||
completely remake new rig contributions from scratch. See for more details: | ||
https://github.com/TheDuckCow/MCprep/issues/317 | ||
|
||
If the "pre#.#.#" suffix is found in any similar prefix named blend files, | ||
evaluate whether the target file itself is eligible. If pre3.0.0 is found | ||
in path, then this function returns False if the current blender version is | ||
3.0 to force loading the non pre3.0.0 version instead, and if pre3.0.0 is | ||
not in the path variable but pre#.#.# is found in another file, then it | ||
returns true. If no pre#.#.# found in any files of the common base, would | ||
always return True. | ||
This function searches for "pre#.#.#" in the filename of passed in files, | ||
where if found, it indicates that file should only be used if the current | ||
running blender version is below that version (non inclusive). The function | ||
then finds the highest versioned file and checks if this_file is that | ||
the same one. If so, it returns true, otherwise returns false. Most rigs | ||
do not have versioned names are are assumed to work with the latest blender | ||
version, hence they are treated as a "max_ver" to never get thrown out. | ||
|
||
Examples, presuming current blender is v2.93 | ||
for list ["rig pre2.8.0", "rig"] | ||
-> returns true for "rig" | ||
for list ["rig pre2.8.0", "rig pre3.0.0", "rig"] | ||
-> returns true for "rig pre3.0.0" | ||
|
||
""" | ||
basename = os.path.splitext(this_file)[0] | ||
max_ver = (99, 99, 99) # Standin for an impossibly high blender version. | ||
|
||
# Regex code to any matches of pre#.#.# at very end of name. | ||
# (pre) for exact match, | ||
|
@@ -129,32 +137,59 @@ def tuple_from_match(match): | |
return tuple_ver | ||
|
||
matches = find_suffix.search(basename) | ||
base_match = basename | ||
|
||
# Exit early, or find the common base after splitting out " pre#.#.#.blend" | ||
if matches: | ||
tuple_ver = tuple_from_match(matches) | ||
res = util.min_bv(tuple_ver) | ||
# If the suffix = 3.0 and we are BELOW 3.0, return True (use this file) | ||
return not res | ||
base_match = re.split(code, basename)[0].strip() | ||
this_ver = tuple_from_match(matches) | ||
res = util.min_bv(this_ver, inclusive=True) | ||
if res is True: | ||
# E.g. rig pre3.0.0 in blender 3.0.1 or 3.0.0, so file ineligible. | ||
return False | ||
else: | ||
# ie rig pre3.0.0 with current blender 2.9, so this is eligible - | ||
# but another file might still be a better match. | ||
pass | ||
else: | ||
this_ver = max_ver | ||
|
||
# Remaining case: no suffix found in this target, but see if any other | ||
# files have the same base and *do* have a suffix indicator. | ||
# Iterate over all files, structure of: | ||
# List of list[filepath, tuple detected] | ||
other_eligible = [] | ||
for afile in all_files: | ||
if not afile.startswith(basename): | ||
continue | ||
if afile == this_file: | ||
continue # Already checked above. | ||
if not afile.startswith(base_match): | ||
continue # Not part of the same base name, skip it | ||
|
||
base_afile = os.path.splitext(afile)[0] | ||
matches = find_suffix.search(base_afile) | ||
if matches: | ||
tuple_ver = tuple_from_match(matches) | ||
res = util.min_bv(tuple_ver) | ||
# If the suffix = 3.0 in `afile` file, and current blender is | ||
# below 3, then `afile` is the file that should be loaded, and the | ||
# current file `this_file` is to be skipped. | ||
return res | ||
|
||
# If no matches (the most common case), then this file is eligible. | ||
return True | ||
res = util.min_bv(tuple_ver, inclusive=True) | ||
if res is False: | ||
# E.g. rig pre3.0.0 in blender 2.9 is an eligible choice | ||
other_eligible.append([tuple_ver, afile]) | ||
else: | ||
# This would be the typical case of a "latest, non versioned" file. | ||
# E.g. rig.blend with no "pre3.3.0" text, also a eligible choice. | ||
# Give an artificial version to compare against for next step. | ||
other_eligible.append([max_ver, afile]) | ||
|
||
if len(other_eligible) == 1: | ||
# Only this_file is eligible, and thus must be "the one". | ||
return True | ||
|
||
# If multiple files, then we should use the one that is the largest version | ||
# without being equal to or greater than the current running blender ver. | ||
sorted_eligible = sorted(other_eligible, key=lambda x: x[0]) | ||
latest_allowed = None | ||
for tple, afile in sorted_eligible: | ||
if util.min_bv(tple, inclusive=True) is False: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing here |
||
latest_allowed = afile | ||
else: | ||
break | ||
|
||
return latest_allowed != this_file | ||
|
||
|
||
def attemptScriptLoad(path): | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inclusive
is set toTrue
by defaultThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, I spent a lot of time starting and it and it ended up helping me read it having the argument specifically there. I nearly just removed the function call in favor of bpy.app.version to make it easier to read, but we can do that in the 3.5 release (even though bpy.app.version is safely available in 2.79, no reason to break the compatibility lower just for this).