Complete Milestone 4 writing acceptance and migration coverage (#26, #27)

README.md

@@ -4,8 +4,8 @@ Rivencraft is a clean, incremental port of the ideas and gameplay of
 [Mystcraft Legacy](https://github.com/Mystcraft/Mystcraft-Legacy) to Minecraft
 1.21.1 on NeoForge.
 
-The bootstrap, books/linking, and Ages milestones are complete. Milestone 4 is
+The bootstrap, books/linking, Ages, and writing-system milestones are complete.
-building the writing system on a data-pack symbol catalog and persistent
+The writing system uses a data-pack symbol catalog and persistent
 ordered-writing model. The project currently provides functional Linking Books
 and randomly seeded, persistent vanilla-like Ages created from page-free
 Descriptive Books. Instability and full symbol-to-world grammar interpretation

docs/PORTING.md

@@ -35,7 +35,8 @@ acceptance issue and all required implementation issues are complete.
    [Milestone 3](https://gitea.tail180fb1.ts.net/dnorberg/rivencraft/milestone/8).
 4. **Writing system** — pages, notebooks, desks, book assembly, and the baseline
    Ink Mixer for Link Panels. Symbol effects are deferred until Milestone 5.
-   **In progress.**
+   **Complete.** See [Milestone 4](https://gitea.tail180fb1.ts.net/dnorberg/rivencraft/milestone/9)
+   and [writing-system data and migration](WRITING_DATA.md).
 5. **Age grammar** — translate symbol combinations into modern biome, noise,
    dimension-type, and world-generation configuration. Optional integrations
    may add composable biome regions and seasonal behavior.
@@ -63,11 +64,11 @@ The writing system and Age grammar will build on the existing page-free Age
 path. Page-free books remain a useful deterministic fallback and test fixture
 after written symbol combinations become available.
 
-Milestone 4 begins with a versioned, data-pack-defined symbol catalog and an
+Milestone 4 delivered a versioned, data-pack-defined symbol catalog and an
 immutable ordered-writing model. See [WRITING_DATA.md](WRITING_DATA.md). These
 contracts preserve unknown symbol identifiers and store author intent; they do
 not interpret symbols into Age generation, which remains Milestone 5 work.
-The Ink Mixer is included in Milestone 4 after book assembly; its first release
+The Ink Mixer is included in Milestone 4 after book assembly; it
 uses paper and Ink Vials to make baseline Link Panels. Adding modifier
 ingredients and symbol-driven link properties belongs to Milestone 5.
 

docs/WRITING_DATA.md

@@ -65,11 +65,11 @@ unresolved symbol, and baseline link panel. Symbol and unresolved-symbol pages
 retain their namespaced symbol reference; unresolved pages can be inspected and
 removed but are rejected as usable writing until their data pack returns.
 
-Only server-side writing operations may create a known symbol
+Only server-side writing operations may create a known symbol page or link
-page or link panel from vanilla paper. The shared `PageItem` creation and validation gates reject
+panel from vanilla paper. The shared `PageItem` creation and validation gates
-new pages for identifiers absent from the server catalog. Later Writing Desk
+reject new pages for identifiers absent from the server catalog. The Writing
-and Book Binder packets must call these gates rather than accepting page
+Desk refuses to copy an unresolved page, and the Book Binder keeps such a page
-contents supplied by a client.
+removable but will not assemble a book from it until its symbol resolves again.
 
 The item model uses server-assigned custom-model data derived from the symbol
 identifier. Built-in pages reproduce Mystcraft Legacy's four-word poem
@@ -84,6 +84,29 @@ placement. The surface draws the native 30×40 portrait page region from
 Legacy's book UI atlas; symbols are layered at a uniform scale, while the
 square item models remain unchanged in inventories.
 
+A Notebook holds 27 ordered positions. Its pages, names, gaps, and unresolved
+IDs are saved on the item, so moving the Notebook between an inventory, Desk,
+and Binder does not rearrange them. A loose Page in a Desk tab exposes just one
+position; a Notebook exposes all 27. The Desk has 88 persistent collection
+tabs, shown four at a time. Use the up/down tab controls to move by four, put
+paper and Ink Vials in their input slots, select a stored page, and take the
+copy from the virtual output. Taking it spends one paper and the configured
+symbol-copy ink cost. The `AZ`, `ALL`, search, and category controls only change
+which stored pages are displayed; the server rechecks the actual source before
+copying. Moving a page within the Desk or dragging it into the inventory removes
+the original only when the destination accepts it.
+
+The Book Binder takes a leather or empty-Notebook cover, an Age title, one Link
+Panel, and optionally up to 27 ordered written Pages. A filled Notebook can be
+imported at a chosen strip position; the empty Notebook remains recoverable.
+The preview result is virtual. Taking the completed Descriptive Book commits the
+inputs once, creates its Age identity, and saves schema-versioned
+`OrderedWriting` plus the Age reference on the book. Link-Panel-only books
+create random Ages. A written symbol does **not** yet change that generation.
+Inputs, title, and pending Age identity survive Binder save/load; breaking the
+Binder returns stored inputs. The completed book retains both its writing and
+Age identity through item save/load and copying.
+
 ## Ink and baseline link panels
 
 An Ink Vial is crafted from two black dye, a glass bottle, and a water bucket.
@@ -103,3 +126,31 @@ resolved page, then spends paper and ink only when that copy is taken. The
 desk's 16-unit ink meter refills from an Ink Vial when needed and returns a
 vanilla glass bottle. Link-property modifiers are
 deferred beyond this baseline flow.
+
+The Ink Mixer is the station version of Link Panel preparation. Put paper and
+Ink Vials in its inputs and take the Link Panel from its virtual output. Its
+16-unit basin refills only when needed and only if the glass-bottle output has
+space. The basin and inputs persist through save/load; breaking the Mixer
+returns stored items. A Link Panel plus leather also crafts an unwritten
+Linking Book. JEI, when installed, displays the Mixer operation and the
+writing, binding, and Linking Book crafting flows; it is not required to play.
+
+## Migration and validation
+
+The 0.3.7.2 bare-list `OrderedWriting` form reads as schema version 1 without
+changing entry order. Descriptive Books with no writing component keep their
+existing Age behavior. Old eight-slot Desks migrate their four collections,
+ink supply, paper, bottle return, and legacy output into the 88-tab layout;
+saved inputs are never silently discarded. Missing symbol IDs survive page,
+Notebook, Binder, and assembled-book persistence instead of being substituted
+or erased.
+
+Dedicated-server GameTests exercise codec and item round-trips, legacy data,
+workstation NBT reload, break drops, full-inventory refusal, forged menu IDs,
+and replayed output requests. World saves and chunk unloads use the same block
+entity NBT persistence path as those reload tests. This does not replace a
+manual smoke test of loading an older world before updating a production save.
+
+Milestone 4 stops at preserving author intent and offering a playable random
+Age fallback. Milestone 5 will define symbol grammar and validation, translate
+valid writing into actual Age generation, and introduce symbol and ink modifiers.

gradle.properties

@@ -36,7 +36,7 @@ mod_name=Rivencraft
 # The license of the mod. Review your options at https://choosealicense.com/. All Rights Reserved is the default.
 mod_license=LGPL-3.0-or-later
 # The mod version. Rivencraft uses the x.y.z.b policy documented in README.md.
-mod_version=0.3.9.2
+mod_version=0.4.0.0
 immersive_portals_version=6.0.7
 # The group ID for the mod. It is only important when publishing as an artifact to a Maven repository.
 # This should match the base package used for the mod sources.

src/main/java/net/dnorberg/rivencraft/block/entity/BookBinderBlockEntity.java

@@ -105,7 +105,7 @@ public final class BookBinderBlockEntity extends BlockEntity implements Containe
                 || items.get(LINK_PANEL).getCount() != 1) return false;
         for (int slot = FIRST_PAGE; slot < SIZE; slot++) {
             ItemStack page = items.get(slot);
-            if (!page.isEmpty() && (!validPage(page) || page.getCount() != 1
+            if (!page.isEmpty() && (!validPage(page) || !PageItem.isUsableWriting(page) || page.getCount() != 1
                     || PageItem.content(page).kind() == PageContent.Kind.BLANK)) return false;
         }
         return true;

src/main/java/net/dnorberg/rivencraft/gametest/BookBinderGameTests.java

@@ -74,6 +74,25 @@ public final class BookBinderGameTests {
         helper.succeed();
     }
 
+    @GameTest(template = "empty")
+    public static void missingSymbolPageSurvivesReloadButCannotBeAssembled(GameTestHelper helper) {
+        BookBinderBlockEntity binder = place(helper);
+        binder.setTitle("Recoverable Writing");
+        binder.setItem(BookBinderBlockEntity.COVER, new ItemStack(Items.LEATHER));
+        binder.setItem(BookBinderBlockEntity.LINK_PANEL, PageItem.createLinkPanelPage());
+        SymbolReference missing = new SymbolReference(ResourceLocation.parse("example:removed_symbol"));
+        binder.setItem(BookBinderBlockEntity.FIRST_PAGE, PageItem.createRecoveredPage(missing));
+        var saved = binder.saveWithoutMetadata(helper.getLevel().registryAccess());
+        var restored = new BookBinderBlockEntity(binder.getBlockPos(), binder.getBlockState());
+        restored.loadWithComponents(saved, helper.getLevel().registryAccess());
+        helper.assertValueEqual(PageItem.content(restored.getItem(BookBinderBlockEntity.FIRST_PAGE))
+                .symbol().orElseThrow(), missing, "Reload must preserve the removed data-pack symbol ID");
+        helper.assertFalse(restored.canAssemble(), "Missing symbol cannot create a new written Age");
+        helper.assertTrue(restored.getItem(BookBinderBlockEntity.FIRST_PAGE).is(ModItems.PAGE.get()),
+                "Rejected writing must remain recoverable as a Page");
+        helper.succeed();
+    }
+
     @GameTest(template = "empty")
     public static void assemblyIsAtomicAndRetainsPageOrder(GameTestHelper helper) {
         BookBinderBlockEntity binder = place(helper);
@@ -237,6 +256,39 @@ public final class BookBinderGameTests {
         helper.succeed();
     }
 
+    @GameTest(template = "empty")
+    public static void forgedBinderActionsAndReplayedOutputCannotDuplicateBook(GameTestHelper helper) {
+        BookBinderBlockEntity binder = place(helper);
+        binder.setTitle("Single Assembly");
+        binder.setItem(BookBinderBlockEntity.COVER, new ItemStack(Items.LEATHER));
+        binder.setItem(BookBinderBlockEntity.LINK_PANEL, PageItem.createLinkPanelPage());
+        var first = helper.makeMockServerPlayerInLevel();
+        var second = helper.makeMockServerPlayerInLevel();
+        for (var player : List.of(first, second)) player.moveTo(binder.getBlockPos().getX() + 0.5,
+                binder.getBlockPos().getY() + 0.5, binder.getBlockPos().getZ() + 0.5);
+        BookBinderMenu firstMenu = new BookBinderMenu(1, first.getInventory(), binder);
+        BookBinderMenu secondMenu = new BookBinderMenu(2, second.getInventory(), binder);
+        helper.assertFalse(firstMenu.clickMenuButton(first, BookBinderMenu.SELECT_BASE + BookBinderBlockEntity.PAGE_COUNT),
+                "A fabricated page selection must be rejected");
+        helper.assertFalse(firstMenu.clickMenuButton(first, BookBinderMenu.STRIP_CLICK_BASE
+                + BookBinderBlockEntity.PAGE_COUNT + 1), "A fabricated strip address must be rejected");
+        helper.assertTrue(firstMenu.quickMoveStack(first, BookBinderMenu.RESULT).is(ModItems.DESCRIPTIVE_BOOK.get()),
+                "The first viewer must receive the single assembled book");
+        helper.assertTrue(secondMenu.quickMoveStack(second, BookBinderMenu.RESULT).isEmpty(),
+                "Another viewer must not take a stale result");
+        helper.assertTrue(firstMenu.quickMoveStack(first, BookBinderMenu.RESULT).isEmpty(),
+                "Replaying the output request must not create a second book");
+        int books = first.getInventory().items.stream().filter(stack -> stack.is(ModItems.DESCRIPTIVE_BOOK.get()))
+                .mapToInt(ItemStack::getCount).sum();
+        helper.assertValueEqual(books, 1, "Only one completed book may be emitted");
+        helper.assertTrue(binder.getItem(BookBinderBlockEntity.COVER).isEmpty(), "Cover must be consumed once");
+        helper.assertTrue(binder.getItem(BookBinderBlockEntity.LINK_PANEL).isEmpty(), "Panel must be consumed once");
+        helper.getLevel().setBlock(binder.getBlockPos(), Blocks.AIR.defaultBlockState(), 3);
+        helper.assertFalse(firstMenu.clickMenuButton(first, BookBinderMenu.IMPORT_NOTEBOOK),
+                "A stale binder menu must reject packets after block removal");
+        helper.succeed();
+    }
+
     @GameTest(template = "empty")
     public static void legacyStripInsertsAndRemovesPagesInOrder(GameTestHelper helper) {
         BookBinderBlockEntity binder = place(helper);

src/main/java/net/dnorberg/rivencraft/gametest/InkMixerGameTests.java

@@ -12,7 +12,9 @@ import net.minecraft.gametest.framework.GameTestHelper;
 import net.minecraft.world.inventory.ClickType;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.Items;
+import net.minecraft.world.entity.item.ItemEntity;
 import net.minecraft.world.level.block.Blocks;
+import net.minecraft.world.phys.AABB;
 import net.neoforged.neoforge.gametest.GameTestHolder;
 import net.neoforged.neoforge.gametest.PrefixGameTestTemplate;
 
@@ -98,6 +100,14 @@ public final class InkMixerGameTests {
                 "Shift-click must consume only one paper");
         helper.getLevel().setBlock(mixer.getBlockPos(), Blocks.AIR.defaultBlockState(), 3);
         helper.assertFalse(menu.stillValid(player), "Breaking the mixer must invalidate its menu");
+        var drops = helper.getLevel().getEntitiesOfClass(ItemEntity.class,
+                new AABB(mixer.getBlockPos()).inflate(2));
+        int paper = drops.stream().filter(entity -> entity.getItem().is(Items.PAPER))
+                .mapToInt(entity -> entity.getItem().getCount()).sum();
+        int bottles = drops.stream().filter(entity -> entity.getItem().is(Items.GLASS_BOTTLE))
+                .mapToInt(entity -> entity.getItem().getCount()).sum();
+        helper.assertValueEqual(paper, 1, "Breaking the mixer must return unused paper once");
+        helper.assertValueEqual(bottles, 1, "Breaking the mixer must return the consumed vial's bottle once");
         helper.succeed();
     }
 }

src/main/java/net/dnorberg/rivencraft/gametest/WritingDataGameTests.java

@@ -3,7 +3,9 @@ package net.dnorberg.rivencraft.gametest;
 import com.mojang.serialization.JsonOps;
 import io.netty.buffer.Unpooled;
 import java.util.List;
+import java.util.UUID;
 import net.dnorberg.rivencraft.Rivencraft;
+import net.dnorberg.rivencraft.book.AgeReference;
 import net.dnorberg.rivencraft.writing.OrderedWriting;
 import net.dnorberg.rivencraft.writing.PageContent;
 import net.dnorberg.rivencraft.writing.SymbolCatalog;
@@ -71,8 +73,12 @@ public final class WritingDataGameTests {
                 "0.3.7.2 bare-list writing must load into schema version 1 intact");
 
         ItemStack oldPageFreeBook = new ItemStack(ModItems.DESCRIPTIVE_BOOK.get());
+        AgeReference originalAge = new AgeReference(UUID.randomUUID(), "Legacy Page-Free Age");
+        oldPageFreeBook.set(ModDataComponents.AGE_REFERENCE.get(), originalAge);
         CompoundTag saved = (CompoundTag) oldPageFreeBook.saveOptional(helper.getLevel().registryAccess());
         ItemStack restored = ItemStack.parseOptional(helper.getLevel().registryAccess(), saved);
+        helper.assertValueEqual(restored.get(ModDataComponents.AGE_REFERENCE.get()), originalAge,
+                "A page-free book must keep its existing Age identity");
         helper.assertValueEqual(restored.getOrDefault(ModDataComponents.BOOK_WRITING.get(), OrderedWriting.EMPTY),
                 OrderedWriting.EMPTY, "Older page-free books must not require writing migration");
         helper.succeed();
@@ -136,6 +142,10 @@ public final class WritingDataGameTests {
         );
 
         ItemStack recovered = PageItem.createRecoveredPage(unknown);
+        CompoundTag recoveredTag = (CompoundTag) recovered.saveOptional(helper.getLevel().registryAccess());
+        ItemStack restoredRecovered = ItemStack.parseOptional(helper.getLevel().registryAccess(), recoveredTag);
+        helper.assertValueEqual(PageItem.content(restoredRecovered).symbol().orElseThrow(), unknown,
+                "A missing symbol must survive a full Page item reload");
         helper.assertFalse(PageItem.isUsableWriting(recovered),
                 "An unresolved page must remain recoverable without becoming valid writing");
         try {

src/main/java/net/dnorberg/rivencraft/gametest/WritingDeskGameTests.java

@@ -249,11 +249,40 @@ public final class WritingDeskGameTests {
                 "First viewer should take the last available copy");
         helper.assertTrue(secondMenu.quickMoveStack(second, WritingDeskMenu.COPY_RESULT).isEmpty(),
                 "Second viewer must not copy the already-consumed paper");
+        helper.assertTrue(firstMenu.quickMoveStack(first, WritingDeskMenu.COPY_RESULT).isEmpty(),
+                "Replaying the first viewer's output request must not make another page");
         helper.assertTrue(desk.getItem(WritingDeskBlockEntity.PAPER).isEmpty(),
                 "Shared desk must spend paper only once");
         helper.succeed();
     }
 
+    @GameTest(template = "empty")
+    public static void forgedDeskSelectionsAndStaleMenuCannotCopy(GameTestHelper helper) {
+        BlockPos pos = helper.absolutePos(new BlockPos(1, 1, 1));
+        helper.getLevel().setBlock(pos, ModBlocks.WRITING_DESK.get().defaultBlockState(), 3);
+        WritingDeskBlockEntity desk = (WritingDeskBlockEntity) helper.getLevel().getBlockEntity(pos);
+        desk.setItem(0, PageItem.createLinkPanelPage());
+        desk.setItem(WritingDeskBlockEntity.PAPER, new ItemStack(Items.PAPER));
+        desk.setItem(WritingDeskBlockEntity.INK, new ItemStack(ModItems.INK_VIAL.get()));
+        var player = helper.makeMockServerPlayerInLevel();
+        player.moveTo(pos.getX() + 0.5, pos.getY() + 0.5, pos.getZ() + 0.5);
+        WritingDeskMenu menu = new WritingDeskMenu(1, player.getInventory(), desk);
+        helper.assertFalse(menu.clickMenuButton(player, WritingDeskMenu.CATALOG_BUTTON_BASE + 999),
+                "A fabricated catalog index must be rejected");
+        helper.assertFalse(menu.clickMenuButton(player, WritingDeskMenu.ALL_PAGE_BUTTON_BASE + 27),
+                "A fabricated page address must be rejected");
+        helper.assertFalse(menu.clickMenuButton(player, WritingDeskMenu.extractPageButtonId(0, 0,
+                WritingDeskBlockEntity.PAPER)), "A page cannot be extracted into a material slot");
+        helper.assertValueEqual(desk.getItem(WritingDeskBlockEntity.PAPER).getCount(), 1,
+                "Forged actions must not spend paper");
+        helper.assertTrue(desk.getItem(0).is(ModItems.PAGE.get()), "Forged actions must not remove the source");
+        helper.getLevel().setBlock(pos, Blocks.AIR.defaultBlockState(), 3);
+        helper.assertFalse(menu.clickMenuButton(player, 0), "A stale menu must reject selection packets");
+        helper.assertTrue(menu.quickMoveStack(player, WritingDeskMenu.COPY_RESULT).isEmpty(),
+                "A stale menu must reject output packets");
+        helper.succeed();
+    }
+
     @GameTest(template = "empty")
     public static void catalogAggregatesKnownSymbolsAcrossCollections(GameTestHelper helper) {
         BlockPos pos = helper.absolutePos(new BlockPos(1, 1, 1));