feat(tools): integrate ModBuilder suite for mod project management and asset building - #376
feat(tools): integrate ModBuilder suite for mod project management and asset building#376undead2146 wants to merge 76 commits into
Conversation
f28d270 to
fd7f4c6
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
| IsBig = itemVm.IsBig, | ||
| BigSuffix = itemVm.BigSuffix, | ||
| SetGameLanguageOnInstall = itemVm.SetGameLanguageOnInstall, | ||
| Files = ParseItemFiles(itemVm, existingItem), |
There was a problem hiding this comment.
CRITICAL: ParseItemFiles silently drops every BundleFile property except AbsSourceFile on save
PopulateBundleItems always seeds SourcePattern (joined AbsSourceFile list or the default glob), so the existingItem?.Files branch here is effectively dead: every save rebuilds file entries as new BundleFile { AbsSourceFile = ... }, discarding AbsSourceParent, RelTargetFile, Params, ExcludeMarkersList, and RegistryDef. BuildEngineService reads file.RelTargetFile for build outputs and ConfigurationLoaderService depends on AbsSourceParent, so a load→save round-trip through this editor corrupts the project's target mappings and per-file conversion parameters. Preserve existingItem.Files (or round-trip the full objects) instead of re-parsing the pattern string.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| var files = new List<BundleFile>(); | ||
| if (!string.IsNullOrWhiteSpace(itemVm.SourcePattern)) | ||
| { | ||
| var patterns = itemVm.SourcePattern.Split([';', ','], StringSplitOptions.RemoveEmptyEntries); |
There was a problem hiding this comment.
WARNING: Pattern is joined with "; " but split on both ';' and ','
Absolute paths legitimately containing commas (e.g. C:\My Mods, WIP\file.tga) get truncated mid-path and produce corrupted BundleFile entries on save. Join and split must use the exact same separator set, and a separator that cannot appear in paths is safer for this round-trip encoding.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return; | ||
| } | ||
|
|
||
| BundleItems.Remove(SelectedBundleItem); |
There was a problem hiding this comment.
SUGGESTION: PackItemSelections is not refreshed after removing a bundle item
The checklist for the currently selected pack still shows the removed item, and re-checking it re-adds the dangling name to SelectedBundlePack.ItemNames, which then gets persisted. Call UpdatePackItemSelections() here (mirroring AddBundleItem).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return ConvertPsdToStandardImage(sourcePath, targetPath, targetExt, parameters); | ||
| } | ||
|
|
||
| using var image = await Image.LoadAsync(sourcePath, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
WARNING: CPU-bound conversion is no longer offloaded to the thread pool
The previous implementation wrapped decode + ApplyResizeParameters (including the full channel-split ResizeRgbaChannelsSeparately pass) in Task.Run. Now the heavy decode/resize runs on the caller's thread; when ConvertImageAsync is invoked from a UI context this blocks the UI thread for the duration of the resize. Keep the Task.Run offload (as ImageConversionService still does) or document the thread-context requirement.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ms.Position = 0; | ||
| using var loaded = Image.Load(ms); | ||
| var resized = ImageProcessingHelper.ApplyResizeParameters(loaded, parameters); | ||
| ImageProcessingHelper.SaveImageToTargetAsync(resized, targetPath, targetExt).GetAwaiter().GetResult(); |
There was a problem hiding this comment.
WARNING: Sync-over-async blocking on SaveImageToTargetAsync(...).GetAwaiter().GetResult()
This refactor introduces blocking waits inside an otherwise async pipeline (repeated at line 529). Make ConvertPsdToStandardImage async and await the save; blocking on async I/O burns a thread-pool thread and is deadlock-prone if a synchronization context is ever introduced.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| FilesPerSecond = 0; | ||
|
|
||
| _stopwatch.Restart(); | ||
| _cancellationTokenSource?.Dispose(); |
There was a problem hiding this comment.
WARNING: Previous CancellationTokenSource is disposed without being cancelled first
If StartBuild is called while a prior build's UpdateElapsedTimeAsync loop is still running, that loop holds the old token: it never observes cancellation (disposed sources are not cancelled), keeps ticking alongside the new loop (duplicate elapsed-time updates), and its next Task.Delay(1000, token) registration on the disposed source throws ObjectDisposedException in a fire-and-forget task. Call Cancel() on the old source before disposing it.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| /// </summary> | ||
| internal static class ImageProcessingHelper | ||
| { | ||
| public static readonly Dictionary<string, ResamplingMode> ResamplingModes = new(StringComparer.OrdinalIgnoreCase) |
There was a problem hiding this comment.
SUGGESTION: ResamplingModes is a publicly mutable static Dictionary
Any caller can mutate this shared lookup table at runtime, corrupting resize behavior globally. Expose it as IReadOnlyDictionary<string, ResamplingMode> (backed by a private dictionary) or an ImmutableDictionary.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…d with theme tokens
…yout and fix update view crash
…s, and security hotspots - Extract shared ImageProcessingHelper for image resizing and channel manipulation, eliminating duplicate code in ImageConversionService and CrunchImageConversionService - Refactor StringTableConversionService to reuse IExternalToolService - Refactor ConfigEditorViewModel into focused helper methods to reduce cognitive complexity - Replace insecure temp file creation with Path.GetRandomFileName in BuildEngineService - Add suppression and justification for legacy C&C game MD5 checksums and switch FileManagerViewModel to SHA-256 - Add regex evaluation timeout in TextProcessingService - Implement IDisposable and cancellation token disposal in BuildProgressViewModel and propagate cancellation tokens - Fix automatic variable conflict in build-check.ps1 and conditional syntax in build-check.sh - Add missing assertions in BuildEngineServiceTests
…ts, and code smells - Delegate MD5 release checksum computation to Md5HashProvider and add GlobalSuppressions entry - Eliminate unreachable branches in ConfigEditorViewModel with FirstOrDefault - Mark BuildProgressViewModel sealed and introduce StageStatusPending constant - Refactor RunGameAsync into helper methods to reduce cognitive complexity
4d81211 to
8b61176
Compare
|
ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing |
…g, and add sample projects
…ate view namespaces
…t generator, and robust wildcard staging
…RecentProjectsAsync (S8949)
…ion key, and dashboard actions
…ntainability issues
…over sample on dashboard, and report accurate build progress
…over all sample projects, and ensure real-time build file counting
|
1 participant
Overview
This PR integrates ModBuilder into GenHub as a tool plugin suite for creating, configuring, converting, and packaging Command & Conquer Generals / Zero Hour mods and custom content.
In addition, this branch has been updated and synchronized with
upstream/development, resolving merge conflicts and adapting service dependencies to the latest architectural standards.Changes
1. ModBuilder Core Architecture & Services
IProjectConfigService):.mbprojproject files, project structure generation, recent projects history, and templates.IConfigurationProviderServiceto ensure storage location and user-relocated data directory conventions are honored.IBuildEngineService):ImageConversionService: TGA/PNG/DDS texture conversions, compression, mipmap generation, and format validation using Magick.NET.StringTableConversionService: CSF string table compiling, decompiling, encoding validation, and parsing.TextProcessingService&FileConversionService: INI and script processing, macro replacement, and format transformation.ArchiveService: BIG archive creation, packing, verification, and compression.ExternalToolService: Safe, throttled external process execution (crunch, gametextcompiler, blender) with cross-platform shell and binary invocation.BuildCacheService&FileHashRegistryService: Content hash tracking, incremental build cache validation, and cache invalidation.ModBuilderToolPlugin: ImplementsIToolPluginfor dynamic tool discovery in the GenHub tools registry.ModBuilderModule: Service registration module for core services, view models, and conversion providers.2. UI & ViewModels (Avalonia & MVVM)
ModBuilderView: Main host view with project dashboard, file explorer, pack editor, and settings.ProjectDashboardView: Project overview, metrics, quick actions, and recent projects launcher.FileManagerPanel&FileTreeItem: Hierarchical asset explorer with file type icons, status indicators, and context actions.BundlePackEditorDialog&ConfigEditorDialog: Visual editors for mod bundle items and pack configurations.BuildProgressOverlay,ProgressCard, andMetricDisplay: Interactive build progress visualization and live telemetry.ModBuilderViewModel,ProjectDashboardViewModel,FileManagerViewModel,BundlePackEditorViewModel,BundleItemEditorViewModel,ConfigEditorViewModel,BuildProgressViewModel,SettingsPanelViewModel.3. Upstream Synchronization & Conflict Resolution
developmentbranch changes intofeat/modbuilder.GenHub.slnto includeGenHub.MacOS,GenHub.Benchmarks,GenHub.Tests.MacOS, andGenHub.Tests.Performance.GameProfileSettingsViewModel.cs.ExternalToolServiceTestswith cross-platform script creation to execute cleanly across Windows, Linux, and macOS.Technical Details
Testing
ProjectConfigServiceTests: Project creation, saving, loading, recent projects tracking, and template initialization.ExternalToolServiceTests: Tool validation, argument passing, working directory execution, cancellation, and failure handling.Greptile Summary
The PR adds the ModBuilder tool suite, including project configuration, incremental build orchestration, conversions, external tools, archive packaging, plugin registration, Avalonia UI, and extensive tests and documentation. The build and packaging paths currently contain failure-state, cached-state, cancellation, and artifact-replacement defects that should be corrected before merge.
Confidence Score: 1/5
The PR is not safe to merge until failed assets correctly fail builds, per-run steps stop leaking through cached state, cancellation terminates child tools, and archive replacement preserves valid output on failure.
The new build pipeline can report incomplete output as successful, execute stages retained from previous runs, leave cancelled external processes running, and destroy the last valid archive during a failed replacement.
Files Needing Attention: GenHub/GenHub/Features/Tools/ModBuilder/Services/BuildEngineService.cs, GenHub/GenHub/Features/Tools/ModBuilder/Services/ExternalToolService.cs, GenHub/GenHub/Features/Tools/ModBuilder/Services/ArchiveService.cs
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD UI[ModBuilder UI] --> Project[Project configuration] UI --> Engine[Build engine] Project --> Setup[Cached build structure] Setup --> Engine Engine --> Pre[Pre-build] Engine --> Clean[Clean] Engine --> Assets[Asset conversion] Assets --> Images[Image conversion] Assets --> Strings[String-table conversion] Assets --> Tools[External tools] Engine --> Archives[Bundle and release archives] Engine --> Install[Install and run] Engine --> Result[Build result and progress] Result --> UIPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "test(modbuilder): align parallel benchma..." | Re-trigger Greptile
Context used:
Note
Add ModBuilder suite for mod project management and asset building
BuildEngineService(pipeline orchestration),ArchiveService(BIG/ZIP creation),BuildCacheService(MD5-based incremental builds),ImageConversionService(PSD/TGA/DDS with alpha detection),FileConversionService(routing by file type),ExternalToolService(process pool execution), and several supporting services.ModBuilderToolPluginvia a newModBuilderModuleDI extension called fromAppServices.ModBuilderView,ProjectDashboardView,FileManagerPanel,SettingsPanel,BuildProgressOverlay,ConfigEditorDialog,BundlePackEditorDialog, and reusable controls (BuildLogEntry,FileTreeItem,ProgressCard,MetricDisplay) with a dark-themed design system and custom converters.DefaultFileBufferSizeinIoConstantschanges from 4096 to 65536, affecting all file operations that rely on this constant; compiled bindings are also disabled globally (AvaloniaUseCompiledBindingsByDefault=false).Macroscope summarized e0a6c14.