tech-debt: tiers 2+3 refactoring - #64
Open
sshcrack wants to merge 98 commits into
Open
Conversation
- Added virtual destructors to Scene, PostProcessingEffect, TransitionEffect, Post
- Rewrote PropertyMacros.h to use std::make_shared (avoids {} forwarding issue)
- Removed void(*)(T*) custom-deleter from unique_ptr return types in factory methods
- Converted all {new T(), [](T*){}} and unique_ptr<T, void(*)(T*)>(new T(), ...)
patterns to std::make_unique<T>()
- Fixed PixelJoint ScrapedPost internal types
- Fixed all create_scenes/create_effects/create_transitions/create_image_providers
factory methods across ~50 plugin files
- Removed unused deleteWrapper/deleteScene lambdas
sshcrack
commented
Jun 18, 2026
sshcrack
left a comment
sshcrack
left a comment
Owner
Author
There was a problem hiding this comment.
Code Review
This is a well-structured refactoring PR that consolidates duplicate code, modernizes C++ usage, improves CI/CD, and tightens security/safety. The extraction of shared color utilities, the apply_pixel_loop template, and the helper functions in scene_management.cpp all reduce code duplication meaningfully. The switch from rand() to <random>, from unique_ptr with custom deleters to make_unique/shared_ptr, and from file(GLOB_RECURSE) to explicit source lists are all good improvements.
I found one bug that will break the desktop app's ability to turn the matrix on/off during shutdown and window iconify.
…ath resolution - Add DEB packaging with debconf configuration, systemd service, and FHS layout - Rename binary from main to led-matrix for service ExecStart consistency - Fix matrix plugin loader to check PLUGIN_DIR env var, then dev plugins/ dir, then FHS path (/usr/lib/led-matrix/plugins/) - matches desktop loader - Set WorkingDirectory=/var/lib/led-matrix in systemd unit so images/ working dir resolves under data directory - Fix Shadertoy plugin custom_shader_dir() to use LED_MATRIX_DATA_DIR instead of get_exec_dir() /data/custom_shaders - Remove stale install rule from shared/matrix/CMakeLists.txt that duplicated the root CMakeLists.txt's install to wrong path - Fix Countdown/WeatherOverview plugin font install destinations - Remove dead countdown_font_dir constant with old path - Rewrite update_service.sh for dpkg -i workflow - Rewrite install_led_matrix.sh for .deb download + dpkg install - Delete obsolete scripts/release.ps1
Migrate to DEB packaging with FHS paths and fix plugin loader path resolution
…897→3 files) - WeatherScene.cpp split into WeatherScene.cpp (core dispatch), weather_rendering.cpp (cloud/lightning/fog/aurora/rainbow/utilities), weather_particles.cpp (rain/snow/shooting-stars), weather_cache.cpp (icon download/cache management) - SortingVisualizerScene.cpp split into SortingVisualizerScene.cpp (core+render), sorting_simple.cpp (bubble/selection/insertion/cocktail/gnome), sorting_complex.cpp (shell/quick/heap/comb/merge) - tier2: fix vcpkg_features.cmake JSON parsing (CMake 4.x MEMBER syntax) - fix: add missing consts.h include to update_routes.cpp, pass .get() for shared_ptr in server.cpp - fix: remove fs::create_directories side effect from GET /api/custom-assets/:type - fix: add /uploads/:filename route for uploaded file serving - fix: wire CollectionProvider upload to /pixeljoint/upload_img endpoint
…and docs - build_upload.sh: remove --delete flag, sync to /usr/ not root - canvas.cpp: add LED_MATRIX_SHARE_DIR fallback guard - desktop/main.cpp: update g_should_turn_off_on_exit on toggle, replace std::async with std::thread - PixelJoint ImageScene: convert remaining custom-deleter unique_ptrs - WeatherParser/WeatherScene: convert localtime() to localtime_r() - PropertyDemoScene: replace C-style casts with static_cast - AGENTS.md, README.md: update CI docs, binary name, DEB install instructions
…cate HTTP) - post_processing_routes.cpp: read params from POST body not query string - main.cpp: remove duplicate shutdown_matrix call and stale globals - CMakeLists.txt: remove undefined LIB_NAMES variable - data.h: remove dead from_json declarations for raw pointers
…Layout useMatch, CI glob, dead code
…60 factor without rescaling)
…tains(), dead code, missing locks, type safety
…tale [ ! -f ] guard)
…ardcode, sudo removal, emulator render, scene exclusion, dangling ref, TOCTOU, hue normalization, thread ordering, gitignore, preview_gen install, cmake version consistency
…web, packaging, and CI - Use-after-free: reorder cleanup in main.cpp (delete config after UpdateManager stop) - Update release asset filter from .tar.gz to .deb (update_routes.cpp) - Quote Spotify credentials in debconf-generated EnvironmentFile (postinst) - Guard dpkg -i with DEBIAN_FRONTEND=noninteractive (update_service.sh) - Make debconf config idempotent: skip prompts when answers already exist - Update config.json version field on upgrades via jq (postinst) - Consolidate db_go calls into single call at end (config) - Replace literal 'false' with '' in build-matrix.yml artifact paths - Add RestartSec=5 to systemd unit (led-matrix.service.in) - Guard vcpkg_features.cmake against empty features object - Use specific glob build/led-matrix-*.deb in build_upload.sh - Fix postrm to remove specific files instead of rm -rf whole dir - Fix FallingSandScene random direction bias (== 0 check) - Change showMainWindow to std::atomic<bool> for thread safety - Add try-catch around WebsocketClient::shared_from_this() (bad_weak_ptr) - Flush D-Bus connection after send in single_instance_manager - Disable noUnusedLocals/Parameters in tsconfig.json - Add try/catch + error toast to confirmUpload in AssetManager - Move ErrorBoundary to wrap Layout, add programmatic reset - Remove pre-existing unused imports and dead code references Fixes identified during comprehensive PR review with 4 parallel subagents covering build system, plugins, daemon/desktop/shared libs, and web frontend.
…ns, web, packaging, CI - build: fix CI JSON syntax, upload path literal false, install script DEB regex, vcpkg guards, DEBIAN_FRONTEND for automated updates - daemon: shutdown order (UpdateManager first), .tar.gz+.deb dual matching, POST body+query fallback, separate error_code vars, upload path traversal guard - desktop: restore atomic<bool> for showMainWindow, shared_from_this() try-catch, dbus_connection_flush, General mutex locking, waitpid -1 guard - shared: portable localtime_r/localtime_s for MSVC builds - plugins: PingPongGameScene backward-compat (revert defaults+60f mul), SnakeGameScene div-by-zero guard, WeatherScene div-by-zero guards, weather_particles render pos fix, particle init on empty vector - web: ErrorBoundary wraps Layout, setState reload, restore upload error handling, ESM-compatible __dirname - packaging: restore SPOTIFY_ID quotes, update config.json on upgrade, debconf only ask unset questions, postrm targeted purge
…ing, debconf purge, path fixes, misc
…data races, misc bugs
sshcrack
enabled auto-merge
August 7, 2026 05:50
|
Laptop MCP seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
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
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.
Implements medium and low priority refactoring from plans/tier2-medium.md and plans/tier3-low.md.