diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..64b258a --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,141 @@ +# Working on PhotoSync + +A Fabric client mod that uploads Minecraft screenshots to Immich and browses the +album in game, built from one source tree for nine Minecraft versions +(1.20 → 26.2). Read [`README.md`](README.md) for what it does and +[`docs/PORTING.md`](docs/PORTING.md) before touching anything that imports +`net.minecraft`. + +This file is the orientation an agent needs before its first edit: where code +goes, what may import what, and which oddities are deliberate. + +## Commands + +```sh +./gradlew test # the shared modules' unit tests +./gradlew :platform:1.21.8:build # one bucket, remapped jar included +./gradlew buildAllPlatforms # all nine — the real check before finishing +./gradlew :platform:1.21.8:runClient # dev client for one bucket +``` + +`buildAllPlatforms` takes a few minutes and is the only thing that proves a +change compiles everywhere. A change that only touches `shared/` still has to +pass it, because each bucket compiles the shared sources at its own bytecode +level. If the machine has no network but a populated Gradle cache, add +`--offline`. + +Two probes answer "what does this method look like on every version", by running +`javap` across the cached Minecraft jars: + +```sh +python3 tools/probe-api.py # the fixed list of members the bridge depends on +python3 tools/probe-class.py net.minecraft.client.gui.GuiGraphics '(?i)blit' +``` + +Use them instead of recalling a signature. Every table in `docs/PORTING.md` was +produced this way, and the ones that were not were wrong. + +## The module map + +``` +shared/core no Minecraft, no GUI. Config, uploads, providers, timeline, + ThumbHash. Where behaviour that can be unit-tested lives. +shared/mc-api the seam: interfaces the game must implement, no bodies. +shared/ui every screen and widget, drawn through mc-api only. +shared/client wiring — screenshot becomes upload becomes notification. +platform/common a source root, not a project: the adapter code that is + textually identical on all nine buckets. +platform/ one bucket's adapters (dev.photosync.platform.impl) and mixins. +``` + +The dependency arrow points one way: `platform` → `client` → `ui` → `mc-api` → +`core`. Each module's `package-info.java` states its own rule and is worth +reading before adding a file to it. The two that get violated by accident: + +- **Nothing under `shared/` may import `net.minecraft`, `net.fabricmc` or + `org.lwjgl`.** If shared code needs something from the game, the answer is a + new method on an `mc-api` interface plus nine adapter implementations — never + an import. Adding to that seam is expensive on purpose, so first check whether + the thing can be computed from what `RenderBridge` already exposes. +- **`shared/` compiles to Java 17 bytecode** (`shared_java` in + `gradle.properties`), because 1.20–1.20.4 run on a Java 17 JVM. No virtual + threads, no Java 21 pattern matching, no `HttpClient.close()`. Platform modules + compile at their own bucket's level, which is why the same idiom can be legal + in `platform/26.2` and rejected in `shared/core`. + +## Where a change goes + +| You want to | Put it in | +| --- | --- | +| change upload, retry or queue behaviour | `shared/core/upload` | +| add a setting | `shared/core/config` (a record + `normalized()`), then `SettingsScreen` and `en_us.json` | +| support another photo service | `shared/core/provider/`, implementing `PhotoProvider` + `ProviderFactory`, added to the list in `PhotoSync`'s one-argument constructor. No UI or platform change needed — the settings screen builds its fields from `ProviderDescriptor` | +| change a screen | `shared/ui/screen` | +| add a drawing primitive | `RenderBridge` **and all nine** `RenderAdapter`s. Read `docs/PORTING.md` §3 first | +| fix something on one Minecraft version | that bucket's `platform//…/impl` | +| fix something on every Minecraft version | `platform/common` — but only if it compiles on all nine | + +The `platform/common` rule has no third case: a class lives there while all nine +compilations accept it, and the day one of them stops, it moves down into all +nine copies of `impl`. Do not add a version check to keep it in `common`. + +## Conventions + +The compiler does not enforce these; reviewers do. + +- **Lombok with fluent accessors.** `lombok.config` at the repo root sets + `lombok.accessors.fluent = true`, so a `@Getter` on `kind` generates `kind()`, + not `getKind()`. Records are the default for data; `@Builder(toBuilder = true)` + for config records. +- **No utility classes, no scattered constants.** A constant belongs to the class + that uses it, as a `private static final` next to that use. If a static helper + is tempting, it usually means the behaviour belongs on one of the objects. +- **No new dependencies.** Gson and SLF4J are `compileOnly` because Minecraft and + Fabric Loader already ship them; nothing is shaded and nothing is jar-in-jar'd, + which is why each jar is ~270 KB. Do not write a JSON parser, an HTTP client or + a base64 encoder — the platform has all three. +- **Comments explain why, not what.** The existing ones record measurements and + decisions ("Immich's default thumbnail format is WebP, which the game's decoder + cannot read"). Match that; a comment restating the code is noise. +- **Don't over-abstract.** One provider interface exists because a second provider + is a stated goal. An interface with one implementation and no second in sight is + not the house style. +- **User-visible strings are translation keys**, in + `shared/client/src/main/resources/assets/photosync/lang/en_us.json`. Adding keys + is safe on every version; renaming them is not. + +## Things that are the way they are on purpose + +Verify before "fixing" any of these — each one cost a measurement. + +- **No refmap.** Loom's non-legacy mixin remapping rewrites annotations in place. + There is no `refmap.json` and none is needed; `docs/PORTING.md` §6 shows how to + confirm it with `javap -p -v` on a built jar. +- **`CaptureMixin` targets `method = "*"`** because the private helper that writes + the screenshot has three different names across the range, and it redirects the + `File` overload rather than the `Path` one because the two delegate — redirecting + both publishes every capture twice. +- **26.x uses a generated 47-byte identity mappings jar** (root `build.gradle`), + because those Minecraft jars ship deobfuscated and Loom still demands a mappings + artifact. +- **The UI draws its own widgets.** Minecraft's drawing primitives have been stable + since 1.20; its widget constructors have not. +- **Immich renditions are a ladder, not a constant** (`ImmichProvider.thumbnail`): + the grid asks for `thumbnail` then `preview`, an opened photo asks for `fullsize` + then `preview`, and a rendition that answers with WebP, a 404 or a 403 is retired + for the session. Minecraft decodes PNG and JPEG only — stb_image reads neither + WebP nor AVIF — so a rendition's format is a correctness concern, not a + preference. +- **`ThumbnailCache` floors capacity at 1, not 8.** The grid's real floor comes + from `BrowserSettings.normalized()`; the detail cache wants two, because + full-resolution textures are tens of megabytes each. + +## Testing + +`shared/core` is the only module with meaningful test coverage, and it is thin — +`ThumbHashTest` is what exists. Anything you add to `core` that can be tested +without a game should come with tests; `ui` and `platform` are verified by +compiling all nine buckets and running a dev client. + +There is no test that catches a broken adapter. If you change anything under +`platform/`, run `runClient` on that bucket and look at the screen. diff --git a/README.md b/README.md index eb95057..5a0dac6 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,14 @@ for anyone who wants to watch them. photo website does: the per-day counts arrive before any image does, so the scrollbar is honest about the album's real size from the first frame, and the thumbnails for a day are fetched only when you scroll near them. Click a photo to -open it larger, Escape to go back. It is read-only — PhotoSync will not delete, -move or favourite anything on your server. +open it, Escape to go back. It is read-only — PhotoSync will not delete, move or +favourite anything on your server. + +An opened photo is fetched at the largest size your server will serve — the +original if it has full-size renditions enabled, its 1440px preview otherwise. +The blur you see for the moment before it arrives is the thumbhash the timeline +already carried, and it says so while it is standing in, so a blur is never +mistaken for the photo. Videos in the album show their preview frame with a video badge in the corner. There is no playback; this is a screenshot mod. @@ -98,6 +104,12 @@ puts it through the same queue as a manual one. - **Only while in a world** — on by default. No point photographing the main menu. - **Skip while a screen is open** — on by default. Your inventory is not a landscape. +- **Skip if you have not moved** — on by default. A cycle is skipped when the + camera has been in the same place, looking the same way, for a whole interval: + that shot would be the previous one again. It resumes on its own the moment you + move or look somewhere else, so an afk stretch costs you one photo rather than + a hundred identical ones. The view is judged from the camera, so following + someone in spectator mode counts as moving. Automatic captures are recorded separately from manual ones, so the two are distinguishable everywhere they appear. @@ -164,9 +176,14 @@ does not queue against a server it cannot reach, because the result is a list of failures you cannot act on. **Thumbnails are blank in Browse** — Minecraft's image decoder reads PNG and JPEG -only, and Immich's default thumbnail format is WebP. PhotoSync detects this and -switches to JPEG previews for the rest of the session, so this should self-correct -after the first few tiles. +only, and Immich's default thumbnail format is WebP. PhotoSync notices on the +first tile and asks for JPEG previews instead for the rest of the session, so +this self-corrects on its own. + +**An opened photo says it could not load** — close it and open it again, which +retries. If it fails every time, the log line says why; the usual cause is +Administration → Settings → Image → Preview format set to WebP on the server, +which nothing on this side can decode. Set it to JPEG. **A jar refuses to load** — check the table above. Fabric enforces each jar's declared version range, so this means the jar is for a different bucket. diff --git a/docs/PORTING.md b/docs/PORTING.md index a637f5b..995ee9d 100644 --- a/docs/PORTING.md +++ b/docs/PORTING.md @@ -348,6 +348,8 @@ deletes the placeholder. | render target | `Minecraft.getMainRenderTarget()` | ✓ | ✓ | **`gameRenderer.mainRenderTarget()`** | | `Minecraft.stop()`, `getWindow()` | ✓ | ✓ | ✓ | ✓ | | `execute(Runnable)`, `isSameThread()` | inherited from `BlockableEventLoop`, unchanged | ✓ | ✓ | ✓ | +| camera entity | `Minecraft.getCameraEntity()` → `world.entity.Entity` | ✓ | ✓ | ✓ | +| camera pose | `Entity.getX/getY/getZ/getYRot/getXRot` | ✓ | ✓ | ✓ | | `Util` package | `net.minecraft.Util` | ✓ | **`net.minecraft.util.Util`** | ✓ | | `Util.ioPool()` | `ExecutorService` | **`TracingExecutor`** | ✓ | ✓ | diff --git a/platform/1.20.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.20.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 474ed4c..1bf52bf 100644 --- a/platform/1.20.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.20.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/1.20.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.20.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 474ed4c..1bf52bf 100644 --- a/platform/1.20.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.20.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/1.20.6/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.20.6/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 474ed4c..1bf52bf 100644 --- a/platform/1.20.6/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.20.6/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/1.21.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.21.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 474ed4c..1bf52bf 100644 --- a/platform/1.21.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.21.1/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/1.21.11/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.21.11/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 032baca..c87a6d7 100644 --- a/platform/1.21.11/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.21.11/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.util.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/1.21.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.21.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 474ed4c..1bf52bf 100644 --- a/platform/1.21.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.21.4/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/1.21.5/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.21.5/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 474ed4c..1bf52bf 100644 --- a/platform/1.21.5/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.21.5/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/1.21.8/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/1.21.8/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 474ed4c..1bf52bf 100644 --- a/platform/1.21.8/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/1.21.8/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -34,6 +37,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().screen != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/platform/26.2/src/main/java/dev/photosync/platform/impl/GameAdapter.java b/platform/26.2/src/main/java/dev/photosync/platform/impl/GameAdapter.java index 702487c..78edc11 100644 --- a/platform/26.2/src/main/java/dev/photosync/platform/impl/GameAdapter.java +++ b/platform/26.2/src/main/java/dev/photosync/platform/impl/GameAdapter.java @@ -1,13 +1,16 @@ package dev.photosync.platform.impl; +import dev.photosync.core.capture.CameraPose; import dev.photosync.mcapi.GameContext; import lombok.extern.slf4j.Slf4j; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.util.Util; import net.minecraft.client.Minecraft; +import net.minecraft.world.entity.Entity; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; /** Ambient client state, the render thread, and the way out of the game. */ @Slf4j @@ -35,6 +38,20 @@ public final class GameAdapter implements GameContext { return Minecraft.getInstance().gui.screen() != null; } + /** + * The camera entity rather than the player, so that a spectator following + * someone else is judged on what the screenshot will actually show. It falls + * back to the player, and is null until one exists. + */ + @Override + public Optional cameraPose() { + Entity camera = Minecraft.getInstance().getCameraEntity(); + return camera == null + ? Optional.empty() + : Optional.of(new CameraPose(camera.getX(), camera.getY(), camera.getZ(), + camera.getYRot(), camera.getXRot())); + } + @Override public Path configDirectory() { return configDirectory; diff --git a/shared/client/src/main/java/dev/photosync/client/AutoCapture.java b/shared/client/src/main/java/dev/photosync/client/AutoCapture.java index e6bd88e..fe1ce89 100644 --- a/shared/client/src/main/java/dev/photosync/client/AutoCapture.java +++ b/shared/client/src/main/java/dev/photosync/client/AutoCapture.java @@ -1,5 +1,6 @@ package dev.photosync.client; +import dev.photosync.core.capture.CameraPose; import dev.photosync.core.capture.CaptureOrigin; import dev.photosync.core.capture.CapturedScreenshot; import dev.photosync.core.config.AutoCaptureSettings; @@ -9,6 +10,7 @@ import lombok.extern.slf4j.Slf4j; import java.io.IOException; import java.nio.file.Path; +import java.util.Optional; import java.util.function.Consumer; import java.util.function.Supplier; @@ -23,6 +25,11 @@ import java.util.function.Supplier; * *

Off by default, and it stays off until the player says otherwise: silently * filling someone's screenshots folder is not a feature. + * + *

When it is on, a cycle is skipped if the camera has not moved for a whole + * interval, because that shot would be the previous one again. The condition is + * "has not moved since the last capture", expressed as a duration so that it + * survives the player wandering off and coming back to the same spot. */ @Slf4j public final class AutoCapture { @@ -36,6 +43,10 @@ public final class AutoCapture { private long nextCaptureMillis; private boolean capturing; + /** The last view that differed from the one before it, and when it arrived. */ + private CameraPose restingPose; + private long movedAtMillis; + public AutoCapture(GameContext game, ScreenshotService screenshots, Supplier settings, Consumer sink) { this.game = game; @@ -51,11 +62,15 @@ public final class AutoCapture { // Disarmed, so switching the feature on never fires immediately -- // it always waits a full interval first. nextCaptureMillis = 0; + restingPose = null; return; } long now = System.currentTimeMillis(); long interval = current.intervalSeconds() * 1000L; + // Every tick, so that stillness is measured over the whole interval + // rather than sampled at its two ends. + observePose(now); if (nextCaptureMillis == 0) { nextCaptureMillis = now + interval; return; @@ -75,11 +90,47 @@ public final class AutoCapture { // where it is and the shot happens as soon as it closes. return; } + if (current.skipWhenStill() && stillFor(interval, now)) { + // The view is the one the last screenshot already has. Skip this + // cycle and arm the next: the moment the player moves, the timer + // resumes on its own, so nothing has to be re-enabled. + log.debug("Skipping an automatic capture: the view has not changed in {}s", + current.intervalSeconds()); + nextCaptureMillis = now + interval; + return; + } nextCaptureMillis = now + interval; take(current.fileNameSuffix()); } + /** + * Notes where the camera is, and when it last went somewhere new. + * + *

The comparison is against the last pose that differed rather + * than against the previous tick, so a player drifting slowly -- in water, on + * a boat, turning by a pixel at a time -- still counts as moving once the + * movement adds up, instead of being called still forever. + */ + private void observePose(long now) { + Optional pose = game.cameraPose(); + if (pose.isEmpty()) { + // No world, or no player in it yet. Nothing to compare, and stillness + // is not claimed against a view that does not exist. + restingPose = null; + return; + } + if (restingPose == null || pose.get().movedFrom(restingPose)) { + restingPose = pose.get(); + movedAtMillis = now; + } + } + + /** True once the view has been the same for a full capture cycle. */ + private boolean stillFor(long millis, long now) { + return restingPose != null && now - movedAtMillis >= millis; + } + private void take(String suffix) { capturing = true; screenshots.capture(suffix).whenComplete((file, failure) -> { diff --git a/shared/client/src/main/resources/assets/photosync/lang/en_us.json b/shared/client/src/main/resources/assets/photosync/lang/en_us.json index 629828f..32e02bf 100644 --- a/shared/client/src/main/resources/assets/photosync/lang/en_us.json +++ b/shared/client/src/main/resources/assets/photosync/lang/en_us.json @@ -37,7 +37,9 @@ "photosync.browse.failed": "Could not load the timeline", "photosync.browse.empty": "Nothing here yet", "photosync.browse.page_failed": "Could not load these photos", - "photosync.browse.opening": "Loading...", + "photosync.browse.opening": "Loading the full photo...", + "photosync.browse.open_failed": "Could not load this photo", + "photosync.browse.open_failed.hint": "Close and open it again to retry.", "photosync.browse.close_hint": "Esc to close", "photosync.browse.status": "%s photos in %s days", @@ -82,6 +84,8 @@ "photosync.settings.suffix": "Name suffix", "photosync.settings.only_in_world": "Only while in a world", "photosync.settings.skip_when_screen_open": "Skip while a screen is open", + "photosync.settings.skip_when_still": "Skip if you have not moved", + "photosync.settings.skip_when_still.detail": "Skips a cycle when the view has not changed since the last automatic screenshot.", "photosync.settings.notify": "Show messages in the corner", "photosync.settings.notify.detail": "One line, bottom left, for a moment.", diff --git a/shared/core/src/main/java/dev/photosync/core/capture/CameraPose.java b/shared/core/src/main/java/dev/photosync/core/capture/CameraPose.java new file mode 100644 index 0000000..e91d2c2 --- /dev/null +++ b/shared/core/src/main/java/dev/photosync/core/capture/CameraPose.java @@ -0,0 +1,39 @@ +package dev.photosync.core.capture; + +/** + * Where a screenshot would be taken from, and which way it would be looking. + * + *

Five plain numbers rather than anything belonging to the game, so that the + * automatic-capture timer can tell whether the view has changed without this + * module knowing what a camera is. The platform fills it in from whatever entity + * the camera is attached to, which is the player except in spectator mode. + */ +public record CameraPose(double x, double y, double z, float yaw, float pitch) { + + /** + * A player standing still is not perfectly still -- riding a boat, bobbing in + * water or resting on a slime block all jitter the last decimals -- so + * "moved" needs a floor. Five centimetres and half a degree sit well below + * anything a player does on purpose: one sneaking tick covers 0.065 blocks, + * and a single pixel of mouse movement turns the view by more than half a + * degree at default sensitivity. + */ + private static final double POSITION_EPSILON = 0.05; + private static final float ROTATION_EPSILON = 0.5f; + + /** + * Whether this is a different enough view from {@code other} to count as the + * player having moved -- walked, or looked somewhere else. + * + *

Callers compare against the last pose that differed rather than against + * the previous tick, so movement slower than the epsilon per tick still + * registers once it has accumulated. + */ + public boolean movedFrom(CameraPose other) { + return Math.abs(x - other.x) > POSITION_EPSILON + || Math.abs(y - other.y) > POSITION_EPSILON + || Math.abs(z - other.z) > POSITION_EPSILON + || Math.abs(yaw - other.yaw) > ROTATION_EPSILON + || Math.abs(pitch - other.pitch) > ROTATION_EPSILON; + } +} diff --git a/shared/core/src/main/java/dev/photosync/core/config/AutoCaptureSettings.java b/shared/core/src/main/java/dev/photosync/core/config/AutoCaptureSettings.java index 32ff9c6..05133d9 100644 --- a/shared/core/src/main/java/dev/photosync/core/config/AutoCaptureSettings.java +++ b/shared/core/src/main/java/dev/photosync/core/config/AutoCaptureSettings.java @@ -14,7 +14,8 @@ public record AutoCaptureSettings( int intervalSeconds, String fileNameSuffix, boolean onlyInWorld, - boolean skipWhenScreenOpen) { + boolean skipWhenScreenOpen, + boolean skipWhenStill) { public static final int MIN_INTERVAL_SECONDS = 5; public static final int MAX_INTERVAL_SECONDS = 3600; @@ -26,6 +27,9 @@ public record AutoCaptureSettings( .fileNameSuffix("_auto") .onlyInWorld(true) .skipWhenScreenOpen(true) + // On, because the alternative is a hundred identical photographs + // of wherever the player was standing when they went for lunch. + .skipWhenStill(true) .build(); } diff --git a/shared/core/src/main/java/dev/photosync/core/provider/immich/ImmichProvider.java b/shared/core/src/main/java/dev/photosync/core/provider/immich/ImmichProvider.java index 6efd4e9..3b81617 100644 --- a/shared/core/src/main/java/dev/photosync/core/provider/immich/ImmichProvider.java +++ b/shared/core/src/main/java/dev/photosync/core/provider/immich/ImmichProvider.java @@ -33,12 +33,15 @@ import java.time.ZoneOffset; import java.time.format.DateTimeParseException; import java.util.ArrayList; import java.util.Base64; +import java.util.Collections; import java.util.Comparator; +import java.util.EnumSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; +import java.util.Set; /** * Immich, expressed in PhotoSync's terms. @@ -61,14 +64,24 @@ public final class ImmichProvider implements PhotoProvider { private static final Type BULK_RESULT = new TypeToken>() { }.getType(); + /** + * What to ask for when a tile in the grid needs filling, and what to ask for + * when one is opened, most detailed first. Both end in {@code preview}, the + * only rendition Immich always has. See {@link #thumbnail}. + */ + private static final List GRID_LADDER = List.of(Rendition.THUMBNAIL, Rendition.PREVIEW); + private static final List DETAIL_LADDER = List.of(Rendition.FULLSIZE, Rendition.PREVIEW); + private final ProviderDescriptor descriptor; private final ImmichApi api; /** - * Whether this server's small thumbnails are usable, learned from the first - * one we fetch. See {@link #thumbnail}. + * Renditions this server turned out not to serve in a form the game can + * decode. Learned from the first attempt and skipped from then on, so a + * server missing one costs a single wasted request per session rather than + * one per image. See {@link #thumbnail}. */ - private volatile boolean smallThumbnails = true; + private final Set unusable = Collections.synchronizedSet(EnumSet.noneOf(Rendition.class)); ImmichProvider(ProviderDescriptor descriptor, ProviderConnection connection) { this.descriptor = descriptor; @@ -198,25 +211,65 @@ public final class ImmichProvider implements PhotoProvider { return new BucketPage(bucket, assets); } + /** + * Walks the ladder for the requested size, taking the first rendition that + * comes back as something the game can decode. + * + *

The rungs above the last are optional on a real server, so a failure + * there is not the caller's problem: it retires that rendition and drops to + * the next one. The last rung is {@code preview}, which every Immich + * installation serves and defaults to JPEG, so its failures are the asset's + * or the server's and go back to the caller unchanged. + */ @Override public byte[] thumbnail(String assetId, ThumbnailSize size) throws ProviderException { // Videos have no still of their own to serve, but Immich renders one for // them at the same endpoint -- which is exactly what a video tile needs. String path = "/assets/" + assetId + "/thumbnail"; - if (size == ThumbnailSize.DETAIL || !smallThumbnails) { - return api.getBytes(path, Map.of("size", "preview")); + List ladder = size == ThumbnailSize.DETAIL ? DETAIL_LADDER : GRID_LADDER; + + for (Rendition rendition : ladder.subList(0, ladder.size() - 1)) { + if (unusable.contains(rendition)) { + continue; + } + try { + byte[] bytes = api.getBytes(path, Map.of("size", rendition.query())); + if (!isWebP(bytes)) { + return bytes; + } + retire(rendition, "it is WebP, which the game's image decoder does not read"); + } catch (ProviderException e) { + switch (e.kind()) { + // Not generated, or this API key may not ask for it. Neither + // changes while the game is running, so stop asking. + case NOT_FOUND, AUTHENTICATION, PROTOCOL -> retire(rendition, e.getMessage()); + // Asking a second time is precisely what we were told not to do. + case RATE_LIMITED -> throw e; + // Might be this asset, might be this minute. Take the smaller + // image now and try the big one again next time. + default -> log.debug("Immich would not serve the {} of {}: {}", + rendition.query(), assetId, e.getMessage()); + } + } } - byte[] small = api.getBytes(path, Map.of("size", "thumbnail")); - if (!isWebP(small)) { - return small; + + Rendition floor = ladder.get(ladder.size() - 1); + byte[] bytes = api.getBytes(path, Map.of("size", floor.query())); + if (isWebP(bytes)) { + // Only reachable on a server whose preview format has been changed + // from the default. Nothing below this rung would help, so say what + // is wrong instead of handing the screen bytes it cannot draw. + throw new ProviderException(ProviderException.Kind.PROTOCOL, + "This Immich server returns WebP previews, and Minecraft decodes only PNG and JPEG. " + + "Set Administration -> Settings -> Image -> Preview format to JPEG."); + } + return bytes; + } + + private void retire(Rendition rendition, String reason) { + if (unusable.add(rendition)) { + log.info("Not using Immich's {} images this session, because {}", rendition.query(), reason); } - // Immich's default thumbnail format is WebP, which the game's image - // decoder cannot read; its previews default to JPEG, which it can. One - // wasted request per session buys correct tiles on those servers, and - // servers already configured for JPEG never take this branch. - log.info("This Immich server serves WebP thumbnails; falling back to previews for the grid"); - smallThumbnails = false; - return api.getBytes(path, Map.of("size", "preview")); } /** RIFF container with a WEBP fourcc, per the WebP specification. */ @@ -226,6 +279,31 @@ public final class ImmichProvider implements PhotoProvider { && bytes[8] == 'W' && bytes[9] == 'E' && bytes[10] == 'B' && bytes[11] == 'P'; } + /** + * The three sizes Immich renders an asset at, named as the {@code size} query + * parameter spells them. + * + *

Only {@code preview} can be relied on. {@code thumbnail} is 250px and + * WebP by default, which is both too small to open and undecodable here; + * {@code fullsize} is off by default, and where it is on it can redirect to + * a download endpoint that a restricted API key is not allowed to follow. + */ + private enum Rendition { + FULLSIZE("fullsize"), + PREVIEW("preview"), + THUMBNAIL("thumbnail"); + + private final String query; + + Rendition(String query) { + this.query = query; + } + + String query() { + return query; + } + } + @Override public void close() { // Nothing to release: java.net.http.HttpClient has no close() before diff --git a/shared/core/src/test/java/dev/photosync/core/capture/CameraPoseTest.java b/shared/core/src/test/java/dev/photosync/core/capture/CameraPoseTest.java new file mode 100644 index 0000000..9b51a3b --- /dev/null +++ b/shared/core/src/test/java/dev/photosync/core/capture/CameraPoseTest.java @@ -0,0 +1,57 @@ +package dev.photosync.core.capture; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The thresholds decide whether a player is "still", which decides whether their + * screenshots folder fills up while they are away from the keyboard. Both + * mistakes are visible: too tight and a boat's bobbing counts as sightseeing, + * too loose and a step to one side does not. + */ +class CameraPoseTest { + + private static final CameraPose RESTING = new CameraPose(100.0, 64.0, -20.0, 45.0f, 10.0f); + + @Test + @DisplayName("an identical pose has not moved") + void identical() { + assertFalse(RESTING.movedFrom(RESTING)); + } + + @Test + @DisplayName("jitter below the thresholds is not movement") + void jitter() { + CameraPose bobbing = new CameraPose(100.02, 63.98, -20.01, 45.2f, 10.3f); + + assertFalse(bobbing.movedFrom(RESTING)); + } + + @Test + @DisplayName("one sneaking tick is movement") + void walked() { + // A sneaking player covers 0.065 blocks per tick -- the slowest way to + // travel, and the closest a deliberate move comes to the threshold. + CameraPose crept = new CameraPose(100.065, 64.0, -20.0, 45.0f, 10.0f); + + assertTrue(crept.movedFrom(RESTING)); + } + + @Test + @DisplayName("looking somewhere else is movement, without leaving the spot") + void lookedAround() { + assertTrue(new CameraPose(100.0, 64.0, -20.0, 46.0f, 10.0f).movedFrom(RESTING), "yaw"); + assertTrue(new CameraPose(100.0, 64.0, -20.0, 45.0f, 11.0f).movedFrom(RESTING), "pitch"); + } + + @Test + @DisplayName("each axis is judged on its own") + void everyAxis() { + assertTrue(new CameraPose(101.0, 64.0, -20.0, 45.0f, 10.0f).movedFrom(RESTING), "x"); + assertTrue(new CameraPose(100.0, 65.0, -20.0, 45.0f, 10.0f).movedFrom(RESTING), "y"); + assertTrue(new CameraPose(100.0, 64.0, -21.0, 45.0f, 10.0f).movedFrom(RESTING), "z"); + } +} diff --git a/shared/mc-api/src/main/java/dev/photosync/mcapi/GameContext.java b/shared/mc-api/src/main/java/dev/photosync/mcapi/GameContext.java index 8bc181f..e78b991 100644 --- a/shared/mc-api/src/main/java/dev/photosync/mcapi/GameContext.java +++ b/shared/mc-api/src/main/java/dev/photosync/mcapi/GameContext.java @@ -1,6 +1,9 @@ package dev.photosync.mcapi; +import dev.photosync.core.capture.CameraPose; + import java.nio.file.Path; +import java.util.Optional; /** The ambient client state PhotoSync has to consult, and the render thread. */ public interface GameContext { @@ -11,6 +14,18 @@ public interface GameContext { /** True while any screen is up, vanilla's or ours. Automatic capture checks this. */ boolean screenOpen(); + /** + * Where the camera is and which way it faces, or empty when there is nothing + * to be looking through -- the title screen, or a world that is still + * loading. + * + *

Sampled every tick by the automatic-capture timer, which uses it to tell + * an idle player apart from one who is actually somewhere new. Nothing else + * needs it, and nothing should read a Minecraft position through it: it is + * deliberately a snapshot of five numbers rather than a handle on an entity. + */ + Optional cameraPose(); + /** {@code .minecraft/config}, where PhotoSync keeps its settings and upload queue. */ Path configDirectory(); diff --git a/shared/ui/src/main/java/dev/photosync/ui/image/ThumbnailCache.java b/shared/ui/src/main/java/dev/photosync/ui/image/ThumbnailCache.java index 83b1a09..829fbfe 100644 --- a/shared/ui/src/main/java/dev/photosync/ui/image/ThumbnailCache.java +++ b/shared/ui/src/main/java/dev/photosync/ui/image/ThumbnailCache.java @@ -55,12 +55,19 @@ public final class ThumbnailCache implements AutoCloseable { this.textures = textures; this.loader = loader; this.size = size; - this.capacity = Math.max(8, capacity); + this.capacity = Math.max(1, capacity); } - /** Follows the player's setting without discarding what is already loaded. */ + /** + * Follows the player's setting without discarding what is already loaded. + * + *

Only one is imposed here, because the caller is the one who knows what + * it is caching: the grid's floor comes from its config setting, and the + * detail view genuinely wants a capacity of two. Nothing thrashes at a small + * capacity anyway -- {@link #endFrame()} refuses to evict what was drawn. + */ public void capacity(int value) { - this.capacity = Math.max(8, value); + this.capacity = Math.max(1, value); } /** Call once at the top of a frame, before any {@link #of} in that frame. */ diff --git a/shared/ui/src/main/java/dev/photosync/ui/screen/SettingsScreen.java b/shared/ui/src/main/java/dev/photosync/ui/screen/SettingsScreen.java index 4d5a82a..7cb9043 100644 --- a/shared/ui/src/main/java/dev/photosync/ui/screen/SettingsScreen.java +++ b/shared/ui/src/main/java/dev/photosync/ui/screen/SettingsScreen.java @@ -215,6 +215,9 @@ public final class SettingsScreen extends PhotoSyncScreen { toggle("photosync.settings.skip_when_screen_open", "", () -> draft().autoCapture().skipWhenScreenOpen(), value -> autoCapture(settings -> settings.toBuilder().skipWhenScreenOpen(value).build())); + toggle("photosync.settings.skip_when_still", "photosync.settings.skip_when_still.detail", + () -> draft().autoCapture().skipWhenStill(), + value -> autoCapture(settings -> settings.toBuilder().skipWhenStill(value).build())); } private void buildNotifications() { diff --git a/shared/ui/src/main/java/dev/photosync/ui/screen/TimelineScreen.java b/shared/ui/src/main/java/dev/photosync/ui/screen/TimelineScreen.java index 503fed3..950fabe 100644 --- a/shared/ui/src/main/java/dev/photosync/ui/screen/TimelineScreen.java +++ b/shared/ui/src/main/java/dev/photosync/ui/screen/TimelineScreen.java @@ -77,8 +77,10 @@ public final class TimelineScreen extends PhotoSyncScreen { BrowserSettings settings = ui.core().config().current().browser(); this.tiles = new ThumbnailCache(ui.bridge().textures(), ui.core().thumbnails(), ThumbnailSize.GRID, settings.thumbnailCacheEntries()); - // Three is enough for the one open photo and the two either side of it. - this.detail = new ThumbnailCache(ui.bridge().textures(), ui.core().thumbnails(), ThumbnailSize.DETAIL, 3); + // Only one photo is ever open, and these are full-resolution textures -- + // a 4K screenshot is 33MB on the GPU. Two, so re-opening the last one is + // instant, and no more than that. + this.detail = new ThumbnailCache(ui.bridge().textures(), ui.core().thumbnails(), ThumbnailSize.DETAIL, 2); } @Override @@ -297,17 +299,40 @@ public final class TimelineScreen extends PhotoSyncScreen { half - uHalf, half - vHalf, half + uHalf, half + vHalf); } + /** + * The opened photo, at whatever resolution has arrived so far. + * + *

What arrives first is the ThumbHash: a blurred 32x32 that is right for a + * tile and nowhere near enough for a full-screen view. It is still worth + * drawing, because it says which photo is opening, but it is dimmed and + * labelled while it stands in -- an unannotated blur is indistinguishable + * from a mod that fetched the wrong size. + */ private void renderOpened(RenderBridge render, int mouseX, int mouseY) { detail.beginFrame(); Rect area = body(); render.fill(area.x(), area.y(), area.width(), area.height(), theme().overlay()); Rect frame = area.inset(6); Optional image = detail.of(opened); - if (image.isPresent()) { - drawContained(render, image.get().texture(), frame); - } else { + + if (detail.failed(opened.id())) { + chrome.notice(render, frame, chrome.translate("photosync.browse.open_failed"), + chrome.translate("photosync.browse.open_failed.hint")); + } else if (image.isEmpty()) { chrome.notice(render, frame, chrome.translate("photosync.browse.opening"), ""); + } else { + drawContained(render, image.get().texture(), frame); + if (image.get().placeholder()) { + render.fill(frame.x(), frame.y(), frame.width(), frame.height(), + theme().fade(theme().overlay(), 0.6f)); + chrome.centered(render, chrome.translate("photosync.browse.opening"), + new Rect(frame.x(), frame.centerY() - render.lineHeight(), frame.width(), render.lineHeight()), + theme().text()); + chrome.busyBar(render, new Rect(frame.centerX() - 60, frame.centerY() + 6, 120, 3), + System.currentTimeMillis(), theme().accent()); + } } + chrome.centered(render, chrome.translate("photosync.browse.close_hint"), area.bottom(render.lineHeight() + 2), theme().textFaint()); detail.endFrame(); @@ -367,7 +392,7 @@ public final class TimelineScreen extends PhotoSyncScreen { @Override public boolean mouseClicked(double mouseX, double mouseY, int button) { if (opened != null) { - opened = null; + closeOpened(); return true; } if (scroll.mouseClicked(mouseX, mouseY, button)) { @@ -401,7 +426,7 @@ public final class TimelineScreen extends PhotoSyncScreen { @Override public boolean keyPressed(int key, int scanCode, int modifiers) { if (opened != null && key == Keys.ESCAPE) { - opened = null; + closeOpened(); return true; } return switch (key) { @@ -425,6 +450,19 @@ public final class TimelineScreen extends PhotoSyncScreen { }; } + /** + * A cache entry that failed stays failed, which is right for a grid tile the + * player is scrolling past and wrong for the one photo they chose to open. It + * holds three entries, so dropping them all is the cheapest way to let a + * second click try again. + */ + private void closeOpened() { + if (detail.failed(opened.id())) { + detail.clear(); + } + opened = null; + } + /** Escape dismisses the open photo before it dismisses the screen. */ @Override public boolean closeOnEscape() {