EffectEngine: Refactoring, stabalizing and fixes

[Lord-Grey]

Summary

Follow-up to #2010, which fixed a double-DECREF after a failed PyList_SetItem call.
A broader audit of the effectengine related code was refactored, stabilized and fixed.

What kind of change does this PR introduce?

• Bugfix
• Code style update
• Refactor

Does this PR introduce a breaking change?

• No

The PR fulfills these requirements:

• When resolving a specific issue, it's referenced in the PR's body (e.g. Fixes: #xxx[,#xxx], where "xxx" is the issue number)

PLEASE DON'T FORGET TO ADD YOUR CHANGES TO CHANGELOG.MD

• Yes, CHANGELOG.md is also updated

---

Related issue

Follows up on #2009 / #2010.

Description

A broader audit of the effectengine Python C-API usage after #2010 found additional reference-counting and null-pointer bugs of the same class.

Changes

Bug fixes (EffectModule.cpp, Effect.cpp)

• json2python: Missing null checks after PyList_New / PyDict_New — null-pointer dereference on allocation failure. Added null guards and corrected cleanup paths.
• setModuleParameters: PyModule_AddObject version-dependent double-DECREF on Python < 3.10. Split capsule-creation and add-failure paths; guard Py_DECREF(capsule) with #if PY_VERSION_HEX >= 0x030A0000.
• setModuleParameters: json2python null return passed directly to PyObject_SetAttrString without a null check → undefined behaviour. Added null guard.
• Wrapper functions (wrap*): All wrappers returned nullptr silently on argument-parse failure without setting a Python exception, causing SystemError in the interpreter. Added PyErr_SetString before each return nullptr.
• wrapImageShow: Out-of-range image ID raised TypeError instead of IndexError. Corrected exception type.
• wrapGetImage: QImageReader::imageCount() returns 0 for non-animated formats (JPEG, PNG). The result was allocated as a zero-element list, silently dropping the image. Now treated as 1 frame with a qMax(count, 1) fallback.
• setupImageSource: file:// URLs passed to QImageReader::setFileName as raw URL strings instead of local paths. Fixed using QUrl::isLocalFile() / url.toLocalFile().

Security fixes (EffectFileHandler.cpp)

• resolveEffectFilePath: Effect name used to construct the JSON file path without sanitising path separators or .. segments — path traversal allowing writes outside the effects directory.
• saveEffectImage: args["file"] appended to the effects directory path without validation — path traversal allowing arbitrary file writes. Fixed by verifying the resolved absolute path stays within the effects directory.

New effect

• Juggler: New built-in effect — coloured balls juggled across the LED image canvas.

Refactoring & improvements

• EffectDefinition: Moved Q_DECLARE_LOGGING_CATEGORY(effect) to EffectDefinition.h; added EffectDefinition.cpp with Q_LOGGING_CATEGORY(effect, "hyperion.effect"). Added qCDebug(effect) traces throughout EffectEngine, EffectModule, and EffectFileHandler. Debug logging is opt-in via qtlogging.ini (#hyperion.effect.debug=true).

• EffectModule.cpp: Extracted setupImageSource and imageToRGB helpers to reduce cognitive complexity and nesting depth in wrapGetImage.

• EffectFileHandler.cpp: Split saveEffect into resolveEffectFilePath, saveEffectImage, and cleanImageSource for clarity. Converted string concatenation in error messages to QString::arg() for safety and readability.

• Effect scripts: Minor stability and style fixes in pacman.py, traces.py, and trails.py.

• Empty image: 0×0 is now the canonical empty image; 1×1 is no longer treated as empty.

• ImageResampler: I422 Format - Consistently use adjusted local cropLeft computed earlier

[hyperion-project]

Hello @Lord-Grey 👋

I'm the Hyperion Project Bot and I want to thank you for
contributing to Hyperion with your pull requests!

To help you and other users test your pull requests faster,
I'll create a link for you to your workflow artifacts.

🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25624002367

Of course, if you make changes to your PR, I will create a new link.

Best regards,
Hyperion Project

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes additional Python C-API refcounting/null-pointer issues in EffectEngine discovered during a broader audit following #2010, and documents the fix in the changelog.

Changes:

• Add missing null checks after PyList_New() / PyDict_New() and adjust related cleanup paths.
• Make PyModule_AddObject() failure cleanup version-aware to avoid double-DECREF on Python < 3.10.
• Guard PyObject_SetAttrString() against json2python() returning nullptr, and update CHANGELOG.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
libsrc/effectengine/EffectModule.cpp	Adds allocation null checks and corrects ref-stealing assumptions around PyList_SetItem() usage.
libsrc/effectengine/Effect.cpp	Splits capsule creation vs add failure handling and adds a nullptr guard before PyObject_SetAttrString().
CHANGELOG.md	Notes the EffectEngine Python C-API refcounting/null-guard fixes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25624457405

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25625968867

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25625995435

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

libsrc/effectengine/EffectModule.cpp:1155

• wrapImageCOffset() ignores both invalid argument counts and PyArg_ParseTuple failure: for argCount != 2 it silently applies a (0,0) translation and returns None, and if parsing fails it may return None with a pending exception. Align this with other wrappers by validating argCount == 2 && PyArg_ParseTuple(...) and setting TypeError (or propagating the parse error) when arguments are invalid.

	int offsetX = 0;
	int offsetY = 0;
	Py_ssize_t argCount = PyTuple_Size(args);

	if (argCount == 2)
	{
		PyArg_ParseTuple(args, "ii", &offsetX, &offsetY);
	}

	getEffect()->_painter->translate(QPoint(offsetX, offsetY));
	Py_RETURN_NONE;
}

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25626252995

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25626459457

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25627030902

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25691698956

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25693173581

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25693680795

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25693690845

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25693699921

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25693710754

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/25694197797

[hyperion-project]

Hey @Lord-Grey I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/26336575448