fix: address unresolved Copilot review feedback from PRs #3, #5, #6

.github/workflows/pr-title-lint.yml

@@ -6,7 +6,7 @@ name: PR title lint
 # break anything, it just lands in the catch-all "Other Changes" section instead of a proper one.
 
 on:
-  pull_request_target:
+  pull_request:
     types:
       - opened
       - edited
@@ -21,7 +21,7 @@ jobs:
     steps:
       - uses: amannn/action-semantic-pull-request@v6
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ github.token }}
         with:
           types: |
             feat

.github/workflows/release-notes.yaml

@@ -33,12 +33,15 @@ jobs:
         id: base
         run: |
           git fetch origin --prune
-          BRANCH=$(git branch -r --contains "${{ github.sha }}" \
+          # --points-at (not --contains) requires the tag to sit exactly at a branch tip, not
+          # just be reachable from one - keeps this deterministic when a tag is reachable from
+          # several branches, and enforces the "tag the tip" invariant this workflow assumes.
+          BRANCH=$(git branch -r --points-at "${{ github.sha }}" \
             | sed 's/^[* ]*origin\///' \
             | grep -E '^[0-9]+\.[0-9]+\.x$|^main$' \
             | head -n1)
           if [ -z "$BRANCH" ]; then
-            echo "::error::Tag ${{ github.ref_name }} isn't on a recognized release branch (expected an N.N.x or main branch tip)."
+            echo "::error::Tag ${{ github.ref_name }} isn't at the tip of a recognized release branch (expected an N.N.x or main branch tip)."
             exit 1
           fi
           echo "Releasing from $BRANCH"

Archie/common/src/main/gametest/net/kernelpanicsoft/archie/gametest/internal/tests/BlockEntityNBTHolderTests.kt

@@ -1,5 +1,6 @@
 package net.kernelpanicsoft.archie.gametest.internal.tests
 
+import dev.architectury.fluid.FluidStack
 import net.kernelpanicsoft.archie.gametest.internal.EMPTY
 import net.kernelpanicsoft.archie.serialization.NBTHolder
 import net.kernelpanicsoft.archie.serialization.Sync
@@ -8,10 +9,14 @@ import net.kernelpanicsoft.archie.serialization.mapField
 import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
 import net.minecraft.nbt.CompoundTag
+import net.minecraft.world.item.ItemStack
+import net.minecraft.world.item.Items
+import net.minecraft.world.level.material.Fluids
 
 /**
  * GameTest coverage for [NBTHolder]: default values, save/load round-tripping for scalar,
- * list, and map delegated fields, and that only [Sync]-annotated fields appear in the sync tag.
+ * list, map, item, fluid, and energy delegated fields, and that only [Sync]-annotated fields
+ * appear in the sync tag.
  */
 @Suppress("unused")
 class BlockEntityNBTHolderTests
@@ -28,6 +33,14 @@ class BlockEntityNBTHolderTests
 		var syncedCounter by intField { 7 }
 	}
 
+	/** [NBTHolder] with one of each resource-storage field kind, used only by the test below. */
+	private class ResourceFixture : NBTHolder by NBTHolder.create()
+	{
+		val items by itemField(1)
+		val tank by fluidField(FluidStack.bucketAmount() * 2)
+		val energy by energyField(1_000)
+	}
+
 	@GameTest(template = EMPTY)
 	fun GameTestHelper.testFieldDefaultsAndPersistenceRoundTrip()
 	{
@@ -84,4 +97,29 @@ class BlockEntityNBTHolderTests
 		succeed()
 	}
 
+	@GameTest(template = EMPTY)
+	fun GameTestHelper.testItemFluidAndEnergyFieldsPersistMutations()
+	{
+		val holder = ResourceFixture()
+		holder.items[0].set(ItemStack(Items.DIAMOND, 5))
+		holder.tank[0].set(FluidStack.create(Fluids.WATER, FluidStack.bucketAmount()))
+		holder.energy.insert(400, false)
+
+		val tag = CompoundTag()
+		holder.saveToTag(tag)
+
+		val loaded = ResourceFixture()
+		loaded.loadFromTag(tag)
+
+		assertEquals(ItemStack(Items.DIAMOND, 5).item, loaded.items[0].getItem().item)
+		assertEquals(5, loaded.items[0].getItem().count)
+		assertEquals(FluidStack.bucketAmount(), loaded.tank[0].getFluid().amount)
+		assertTrue(loaded.tank[0].getFluid().fluid == Fluids.WATER) {
+			"Expected loaded tank to still hold water"
+		}
+		assertEquals(400L, loaded.energy.getStoredAmount())
+		assertEquals(1_000L, loaded.energy.getCapacity())
+		succeed()
+	}
+
 }

Archie/common/src/main/kotlin/net/kernelpanicsoft/archie/gui/composables/basic/Text.kt

@@ -86,11 +86,11 @@ fun Text(
                     pose {
                         scale(fontScale, fontScale, fontScale)
                         translate(x / fontScale, y / fontScale, 0f)
-                        drawString(font, text, 0, 0, color.rgb, dropShadow)
+                        drawString(font, text, 0, 0, color.argb, dropShadow)
                     }
                 } else
                 {
-                    drawString(font, text, x, y, color.rgb, dropShadow)
+                    drawString(font, text, x, y, color.argb, dropShadow)
                 }
             }
         },

Archie/common/src/main/kotlin/net/kernelpanicsoft/archie/gui/util/extension/GuiGraphics.kt

@@ -121,9 +121,14 @@ fun <T> GuiGraphics.pose(block: PoseStack.() -> T): T
 {
     val pose = pose()
     pose.pushPose()
-    val ret  = pose.block()
-    pose.popPose()
-    return ret
+    try
+    {
+        return pose.block()
+    }
+    finally
+    {
+        pose.popPose()
+    }
 }
 
 /**
@@ -134,19 +139,21 @@ fun <T> GuiGraphics.pose(block: PoseStack.() -> T): T
 fun <T> GuiGraphics.scissor(minX: Int, minY: Int, maxX: Int, maxY: Int, block: () -> T): T
 {
     enableScissor(minX, minY, maxX, maxY)
-    val ret = block()
-    disableScissor()
-    return ret
+    try
+    {
+        return block()
+    }
+    finally
+    {
+        disableScissor()
+    }
 }
 
 /** Overload of [scissor] taking the clip bounds as an [IntRect]. */
 fun <T> GuiGraphics.scissor(rect: IntRect, block: () -> T): T
 {
     val (minX: Int, minY: Int, maxX: Int, maxY: Int) = rect
-    enableScissor(minX, minY, maxX, maxY)
-    val ret = block()
-    disableScissor()
-    return ret
+    return scissor(minX, minY, maxX, maxY, block)
 }
 
 /**

Archie/common/src/main/kotlin/net/kernelpanicsoft/archie/networking/NetworkChannel.kt

@@ -177,10 +177,11 @@ open class NetworkChannel(private val id: ResourceLocation) {
      * it is decoded with [spec]'s own [kotlinx.serialization.KSerializer] (via
      * [net.kernelpanicsoft.archie.config.ConfigSpec.serializer]) rather than the reflective one
      * used for ordinary packet classes, since a `ConfigSpec` singleton isn't itself
-     * `@Serializable`. The permission check, persistence, and broadcast/rejection of the
-     * resulting value happen in `decodeDispatchData`, not in the handler registered here (which
-     * just calls [net.kernelpanicsoft.archie.config.ConfigSpec.save] again for symmetry with
-     * [configClientbound]). No-op if [spec] is already registered.
+     * `@Serializable`. The permission check, in-memory decode (mutating [spec]'s fields directly),
+     * and broadcast-or-reject all happen in `decodeDispatchData`, before the handler registered
+     * here ever runs - that handler is the one place that actually persists the result, calling
+     * [net.kernelpanicsoft.archie.config.ConfigSpec.save] to write it to disk. No-op if [spec] is
+     * already registered.
      *
      * Internal: used by [net.kernelpanicsoft.archie.config.ConfigSpec.init] to wire up
      * server/client config sync. Not part of the public packet API.

Archie/common/src/main/kotlin/net/kernelpanicsoft/archie/transfer/ArchieEnergyStorage.kt

@@ -92,12 +92,17 @@ class ArchieEnergyStorage(
 	/** Snapshots this storage's [getCapacity] and stored amount as an [NbtTag], for save/sync. */
 	override fun createSnapshot(): NbtTag = NBT.encodeToNbtTagRootless(serializer(), this)
 
-	/** Restores this storage's capacity and stored amount from a snapshot produced by [createSnapshot]. */
+	/**
+	 * Restores this storage's capacity and stored amount from a snapshot produced by
+	 * [createSnapshot]. `capacity` is clamped to non-negative and `amount` to `0..capacity` - a
+	 * malformed or stale snapshot (e.g. from before a capacity change) shouldn't be able to leave
+	 * this storage over-capacity or negative, which would otherwise wedge [insert]/[extract].
+	 */
 	override fun readSnapshot(snapshot: NbtTag)
 	{
 		val decoded = NBT.decodeFromNbtTagRootless(serializer(), snapshot)
-		this.capacity = decoded.capacity
-		this.amount = decoded.amount
+		this.capacity = decoded.capacity.coerceAtLeast(0)
+		this.amount = decoded.amount.coerceIn(0, this.capacity)
 	}
 
 	override fun update() = onUpdate()
@@ -112,8 +117,8 @@ class ArchieEnergyStorage(
 
 		override fun deserialize(decoder: Decoder): ArchieEnergyStorage
 		{
-			val capacity = decoder.decodeLong()
-			val amount = decoder.decodeLong()
+			val capacity = decoder.decodeLong().coerceAtLeast(0)
+			val amount = decoder.decodeLong().coerceIn(0, capacity)
 			return ArchieEnergyStorage(capacity).also { it.amount = amount }
 		}
 

Archie/docs/gui.md

@@ -115,7 +115,7 @@ filled with a solid color up to a `0f..1f` fraction, in any of four directions:
 
 ```kotlin
 ProgressBar(
-    progress = observeProperty("progress", 0).value ?: 0 / smeltTicks.toFloat(),
+    progress = (observeProperty("progress", 0).value ?: 0) / smeltTicks.toFloat(),
     direction = ProgressDirection.LEFT_TO_RIGHT,
 )