SG-40114 [Temp Workaround] Add compatibility with Houdini 21.0+ #88
Conversation
Workaround for odd behavior in Houdini 21.0+ not seeing the menu file if created by a startup script
Remove old code handling unsupported Houdini that was targetting Qt5 and was not Qt6-ready. Was causing a AttributeError: function 'PyCObject_AsVoidPtr' not found
…h21.0' into ticket/SG-40114-temp-workaround-houdini-21.0
…or-h21.0' into ticket/SG-40114-temp-workaround-houdini-21.0
There was a problem hiding this comment.
Pull Request Overview
This PR implements a temporary workaround to address compatibility issues with Houdini 21.0+. The changes focus on fixing menu functionality and resolving Python API compatibility problems.
Key changes:
- Adds pre-launch menu construction for Houdini 21.0+ compatibility
- Removes deprecated PyCObject_AsVoidPtr usage that causes errors in newer Python versions
- Simplifies Windows dialog handling to work with modern Houdini versions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| startup.py | Adds conditional menu generation logic for Houdini 21.0+ before application launch |
| engine.py | Removes deprecated PyCObject_AsVoidPtr API calls and simplifies Windows dialog handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if self.get_setting("use_short_menu_name", False): | ||
| mock_engine._menu_name = "FPTR" | ||
|
|
||
| import tk_houdini |
There was a problem hiding this comment.
The import statement should be moved to the top of the file with other imports to follow Python best practices and improve code readability.
| pass | ||
|
|
||
| mock_engine = MockEngine() | ||
| mock_engine.logger = self.logger | ||
|
|
||
| mock_engine._menu_name = "Flow Production Tracking" | ||
| if self.get_setting("use_short_menu_name", False): | ||
| mock_engine._menu_name = "FPTR" |
There was a problem hiding this comment.
Creating a mock engine class inline with dynamic attribute assignment is fragile. Consider defining a proper MockEngine class with init method or using a more structured approach like a namedtuple or dataclass.
| pass | |
| mock_engine = MockEngine() | |
| mock_engine.logger = self.logger | |
| mock_engine._menu_name = "Flow Production Tracking" | |
| if self.get_setting("use_short_menu_name", False): | |
| mock_engine._menu_name = "FPTR" | |
| def __init__(self, logger, menu_name): | |
| self.logger = logger | |
| self._menu_name = menu_name | |
| menu_name = "Flow Production Tracking" | |
| if self.get_setting("use_short_menu_name", False): | |
| menu_name = "FPTR" | |
| mock_engine = MockEngine(self.logger, menu_name) |
| ctypes.windll.user32.SetActiveWindow(hwnd) | ||
|
|
There was a problem hiding this comment.
The code assumes dialog.winId() returns a valid Windows handle without validation. This could fail if winId() returns None or an invalid handle, potentially causing ctypes.windll.user32.SetActiveWindow to fail silently or raise an exception.
| ctypes.windll.user32.SetActiveWindow(hwnd) | |
| # Validate hwnd before using it | |
| if hwnd is not None and int(hwnd) != 0: | |
| ctypes.windll.user32.SetActiveWindow(hwnd) | |
| else: | |
| self.logger.warning( | |
| "Could not activate dialog window: invalid winId() returned ({})".format(hwnd) | |
| ) |
This PR is a temporary workaround trying to make the code compatible with Houdini 21.0+.
Important
This PR is only a workaround and we don't intend to merge it as is.
It is not designed to be used for production.
This PR is collection of merges of other PRs:
'PyCObject_AsVoidPtr' not founderror