Plugin system, layers refactor, workshop plugin refactor. Refactor of a lot of things. - #44
Merged
Conversation
…o plugin - CoreContext stripped of rail_views/add_rail_view (moved to AppContext) - Plugin base gains dependencies list; register() no-op; setup_all() toposorts - SteamCmdService paths computed live from ctx.config (no set_prefix/_derive_paths) - SteamCmdProgress/ItemStatus/Result types moved to download_proto - save_config deferred until after successful SteamCMD install - AppContext: owns UI PluginRegistry, rail_views, ui_prefs; sync setup_all/async init_all - Steam Workshop moved into ui/plugins/steam_workshop/ (self-contained package) - BaseViewPanel drops ui_prefs param; reads from app_ctx.ui_prefs - MainWindow drops ui_prefs param; reads from app_ctx - Tests updated: no set_prefix tests, config-based path tests
… scheme registration
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 426 |
| Duplication | -4 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
While the PR successfully implements a formal plugin system and separates Core/UI layers, the current implementation is not up to standards due to critical gaps in the plugin lifecycle and missing test coverage for the core registry logic. Codacy analysis indicates that coverage requirements are missing, and a significant complexity increase (+246) was detected in the Steam Workshop injection script.
A primary concern is the initialization order in _app.py, which creates a conflict where plugins lack access to mod data during their setup phase, while swapping the order risks UI plugins missing early lifecycle signals. Additionally, the core plugin sorting logic lacks unit tests, and the main entry point still contains hardcoded plugin references, partially undermining the goal of a fully pluggable architecture.
About this PR
- The core infrastructure for the plugin system (topological sorting and registry lifecycle) in 'core/plugin.py' currently lacks dedicated unit tests. Given this is the foundation for all future extensibility, coverage here is critical.
- The application entry point in '_app.py' still contains hardcoded logic/imports for specific plugins ('steamcmd', 'steamworkshop'), which partially contradicts the goal of a completely independent/pluggable architecture.
Test suggestions
- Found: Verify AppContext correctly initializes and manages rail views independently of CoreContext.
- Found: Verify SteamCmdService still operates correctly as a registered plugin.
- Missing: Verify PluginRegistry correctly sorts plugins by dependencies during setup_all using a complex graph.
- Missing: Verify PX_DISABLED_PLUGINS environment variable correctly prevents specified plugins from loading.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing: Verify PluginRegistry correctly sorts plugins by dependencies during setup_all using a complex graph.
2. Missing: Verify PX_DISABLED_PLUGINS environment variable correctly prevents specified plugins from loading.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
…sing - Extract _parse_disabled_plugins() helper from App._setup() - Add tests/TestPluginRegistryToposort: complex DAG, missing dep, extra_deps, init_all order - Add tests/TestParseDisabledPlugins: empty, single, multiple, whitespace - Fix test_persists_prefix patch target (steam_cmd_service -> core.config)
… fail On Windows the fake_steamcmd fixture creates a .bat script, but service.executable resolves to steamcmd.exe. Copying batch content into a .exe file causes ERROR_BAD_EXE_FORMAT because CreateProcess expects a PE binary. The worker catches the OSError and emits empty succeeded/failed lists. Fix: copy to .cmd (executable by CreateProcess via cmd.exe) and mock the executable property to return the .cmd path.
Owner
Author
|
Blocked until the QT bug is resolved. |
The Codacy CLI generates a results.sarif with one run per tool. github/codeql-action/upload-sarif@v4 rejected it because all runs shared the same category. Split into per-tool files and upload the directory so each run gets a unique auto-detected category.
Owner
Author
|
Found workaround. QWebChannel is messy, but works fine. |
PyXiion
changed the title
Replace global config_dir() calls with ConfigService injected via constructor DI. Tests provide a tmp_path-backed ConfigService so no test touches the real ~/.config/pxmodrim. Changes: - New generic ConfigService in core/config.py with typed load/save using msgspec.Struct (FileNotFoundError and corrupt-data handled gracefully) - CoreContext stores ConfigService, forwards to DiagnosticsService - CommunityRulesService, NoVersionWarningService, UseThisInsteadService now take ConfigService via constructor (previously zero-param) - StartupImpactService reads config_dir from ctx.config_service - SteamCmdService uses ctx.config_service instead of config_dir() - ui/config.py delegates to ConfigService (no more direct Path logic) - tests/conftest.py provides config_service fixture (tmp_path-based) - main_window.py and mod_info_panel.py pass config_service to load/save_ui_prefs - check-deps.py allows core.sort and core.checker to import core.config
…sService ModItem no longer stores diagnostic view state (has_error, has_warning, error_tooltip, warning_tooltip). ModListModel queries DiagnosticsService via summary_for(uuid) in data() instead, making diagnostic state independent of load_mods() timing. - DiagnosticsService: cache _last_summary, add summary_for() API - ModListModel: remove diagnostic fields from ModItem, pull from service - ModListPanel: bridge active_mods_changed correctly, skip commitOrder when only the checked set changes (not order)
…ates - Move steam workshop injection from action_handler.py to bridge_obj.py - Add TypeScript sources for steam workshop webview under ts/ - Add build-inject-js.py script with mtime-based caching - Update SteamWorkshop.qml, view.py, plugin.py for new bridge - Update AGENTS.md with build-js workflow and CI matrix details - Add rimworld-utils build deps to justfile - Remove action_handler.py (replaced by bridge_obj.py) - Update tests for plugin interface changes
Merge main's benchmarks & optimisations (PR #45) into feat/plugins, resolving conflicts in main_window.py and justfile: - main_window: keep HEAD's simple _auto_sort wrapper around CoreContext.auto_sort() over incoming UI-level duplication - justfile: keep HEAD's build-js recipe + variadic build args, add bench recipe from main - context: log sort time in ms instead of seconds - packaging/build: add --nofollow-import-to=pytest,pygments
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Resolves #28. Also refactors code
Extracting Steam integration into a completely independent plugin.