-
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 6 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,38 @@ | |
| /// </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( | ||
|
Check warning on line 45 in GenHub/GenHub.Core/Models/Workspace/ContentTypePriority.cs
|
||
| nameof(contentType), | ||
| contentType, | ||
| $"ContentType '{contentType}' is not mapped in {nameof(ContentTypePriority)}. " + | ||
|
Check warning on line 48 in GenHub/GenHub.Core/Models/Workspace/ContentTypePriority.cs
|
||
| "Add an explicit priority entry for this type.") | ||
|
greptile-apps[bot] marked this conversation as resolved.
Outdated
undead2146 marked this conversation as resolved.
Outdated
|
||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -288,6 +288,13 @@ | |
| 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 @@ | |
|
|
||
| // 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 | ||
|
|
@@ -495,13 +502,24 @@ | |
|
|
||
| // Create source.path mapping if we have a valid source directory | ||
| // This allows GetContentDirectoryAsync to resolve the content location | ||
| bool contentDirCreatedByThisCall = false; | ||
| bool sourcePathWrittenByThisCall = false; | ||
|
Check warning on line 506 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| string? previousSourcePathContent = null; | ||
| 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 +552,30 @@ | |
| { | ||
| 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) | ||
|
Check failure on line 557 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| { | ||
| // 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)) | ||
|
Check failure on line 563 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| { | ||
| // 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) | ||
|
Check failure on line 567 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| { | ||
| // Restore the previous content | ||
| await File.WriteAllTextAsync(sourcePathFile, previousSourcePathContent, CancellationToken.None); | ||
|
Check failure on line 570 in GenHub/GenHub/Features/Content/Services/ContentStorageService.cs
|
||
| } | ||
| 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.