Files

142 lines
7.6 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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/<v> 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.201.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/<name>`, 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/<v>/…/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.