-
Notifications
You must be signed in to change notification settings - Fork 20
fix: reorganize settings UI, fix content priority resolution, and eliminate CAS warnings (#281, #282, #283, #284) #285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Changes from all commits
f7f3a83
077e351
8a2606b
2c3c1c3
e84abd8
5ab0a40
151027f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| using System; | ||
| using GenHub.Core.Models.Enums; | ||
|
|
||
| namespace GenHub.Core.Models.Workspace; | ||
|
|
@@ -14,16 +15,34 @@ public static class ContentTypePriority | |
| /// </summary> | ||
| /// <param name="contentType">The content type.</param> | ||
| /// <returns>Priority value (0-100).</returns> | ||
| /// <exception cref="ArgumentException">Thrown when the content type should not be in a workspace.</exception> | ||
| public static int GetPriority(ContentType contentType) | ||
| { | ||
| return contentType switch | ||
| { | ||
| ContentType.Mod => 100, // Highest: User mods override everything | ||
| ContentType.Patch => 90, // Patches override base content | ||
| ContentType.GameClient => 50, // Community executables override official | ||
| ContentType.ModdingTool => 45, // Modding tools (between Addon and GameClient) | ||
| ContentType.Executable => 45, // Executables (between Addon and GameClient) | ||
| ContentType.Addon => 40, // Addons (maps, etc.) | ||
| ContentType.LanguagePack => 35, // Language packs | ||
| ContentType.MapPack => 30, // Map packs (between GameInstallation and Addon) | ||
| ContentType.Map => 30, // Individual maps | ||
| ContentType.Mission => 30, // Missions | ||
| ContentType.Skin => 20, // Skins | ||
| ContentType.Video => 20, // Videos | ||
| ContentType.Replay => 20, // Replays | ||
| ContentType.Screensaver => 20, // Screensavers | ||
| ContentType.GameInstallation => 10, // Lowest: Base game files | ||
| _ => 0, // Unknown/undefined types | ||
|
|
||
| // These types should not be in workspaces | ||
| ContentType.ContentBundle => throw new ArgumentException($"ContentType {contentType} should not be used in workspace priority resolution"), | ||
| ContentType.PublisherReferral => throw new ArgumentException($"ContentType {contentType} should not be used in workspace priority resolution"), | ||
| ContentType.ContentReferral => throw new ArgumentException($"ContentType {contentType} should not be used in workspace priority resolution"), | ||
| ContentType.UnknownContentType => throw new ArgumentException($"ContentType {contentType} should not be used in workspace priority resolution"), | ||
|
|
||
| _ => throw new ArgumentOutOfRangeException(nameof(contentType), contentType, $"ContentType '{contentType}' is not mapped in {nameof(ContentTypePriority)}. Add an explicit priority entry for this type."), | ||
|
Comment on lines
+40
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid throwing from the hot priority-resolution path.
Consider making these meta/unsupported types deterministic low priority (or introducing 🤖 Prompt for AI Agents |
||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -288,6 +288,13 @@ public async Task<OperationResult<ContentManifest>> StoreContentAsync( | |
| var manifestJson = JsonSerializer.Serialize(updatedManifest, JsonOptions); | ||
| await File.WriteAllTextAsync(manifestPath, manifestJson, cancellationToken); | ||
|
|
||
| // Create source.path marker for CAS-stored content | ||
| // This prevents "Could not resolve source path" warnings when GetContentDirectoryAsync is called | ||
| var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, updatedManifest.Id.Value); | ||
| Directory.CreateDirectory(contentDir); | ||
| var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName); | ||
| await File.WriteAllTextAsync(sourcePathFile, "CAS-ONLY", cancellationToken); | ||
|
|
||
|
Comment on lines
+291
to
+297
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t make marker-file write failure abort successful CAS storage.
Suggested containment- var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, updatedManifest.Id.Value);
- Directory.CreateDirectory(contentDir);
- var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName);
- await File.WriteAllTextAsync(sourcePathFile, "CAS-ONLY", cancellationToken);
+ try
+ {
+ var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, updatedManifest.Id.Value);
+ Directory.CreateDirectory(contentDir);
+ var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName);
+ await File.WriteAllTextAsync(sourcePathFile, "CAS-ONLY", cancellationToken);
+ }
+ catch (Exception markerEx)
+ {
+ _logger.LogWarning(markerEx, "Stored manifest {ManifestId} but failed to write CAS source-path marker", updatedManifest.Id);
+ }🤖 Prompt for AI Agents |
||
| _logger.LogInformation("Successfully stored content for manifest {ManifestId}", manifest.Id); | ||
| return OperationResult<ContentManifest>.CreateSuccess(updatedManifest); | ||
| } | ||
|
|
@@ -408,7 +415,7 @@ public async Task<OperationResult<bool>> RemoveContentAsync(ManifestId manifestI | |
|
|
||
| // Remove source.path mapping file if it exists | ||
| var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, manifestId.Value); | ||
| var sourcePathFile = Path.Combine(contentDir, "source.path"); | ||
| var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName); | ||
| FileOperationsService.DeleteFileIfExists(sourcePathFile); | ||
|
|
||
| // Clean up the data directory if empty | ||
|
|
@@ -473,6 +480,11 @@ private async Task<OperationResult<ContentManifest>> StoreManifestOnlyAsync( | |
| { | ||
| var manifestPath = GetManifestStoragePath(manifest.Id); | ||
|
|
||
| // Declare cleanup tracking variables outside try block so they're accessible in catch | ||
| bool contentDirCreatedByThisCall = false; | ||
| bool sourcePathWrittenByThisCall = false; | ||
| string? previousSourcePathContent = null; | ||
|
|
||
| try | ||
| { | ||
| // Validate manifest for security issues | ||
|
|
@@ -498,10 +510,18 @@ private async Task<OperationResult<ContentManifest>> StoreManifestOnlyAsync( | |
| if (!string.IsNullOrWhiteSpace(sourceDirectory) && Directory.Exists(sourceDirectory)) | ||
| { | ||
| var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, manifest.Id.Value); | ||
| bool dirAlreadyExisted = Directory.Exists(contentDir); | ||
| Directory.CreateDirectory(contentDir); | ||
| contentDirCreatedByThisCall = !dirAlreadyExisted; | ||
|
|
||
| var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName); | ||
|
|
||
| // Backup existing source.path content before overwriting | ||
| if (File.Exists(sourcePathFile)) | ||
| previousSourcePathContent = await File.ReadAllTextAsync(sourcePathFile, cancellationToken); | ||
|
|
||
| var sourcePathFile = Path.Combine(contentDir, "source.path"); | ||
| await File.WriteAllTextAsync(sourcePathFile, sourceDirectory, cancellationToken); | ||
| sourcePathWrittenByThisCall = true; | ||
|
|
||
| _logger.LogInformation( | ||
| "Created source path mapping for {ManifestId}: {SourcePath}", | ||
|
|
@@ -534,6 +554,30 @@ private async Task<OperationResult<ContentManifest>> StoreManifestOnlyAsync( | |
| { | ||
| FileOperationsService.DeleteFileIfExists(manifestPath); | ||
|
|
||
| // Clean up the content dir created for source.path mapping | ||
| var contentDir = Path.Combine(_storageRoot, DirectoryNames.Data, manifest.Id.Value); | ||
| if (contentDirCreatedByThisCall) | ||
| { | ||
| // We created this directory in the current call — safe to remove entirely. | ||
| if (Directory.Exists(contentDir)) | ||
| Directory.Delete(contentDir, recursive: true); | ||
| } | ||
| else if (sourcePathWrittenByThisCall && Directory.Exists(contentDir)) | ||
| { | ||
| // We overwrote an existing source.path file - restore it or delete if it was new | ||
| var sourcePathFile = Path.Combine(contentDir, FileTypes.SourcePathFileName); | ||
| if (previousSourcePathContent != null) | ||
| { | ||
| // Restore the previous content | ||
| await File.WriteAllTextAsync(sourcePathFile, previousSourcePathContent, CancellationToken.None); | ||
| } | ||
| else | ||
| { | ||
| // We created a new file, safe to delete | ||
| FileOperationsService.DeleteFileIfExists(sourcePathFile); | ||
| } | ||
| } | ||
|
undead2146 marked this conversation as resolved.
|
||
|
|
||
|
undead2146 marked this conversation as resolved.
|
||
| // Untrack manifest if we failed to save its metadata but had already tracked/refreshed references. | ||
| await _referenceTracker.UntrackManifestAsync(manifest.Id, CancellationToken.None); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.