6645 ue5 fixes (#10) #6
Conversation
Workarounds for: - Mac - Linux - U5 dev where Shotgun components were renamed to Shotgrid, without backward compatibility in mind.
Added back checking Unreal install in registry for Windows. Revisited the [original code](https://github.com/ue4plugins/tk-unreal/blob/6863ca47b12def732883e7418934ca6590576c9b/startup.py#L111) to make it easier to understand and maintain.
plugins/basic/bootstrap.py
Outdated
| plugin_root_dir = os.path.abspath(os.path.dirname(__file__)) | ||
| sys.path.insert(0, os.path.join(plugin_root_dir, "python")) | ||
|
|
||
| # Temp workaround for UE5 dev: for some reasons the core/python path is missing |
There was a problem hiding this comment.
I think I know why. In UE5, there were changes that enabled some global python interpreter flags. Py_IgnoreEnvironmentFlag and Py_IsolatedFlag. See docs here. This was done to help avoid py2 code from being loaded into py3 interpreter. There is now a UE_PYTHONPATH environment variable you can set to prep the environment for bootstrap. Let me know if that doesn't work. I'm asking around for docs on this change.
There was a problem hiding this comment.
Thanks for that, I'll dig!
| # Shotgun integration components were renamed to Shotgrid from UE5 | ||
| if hasattr(UESGEngine, "get_shotgrid_menu_items"): | ||
| @unreal.ufunction(override=True) | ||
| def get_shotgrid_menu_items(self): |
There was a problem hiding this comment.
Would it be sufficient to leave the old methods in here, calling the new instead? Maybe add a deprecation docstring or warning?
There was a problem hiding this comment.
I think it's good to support backward and forward compatibility. We could log a deprecation warning in "shotgun" methods, mentioning "shotgrid" methods, is it what you have in mind?
There was a problem hiding this comment.
Sorry, my suggestion was poorly written. What I meant to say was, would it be sufficient to leave all methods at the top level (no conditional) and have the old shotgun methods call the new shotgrid methods while also logging a warning when the old ones are called.
There was a problem hiding this comment.
Unfortunately not: the new methods are only available from UE5, we can’t call them in UE4. So the tests and mix and matches are needed.
Mostly addressed CR comments from Epic. Fixed a warning with yaml loading.
|
Please have another look, I think I addressed your comments as much as I could. |
| # open the yaml file and read the data | ||
| with open(plugin_info_yml, "r") as plugin_info_fh: | ||
| plugin_info = yaml.load(plugin_info_fh) | ||
| plugin_info = yaml.load(plugin_info_fh, yaml.SafeLoader) |
There was a problem hiding this comment.
I added this to get rid of a yaml warning, that was not in the first PR you reviewed.
| # UE5 Python ignores PYTHONPATH and only uses UE_PYTHONPATH | ||
| # blindly copy what was set so modules (e.g. SG TK core) are found | ||
| # when bootstrapping. | ||
| required_env["UE_PYTHONPATH"] = os.environ.get("PYTHONPATH") or "" |
There was a problem hiding this comment.
For the PYTHONPATH vs UE_ PYTHONPATH problem, there is nothing clever than I can do since the environment is not set by tk-unreal, so I blindly copy over everything, which ruins the purpose of having a new env. variable, but I would argue that having this new env. variable in the first place is may be not so wise....
Removed the add integration and wrote some doc instead.
Workarounds for:
Added back checking registry on Windows like it was done in the original code