Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions cmd/upbrr/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,8 @@ func runCLIScreenshotHandling(ctx context.Context, coreSvc api.Core, req api.Req
}

finalImages := mergeScreenshotImages(nil, plan.FinalSelections)
if len(plan.SuggestedSelections) == 0 {
pending := pendingScreenshotSelections(plan)
if len(pending) == 0 {
if len(finalImages) > 0 {
return nil
}
Expand All @@ -972,7 +973,7 @@ func runCLIScreenshotHandling(ctx context.Context, coreSvc api.Core, req api.Req
return nil
}

result, err := coreSvc.GenerateScreenshots(ctx, req, plan.SuggestedSelections, api.ScreenshotPurposeFinal)
result, err := coreSvc.GenerateScreenshots(ctx, req, pending, api.ScreenshotPurposeFinal)
if err != nil {
return fmt.Errorf("upbrr: generate screenshots: %w", err)
}
Expand All @@ -992,6 +993,26 @@ func runCLIScreenshotHandling(ctx context.Context, coreSvc api.Core, req api.Req
return nil
}

// pendingScreenshotSelections drops baseline slots that already exist on disk.
// Plan reports the full baseline, so each final-capture path filters its own.
func pendingScreenshotSelections(plan api.ScreenshotPlan) []api.ScreenshotSelection {
if len(plan.ExistingScreenshots) == 0 {
return plan.SuggestedSelections
}
existing := make(map[int]struct{}, len(plan.ExistingScreenshots))
for _, image := range plan.ExistingScreenshots {
existing[image.Index] = struct{}{}
}
pending := make([]api.ScreenshotSelection, 0, len(plan.SuggestedSelections))
for _, selection := range plan.SuggestedSelections {
if _, ok := existing[selection.Index]; ok {
continue
}
pending = append(pending, selection)
}
return pending
}

func mergeScreenshotImages(base []api.ScreenshotImage, extra []api.ScreenshotImage) []api.ScreenshotImage {
if len(extra) == 0 {
return base
Expand Down
45 changes: 45 additions & 0 deletions cmd/upbrr/interactive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"maps"
"os"
"path/filepath"
"slices"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -2261,3 +2262,47 @@ func (c *cliCoreForTest) SaveDescriptionOverride(_ context.Context, req api.Requ
func (c *cliCoreForTest) Close() error {
return nil
}

func TestPendingScreenshotSelectionsSkipsExistingSlots(t *testing.T) {
t.Parallel()

plan := api.ScreenshotPlan{
SuggestedSelections: []api.ScreenshotSelection{
{Index: 0}, {Index: 1}, {Index: 2}, {Index: 3},
},
ExistingScreenshots: []api.ScreenshotImage{
{Index: 1}, {Index: 2},
},
}

indices := func(selections []api.ScreenshotSelection) []int {
out := make([]int, 0, len(selections))
for _, selection := range selections {
out = append(out, selection.Index)
}
return out
}

if got := indices(pendingScreenshotSelections(plan)); !slices.Equal(got, []int{0, 3}) {
t.Fatalf("expected only slots 0 and 3 pending, got %v", got)
}

plan.ExistingScreenshots = nil
if got := indices(pendingScreenshotSelections(plan)); !slices.Equal(got, []int{0, 1, 2, 3}) {
t.Fatalf("expected all 4 slots pending when nothing exists, got %v", got)
}

// Every slot captured: the CLI must find nothing to do rather than reshoot.
plan.ExistingScreenshots = []api.ScreenshotImage{{Index: 0}, {Index: 1}, {Index: 2}, {Index: 3}}
if got := pendingScreenshotSelections(plan); len(got) != 0 {
t.Fatalf("expected no pending slots when every slot is captured, got %v", indices(got))
}

// Plan already excludes captures that drifted off their slot, so a stale file
// never reaches ExistingScreenshots and its slot stays pending.
plan.SuggestedSelections = []api.ScreenshotSelection{{Index: 0}, {Index: 1}}
plan.ExistingScreenshots = []api.ScreenshotImage{{Index: 0}}
if got := indices(pendingScreenshotSelections(plan)); !slices.Equal(got, []int{1}) {
t.Fatalf("expected the uncaptured slot pending, got %v", got)
}
}
121 changes: 77 additions & 44 deletions gui/frontend/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ import {
import { hasFetchedExternalPreviewData } from "./utils/externalPreview";
import { handleExternalLinkClick } from "./utils/externalLinks";
import { normalizeJobStatus } from "./utils/jobStatus";
import {
buildExistingSlotImages,
pendingSelections,
selectionTimestamp,
staleSlotPaths,
} from "./utils/screenshotSlots";
import { isMetadataProgressPathMatch } from "./utils/metadataProgress";
import { dupeSkipReason, isRuleSkippedResult } from "./utils/dupeCheck";
import {
Expand Down Expand Up @@ -1797,17 +1803,42 @@ export default function App() {
);

// Helper functions for screenshot management (not in the hook)
const handleDeleteExistingImage = (image: ScreenshotImage) => {
const handleDeleteExistingImage = async (image: ScreenshotImage) => {
if (!image.Path) return;
if (!globalThis.confirm("Delete this image from the temp folder?")) {
return;
}
const deletedPath = image.Path;
// Only prune local state once the file is actually gone, or it returns on reload.
if (!(await screenshots.deleteScreenshotFile(deletedPath))) {
return;
}
screenshots.setExistingImages((prev) =>
prev.filter((entry) => entry.image.Path !== deletedPath),
);
screenshots.setExistingTrackerImages((prev) =>
prev.filter((entry) => entry.image.Path !== deletedPath),
);
screenshots.setPreviewImages((prev) =>
prev.filter((entry) => entry.image.Path !== deletedPath),
);
if (screenshots.finalImagesRef.current.length > 0) {
screenshots.saveFinalSelections(
await screenshots.saveFinalSelections(
screenshots.finalImagesRef.current.filter((entry) => entry.image.Path !== deletedPath),
);
}
screenshots.setScreenshotPlan((prev) => {
if (!prev) return prev;
return {
...prev,
ExistingScreenshots: (prev.ExistingScreenshots || []).filter(
(entry) => entry.Path !== deletedPath,
),
ExistingTrackerScreenshots: (prev.ExistingTrackerScreenshots || []).filter(
(entry) => entry.Path !== deletedPath,
),
};
});
};

const mergeFinalSelections = (
Expand Down Expand Up @@ -1882,15 +1913,8 @@ export default function App() {
return { selections: nextSelections, targetIndex: resolvedIndex };
};

const normalizeSelectionTimestamp = (entry: ScreenshotSelection) => {
if (Number.isFinite(entry.TimestampSeconds) && entry.TimestampSeconds > 0) {
return entry.TimestampSeconds;
}
if (Number.isFinite(entry.Frame) && entry.Frame > 0) {
return entry.Frame / previewFrameRate;
}
return 0;
};
const normalizeSelectionTimestamp = (entry: ScreenshotSelection) =>
selectionTimestamp(entry, previewFrameRate);

const desiredScreenCount = () => {
if (
Expand Down Expand Up @@ -3034,21 +3058,11 @@ export default function App() {
});
const candidates = autoSelections.length > 0 ? autoSelections : baseSelections;

const resolveTimestamp = (entry: ScreenshotSelection) => {
if (Number.isFinite(entry.TimestampSeconds) && entry.TimestampSeconds > 0) {
return entry.TimestampSeconds;
}
if (Number.isFinite(entry.Frame) && entry.Frame > 0) {
return entry.Frame / previewFrameRate;
}
return 0;
};

const closest = candidates.reduce(
(best, entry) => {
const currentDiff = Math.abs(resolveTimestamp(entry) - timestamp);
const currentDiff = Math.abs(normalizeSelectionTimestamp(entry) - timestamp);
if (!best) return entry;
const bestDiff = Math.abs(resolveTimestamp(best) - timestamp);
const bestDiff = Math.abs(normalizeSelectionTimestamp(best) - timestamp);
if (currentDiff < bestDiff) return entry;
return best;
},
Expand Down Expand Up @@ -3077,7 +3091,7 @@ export default function App() {
const manualSelection = reindexed.selections.find((entry) => {
const source = (entry.Source || "").toLowerCase();
if (source !== "manual") return false;
const ts = resolveTimestamp(entry);
const ts = normalizeSelectionTimestamp(entry);
if (!Number.isFinite(ts) || ts <= 0) return false;
return Math.abs(ts - timestamp) <= tolerance;
});
Expand Down Expand Up @@ -3125,20 +3139,18 @@ export default function App() {
}
};

const buildExistingSelectionIndexSet = () => {
const indices = new Set<number>();
const addImages = (images: ScreenshotImage[] | undefined) => {
if (!images || images.length === 0) return;
images.forEach((image) => {
if (Number.isFinite(image.Index)) {
indices.add(image.Index);
}
});
};
addImages(screenshots.screenshotPlan?.ExistingScreenshots);
addImages(screenshots.existingImages.map((entry) => entry.image));
addImages(screenshots.finalImagesRef.current.map((entry) => entry.image));
return indices;
// Slot -> image currently filling it. A freshly loaded plan is passed in because
// setScreenshotPlan has not flushed yet.
const existingSlotImages = (plan?: ScreenshotPlan | null) => {
const finalPaths = new Set(
screenshots.finalImagesRef.current.map((entry) => entry.image.Path).filter(Boolean),
);
return buildExistingSlotImages(
(plan ?? screenshots.screenshotPlan)?.ExistingScreenshots,
screenshots.previewImages
.map((entry) => entry.image)
.filter((image) => image.Path && finalPaths.has(image.Path)),
);
};

const handleGenerateScreenshots = async () => {
Expand All @@ -3148,16 +3160,17 @@ export default function App() {
return;
}
let selections = screenshots.screenshotSelections;
let loadedPlan: ScreenshotPlan | null = null;
if (selections.length === 0) {
const plan = await screenshots.loadScreenshotPlan(false);
selections = plan?.SuggestedSelections || [];
loadedPlan = await screenshots.loadScreenshotPlan(false);
selections = loadedPlan?.SuggestedSelections || [];
}
if (selections.length === 0) {
screenshots.setScreenshotsError("No screenshot selections available.");
return;
}
const existingIndices = buildExistingSelectionIndexSet();
const filteredSelections = selections.filter((entry) => !existingIndices.has(entry.Index));
const existingSlots = existingSlotImages(loadedPlan);
const filteredSelections = pendingSelections(selections, existingSlots, previewFrameRate);
if (filteredSelections.length === 0) {
screenshots.setScreenshotsError("All requested screenshots already exist.");
return;
Expand All @@ -3167,9 +3180,29 @@ export default function App() {
const result = await runScreenshotCapture(filteredSelections, "final");
screenshots.setFinalResult(result);
const images = result.Images || [];

const capturedPaths = new Set(images.map((image) => image.Path));
const stalePaths = staleSlotPaths(images, existingSlots);
for (const stalePath of stalePaths) {
await screenshots.deleteScreenshotFile(stalePath);
}

const previews = await Promise.all(images.map(screenshots.readScreenshotImage));
const merged = mergeFinalSelections(screenshots.finalImagesRef.current, previews);
await screenshots.saveFinalSelections(merged);
const kept = screenshots.finalImagesRef.current.filter(
(entry) => !stalePaths.has(entry.image.Path),
);
await screenshots.saveFinalSelections(mergeFinalSelections(kept, previews));
screenshots.setPreviewImages((prev) =>
prev.filter((entry) => !stalePaths.has(entry.image.Path)),
);
// Record the captures so a repeat click skips them instead of re-shooting.
screenshots.setScreenshotPlan((prev) => {
if (!prev) return prev;
const retained = (prev.ExistingScreenshots || []).filter(
(entry) => !stalePaths.has(entry.Path) && !capturedPaths.has(entry.Path),
);
return { ...prev, ExistingScreenshots: [...retained, ...images] };
});
} catch (err) {
screenshots.setScreenshotsError(String(err));
} finally {
Expand Down
34 changes: 26 additions & 8 deletions gui/frontend/src/hooks/useScreenshots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ export const useScreenshots = ({

setExistingImages([]);
setExistingTrackerImages([]);
setPreviewImages([]);
// Keep previews still held by final selections; they carry the slot Index.
setPreviewImages((prev) =>
prev.filter((entry) =>
finalImagesRef.current.some((final) => final.image.Path === entry.image.Path),
),
);
setFinalResult(null);

// Load existing screenshot previews
Expand Down Expand Up @@ -439,11 +444,16 @@ export const useScreenshots = ({
[path, idOverrideState, releaseOverrideState],
);

const deleteTrackerImageFile = useCallback(
async (imagePath: string) => {
// Delete one screenshot file; true only when it is gone from disk.
const deleteScreenshotFile = useCallback(
async (imagePath: string): Promise<boolean> => {
const deleter = globalThis.go?.guiapp?.App?.DeleteScreenshot;
if (!deleter || !path.trim() || !imagePath) {
return;
if (!deleter) {
setScreenshotsError("Deleting screenshots is unavailable in this build.");
return false;
}
if (!path.trim() || !imagePath) {
return false;
}
try {
await deleter(
Expand All @@ -452,8 +462,10 @@ export const useScreenshots = ({
normalizeReleaseOverrides(releaseOverrideState?.overrides || {}),
imagePath,
);
return true;
} catch (err) {
setScreenshotsError(String(err));
return false;
}
},
[path, idOverrideState, releaseOverrideState],
Expand Down Expand Up @@ -564,7 +576,12 @@ export const useScreenshots = ({
if (deleted.length === 0) return;
const deletedPaths = new Set(deleted.map((image) => image.Path));
setPreviewImages((prev) => prev.filter((entry) => !deletedPaths.has(entry.image.Path)));
}, [previewImages, deleteImageSet]);
if (finalImagesRef.current.some((entry) => deletedPaths.has(entry.image.Path))) {
await saveFinalSelections(
finalImagesRef.current.filter((entry) => !deletedPaths.has(entry.image.Path)),
);
}
}, [previewImages, deleteImageSet, saveFinalSelections]);

// Delete all final images
const handleDeleteAllFinalImages = useCallback(async () => {
Expand Down Expand Up @@ -620,7 +637,7 @@ export const useScreenshots = ({
const linked = trackerLinkByURL.get(url);
let removedFinal = false;
if (linked?.Path) {
await deleteTrackerImageFile(linked.Path);
await deleteScreenshotFile(linked.Path);
}
await deleteTrackerImageURL(url);
removeTrackerImageURLState(url);
Expand All @@ -634,7 +651,7 @@ export const useScreenshots = ({
removeTrackerImageURLState,
trackerLinkByURL,
removeFinalSelectionInternal,
deleteTrackerImageFile,
deleteScreenshotFile,
],
);

Expand Down Expand Up @@ -799,6 +816,7 @@ export const useScreenshots = ({
addFinalSelection,
removeFinalSelection,
reorderFinalSelections,
deleteScreenshotFile,
handleDeleteAllExistingImages,
handleDeleteAllTrackerImages,
handleDeleteAllPreviewImages,
Expand Down
2 changes: 1 addition & 1 deletion gui/frontend/src/pages/screenshots/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type Props = Readonly<{
handleDeleteAllExistingImages: () => void;
existingTrackerImages: ScreenshotPreviewImage[];
handleDeleteAllTrackerImages: () => void;
handleDeleteExistingImage: (image: ScreenshotImage) => void;
handleDeleteExistingImage: (image: ScreenshotImage) => Promise<void>;
showFrameSelections: boolean;
screenshotSelections: ScreenshotSelection[];
updateSelectionTime: (index: number, value: string) => void;
Expand Down
Loading
Loading