Safeguard block breaking and item damage against collectible behavior exceptions (#282)

[Zaldaryon]

Summary

Contains exceptions from mod collectible behavior callbacks during item damage and block breaking so the server can finish or revert the operation safely. This references #282; it does not diagnose or fix Toolsmith's underlying exception.

Collectible.WalkBehaviors covers 22 callback and getter call sites. Each failure is logged with its callback name. When the API logger is unavailable, the exception is rethrown. The protected two-argument WalkBehaviors method remains available for binary compatibility.

Block breaking preserves veto and resync behavior. The fallback checks the same block using the original solid/fluid layer selection, including lake ice in the fluid layer. It retries only while the original selected block remains. If the callback removes or replaces that block before returning false, normal removal bookkeeping and events continue. A successful fallback applies block-breaking tool wear once. If recovery fails, the existing client-revert path can resync state.

A fallback can invoke collectible callbacks again from the start. Mod callbacks that perform side effects before throwing may therefore repeat those side effects during recovery.

Failure logs are rate-limited by item, block, exception type, callback, and fallback path. Regression scenarios cover throwing behaviors, retry and veto guards, fluid-layer ice, fallback tool wear, survival drops after block replacement, callback-specific logs, and log caps.

Type

• Bug fix
• Performance
• New feature
• Refactor or cleanup
• Docs or build

Validation

• Pinned ILSpy bootstrap applied all patches after rebasing onto indev 66a8530.
• Embedded and unembedded Release builds passed with zero errors (4 and 171 warnings respectively).
• ItemBreakResilienceScenarios: 16/16 passed.
• scripts/smoke-test.ps1 passed the inventory checks and reached WorldReady.

Related issues

Refs #282

[Pixnop]

The fix works and #282 is closed by it: the branch builds, the suite is green at 22/22 here, both patches apply to a pristine baseline, hunk counters match and every hunk carries a marker. Three things need to change first, and they are all the same question: what happens to the EnumHandling a behavior wrote by ref just before it threw.

1. OnBlockBrokenWith now drops a behavior's veto (patch lines 19-27, generated Collectible.cs:730-736). The if (handled != PassThrough) block and the if (handled == PreventSubsequent) return result; line moved inside the try. The behavior writes handled before it throws, so the throw skips both checks: preventDefault stays false, the loop carries on, and control reaches the vanilla default at the bottom. A protection behavior that sets PreventSubsequent to veto a break and then throws now lets the block break, where before the player was kicked and the block stayed. Move the two checks below the catch. Your second hunk already does it that way, which is what makes the asymmetry stand out.

2. WalkBehaviors keeps the handling the throwing behavior wrote (patch lines 46-58, generated Collectible.cs:3569-3580). The catch swallows the exception, then if (handling == PreventDefault) executeDefault = false; runs on that stale value. For DamageItem the default action is the entire durability update, so a behavior that sets PreventDefault and then throws makes the tool take no damage at all, silently, on every break from then on. That is the exact shape of #282: Toolsmith's OnDamageItem has to set PreventDefault, otherwise vanilla durability would stack on top of its own tinkering. The description says the catch lets "item damage reduction and slot clearing on zero durability complete cleanly", and for the case the issue is about it does the opposite. Reset handling = EnumHandling.PassThrough in the catch, or decide the other way and say so in the marker.

3. The server fallback re-breaks from inside the catch (ServerSystemBlockSimulation patch lines 443-451, generated :883-891). Two separate problems. The retry has no guard of its own, so when the original exception is deterministic and came from that same path, an empty hand on a block whose BlockBehavior.OnBlockBroken throws for instance, and that loop is not one this PR wraps, the second call throws the same exception, it escapes TryModifyBlockInWorld and the player is kicked anyway, this time after the drops already spawned. And the guard GetBlock(pos).BlockId != 0 reads the Default layer, which returns the fluid layer when the solid layer is empty. Break sand under still water with a collectible path that throws after SpawnDropsAndRemoveBlock has run: the solid layer is air, Default hands back the water, the fallback calls OnBlockBroken a second time and the drops are duplicated.

Not blocking, but worth the same pass:

• Hunk 3 has no coverage at all. Revert only that try/catch, rebuild, run the two new scenarios: still 2/2 green, because the two Collectible catches swallow everything before it gets there. Reaching it needs a throwing BlockBehavior or a throwing GetDrops.
• Neither faulty behavior touches bhHandling before throwing, so both scenarios exercise the one case where the default still runs, and neither reads the pickaxe afterwards. A scenario that sets bhHandling = PreventDefault before the throw and asserts GetRemainingDurability actually dropped would have caught point 2.
• The behaviors are appended to pickaxe.CollectibleBehaviors on the shared registry object and never removed. Both scenarios share one server, so whichever runs second carries the first one's faulty behavior too. Restore the array in a finally.
• isDedicatedField?.SetValue(server, true) quietly does nothing the day that compiler-generated field name changes, and both scenarios then pass with all three safeguards reverted. Assert.NotNull on the FieldInfo is one line.
• Logging is unbounded now. The player is no longer kicked, so a deterministic mod exception repeats on every break, two Logger.Error calls with a full stack trace each time. Someone mining at five blocks a second writes ten traces a second for as long as they keep going. Log once per behavior type and collectible, or rate limit.
• api?.Logger?.Error(...) is a silent swallow whenever api is null, and it is only set by OnLoadedNative. OnBlockBrokenWith has world in scope and can log through world.Logger.

One design question, asked as a question rather than a condition. ServerMain.DispatchClientPacket_mainthread is already ours and is already the single place where a packet-handling exception decides whether to kick. A policy there would be one hunk in code we own, rather than three inside vanilla method bodies, two of them in the vsapi fork that every version bump has to carry forward. What pushed you towards the three?

[Zaldaryon]

Pixnop, I addressed your requested changes in f85a9c5 and rebased onto current indev at 38c2da1. The patch preserves break vetoes, resets failed callback handling without overriding earlier successful PreventDefault, contains fallback exceptions, and retries only while a solid block remains. The scenarios now cover durability loss and zero-durability slot clearing, both veto modes, callback ordering, deterministic and recoverable fallback failures, and fluid remaining after solid-layer removal. They also restore behavior arrays in finally and assert the dedicated-server reflection field exists.

Verification on the rebased branch: dotnet build VintageStory.slnx -c Release and the embedded build succeeded with existing repository warnings, the focused scenario suite passed all 8 cases, and bash scripts/smoke-test.sh reached RunGame with 21 console commands verified. Please re-review.

[Zaldaryon]

Independent verification pass on f85a9c5, plus an answer to the design question, which I left hanging last round.

I re-derived the generated code rather than reading the patch file, since that is what actually runs. Applied both patches to a clean .baseline and read the results.

Blocker 1 is closed. Collectible.cs:744-757: the try wraps only behavior.OnBlockBrokenWith(...), and both if (handled != EnumHandling.PassThrough) and if (handled == EnumHandling.PreventSubsequent) return result; sit below the catch. A behavior that sets PreventDefault and then throws keeps its veto, and BlockBreak_Should_PreserveBreakVeto_When_CollectibleBehaviorThrowsAfterVeto asserts exactly that, including that the sentinel behavior after it still runs and durability does not move.

Blocker 2 is closed. Collectible.cs:3590-3594: the catch resets the local handling to PassThrough before the two checks, and it only touches that local, so an earlier behavior's successful PreventDefault still suppresses the default action. That is the "without overriding earlier successful PreventDefault" part of my last comment, and BlockBreak_Should_ClearToolSlotWhenFailedDamageBehaviorReachesZeroDurability covers the zero-durability case that is the shape of #282.

Blocker 3 is closed, both halves. ServerSystemBlockSimulation.cs:887-898: the retry is guarded by BlockLayersAccess.Solid rather than the Default layer, so breaking sand under still water no longer re-enters OnBlockBroken after SpawnDropsAndRemoveBlock has run, and the retry has its own catch, so a deterministic throw from that same path can no longer escape TryModifyBlockInWorld and kick the player after the drops already spawned. BlockBreak_Should_NotRetryAfterSolidLayerWasRemoved and BlockBreak_Should_CompleteFallbackAfterOneShotBlockBehaviorException pin both.

The non-blocking items are in as well: StratumLoggedBehaviorExceptions is a ConcurrentDictionary keyed on the behavior type, so a deterministic mod exception logs once per collectible instead of ten stack traces a second, and OnBlockBrokenWith logs through world.Logger rather than api?.Logger. WalkBehaviors still has only api in scope, so that one keeps the nullable logger and StratumLogBehaviorException returns early when it is null. That is a real gap, not an oversight: api is set by OnLoadedNative and WalkBehaviors has no world handle to borrow. The scenarios restore the behavior arrays in finally and assert the dedicated-server reflection field is not null.

Gate on my side, at f85a9c5: dotnet build VintageStory.slnx -c Release -p:EmbedPatchedFiles=true succeeds with 0 errors and the 4 known NU1904 warnings. The 8 ItemBreakResilienceScenarios pass. The full suite is 29 tests, 28 pass and 1 fail: RandomTickScenarios.FarPlatform_Should_StayUntouched_When_DefaultsActive reports far platform received random ticks: 1/1024 converted, which is a random-tick timing scenario with nothing to do with this PR, and it passes in isolation in 16s. smoke-test.ps1 reaches RunGame with no fatal errors.

On the design question, which deserves a real answer rather than a gesture at the alternative.

A policy in DispatchClientPacket_mainthread is one hunk and I would take it if the three sites wanted the same thing. They do not. That method can only answer one bit, kick or do not kick, and the three cases each need something the other two must not do.

OnBlockBrokenWith must keep a veto the throwing behavior already wrote and must not run the vanilla default a second time. WalkBehaviors must drop a failed behavior's PreventDefault so the default action still runs, while leaving an earlier behavior's PreventDefault standing. The retry in ServerSystemBlockSimulation must not fire at all when the solid layer is already empty, or the drops duplicate. None of those is expressible as "do not kick", and each one is a state question about code that has already half-run: the packet handler has mutated the world by the time the exception surfaces, so the recovery has to be local to the operation that failed.

The other cost is portability. Two of the three live in the vsapi fork, and a vsapi hunk has to survive every version bump with the counter arithmetic intact, while a ServerMain hunk is code we own. I took the version that has to be re-derived per release over the version that has to be re-derived per vanilla method. That is a judgement call and yours to overrule: if you would rather have the single choke point and accept "log and continue" as the uniform policy, I will fold the three into one and lose the per-case recovery.

BlockBreak_Should_NotRetryAfterSolidLayerWasRemoved and BlockBreak_Should_ContainRepeatedBlockBehaviorException are the two I would most like a second pair of eyes on, since they are the only coverage of hunk 3 and of the log bound.

Re-requesting review.

[Zaldaryon]

Linux verification, since the target platform is a server and I had only tested on Windows.

Bootstrapped Stratum from scratch in WSL 26.04 on ext4: decompiled VintagestoryLib.dll with the pinned ilspycmd (1009 files), cloned the five forks, applied patches/, 0 patch failures. Then both build passes, because the embed pass reads the sibling outputs by raw path and cannot resolve them on a tree that has never been built.

dotnet build VintageStory.slnx -c Release                              0 errors
dotnet build VintageStory.slnx -c Release -p:EmbedPatchedFiles=true    0 errors
dotnet test  --filter ItemBreakResilienceScenarios    8 passed, 0 failed
dotnet test                                          29 passed, 0 failed
bash scripts/smoke-test.sh   server reached RunGame, 21 console commands verified

I re-read the generated code on Linux rather than trusting the Windows bootstrap, since the two come from separate decompiles. Collectible.cs:750 logs through world.Logger inside the catch and both if (handled != ...) and if (handled == EnumHandling.PreventSubsequent) return result; sit below it at 753 and 759. Collectible.cs:3593 resets handling = EnumHandling.PassThrough in the WalkBehaviors catch before the two checks. ServerSystemBlockSimulation.cs:889 guards the retry on BlockLayersAccess.Solid, and the fallback's own catch is at 894. Same three closures, independently produced.

The full suite is 29 of 29 on Linux. On Windows I got 28 of 29, with RandomTickScenarios.FarPlatform_Should_StayUntouched_When_DefaultsActive reporting far platform received random ticks: 1/1024 converted. It passes here and it passed in isolation there, so that was a load artifact of running 8 item-break scenarios alongside it on Windows, not a flake in the code. Worth recording so nobody chases it.

Re-requesting review. The two scenarios I would still like a second pair of eyes on are unchanged: BlockBreak_Should_NotRetryAfterSolidLayerWasRemoved and BlockBreak_Should_ContainRepeatedBlockBehaviorException, since they are the only coverage of hunk 3 and of the log bound.

[Zaldaryon]

The fixes from the review are on the current head, f85a9c5. The recorded verification for this head includes a clean Release build, 29 passing Atlas scenarios, and a passing smoke run with 21 console probes. The eight focused item-break scenarios cover the veto, durability, fallback, and fluid-layer cases; the shared behavior fixtures are restored, and logging is rate-limited. I have refreshed Pixnop's review request and would appreciate a re-review of this head.

[Zaldaryon]

Pixnop's requested blocking fixes and follow-ups are on ae05c88.

Blocking fixes

• OnBlockBrokenWith evaluates EnumHandling after the catch, preserving a behavior's veto when that behavior throws.
• WalkBehaviors resets failed handling to PassThrough, so a failed OnDamageItem cannot suppress the vanilla durability update.
• The server fallback checks the solid layer and catches a second failure, so fluid-only positions are not retried and a deterministic fallback exception does not escape the break handler.

Non-blocking follow-ups

• Scenarios cover throwing and one-shot BlockBehavior failures, fallback handling, fluid layers, and repeated failure logs.
• Shared behavior arrays are restored in finally; the dedicated-server reflection field is asserted; durability decrement and slot clearing are checked.
• Behavior exceptions are logged once per collectible and behavior type. Server fallback errors are now capped per collectible or block code, exception type, and failure path. OnBlockBrokenWith logs through world.Logger.

Design question
I kept the catches in the break action pipeline. A packet-level catch runs after partial side effects and cannot safely complete item durability, drops, block removal, chunk accounting, and inventory sync. The break pipeline can contain the failing callback while preserving the action's veto and fallback checks.

Verification on this head

• make build: both Release passes succeeded with zero errors. Existing warnings include NU1904 for System.Drawing.Common 4.5.0 and decompiler warnings.
• make smoke: the server reached WorldReady and all 21 console probes passed.
• make scenarios: 55/55 scenarios passed.
• The server patch applies to a pristine baseline, and git diff --check passed.
• GitHub currently reports no checks on this branch.

Please review the new head, ae05c88.

[Zaldaryon]

Pixnop, I rebased fix/issue-282-item-break-exception onto current indev at 05a7627. The new head is 13a6864. The requested veto and durability handling, contained fallback, solid-layer guard, and bounded logging remain covered by the regression scenarios.

On this head, bootstrap applied every patch. Both Release builds succeeded with zero errors. The focused item-break scenarios passed 8/8, the full Atlas suite passed 57/57, and smoke-test.ps1 reached WorldReady without fatal errors. Existing dependency and decompiler warnings remain. GitHub reports no CI checks on this branch.

Please re-review head 13a6864.

[Pixnop]

All nine items from the last round are handled, and I checked them by reverting each fix: the handling checks sit below the catch, a failed handling resets to PassThrough, the fallback has its own catch and reads the solid layer, the scenarios set bhHandling, assert durability and restore the behavior arrays, and the reflection lookup is asserted. 57/57 here, the eight new scenarios included. Three things remain, all in the fallback and the log cap.

1. A break that fails twice still returns true. When the first attempt throws and the fallback throws too, the inner catch logs and execution falls through to BlocksRemoved++, DirtyForSaving and return true. HandleBlockPlaceOrBreak only reverts on false; on true it runs TriggerNeighbourBlocksUpdate and DidBreakBlock. The client had already removed the block locally before sending the packet, so the player sees air where the server and everyone else still have a block, until a relog, and DidBreakBlock fires for a block that is still there. Your own ContainRepeatedBlockBehaviorException shows it: the block is still granite at the end, and a counter on DidBreakBlock reads 2. Returning false after a failed fallback lets the existing revert path resync the client.

2. The retry guard is "solid layer not empty", not "this block is still here". Several vanilla blocks end a successful break by placing another solid block at the same position: tall grass cut with a knife, reeds, layered slow-dig blocks, TransformBreak. If something throws after the block's own OnBlockBroken returned, the guard sees a solid block and runs block4.OnBlockBroken again with the stale block4, so the drops spawn twice. Comparing the id at the position with block4.Id closes it.

3. The log-once keys leave out what tells failures apart, and never expire. Server side the key is the held item code (the block code with an empty hand), the exception type and the fallback flag. With a tool in hand the block is in neither the key nor the message, so a NullReferenceException in one mod's block hides the same exception type in another mod's block for the rest of the run. Collectible side the key is the behavior type alone, shared by OnBlockBrokenWith and the 22 WalkBehaviors callers, and the one line written says "during WalkBehaviors" whatever the callback was. Add the block code and the callback name to the keys, or use a time window per key with a suppressed count.

Tests, in the same pass if you can:

• All eight scenarios run with a Creative player, and Creative skips GetDrops and the drop spawn, so duplicated drops are only covered through invocation counts. One Survival scenario counting spawned item entities would pin point 2.
• The Collectible-side cap has no coverage: with its TryAdd removed, all eight still pass.

Two things for the description. WalkBehaviors backs 22 callbacks, getters included, so all of them now swallow and fall through to the default, not only item damage. And #282's root cause, the Toolsmith exception that does not happen on a vanilla server, is contained here, not explained. I would keep that part open in an issue of its own.

[Zaldaryon]

Pixnop's latest findings are addressed in commit 296458e.

• The fallback returns failure if its callback throws, allowing the existing client-revert path to resync the block.
• The retry checks that the original solid block id is still present.
• A Survival scenario checks drops after block replacement. Logging tests cover callback names and rate limits.
• The PR description now states that this contains server-side failures, does not fix Toolsmith's underlying exception, covers all 22 WalkBehaviors call sites, and references NullReferenceException in Toolsmith OnDamageItem when a tinkered tool breaks, likely a conflict with Stratum source patches #282 instead of closing it.

Validation on the rebased implementation: pinned ILSpy bootstrap applied every patch; embedded and unembedded Release builds passed; ItemBreakResilienceScenarios passed 11/11. The smoke test reached WorldReady before final test-only edits. The only diff-check findings are blank context lines required by the nested unified patch files.

Please review the new head.

[Zaldaryon]

Rebased and pushed current head c9a9fd9 onto indev c5c3aee after #347 merged. The latest run applied all patches, passed both Release builds, passed ItemBreakResilienceScenarios 11/11, and reached WorldReady in the smoke test. GitHub reports no checks for this branch. Please review the current head.

[Pixnop]

All three points from the last round are fixed, and each fix has a scenario that goes red when I revert it: a double failure now returns false and DidBreakBlock stays silent, the retry guard compares block ids, and the log keys carry the block and the callback. The veto return in ae9add5 and the restored two-parameter WalkBehaviors are good catches too. 61/61 here. Three things before merge, then a few notes.

1. BlockBreak_Should_NotRetryOldBlockOrDuplicateDropsAfterReplacement is flaky. It failed in 6 of 33 runs of the class here, always at line 361 ("must not spawn its drops twice"). In every failure the item count went down, no new entity id appeared and the behavior ran once. The Survival player stands 1.58 blocks from the block centre, the pickup radius is 1.5, and the random spawn velocity sometimes carries a stone into range during the 5 ticks. So the scenario reports a duplicate when a stone was picked up, and a pickup could just as well hide a real duplicate. Moving the player a few blocks away, or counting spawns with OnEntitySpawn instead of live entities, fixes both.

2. The retry guard reads the solid layer, but block4 can come from the fluid layer. Lake ice and glacier ice (BlockLakeIce: fluid layer, solid sides) are taken from the fluid layer at the top of TryModifyBlockInWorld, so the guard at patch line 456 is always false for them. If the first attempt throws before the ice is removed, there is no fallback, nothing is left to throw, and the method returns true: the point 1 symptom again, a desynced client and a DidBreakBlock for a block that is still there. Reading the layer block4 was taken from closes it.

3. Before OnLoadedNative, a behavior exception now disappears without a trace. WalkBehaviorsCore logs through api?.Logger, and api is only set in OnLoadedNative, which the server reaches after the mod handler has run. A mod that calls GetMaxDurability, GetMiningSpeed and the like from AssetsLoaded, AssetsFinalize or StartServerSide while a behavior throws gets the default value and no log line anywhere. Vanilla let that exception reach the mod loader. Rethrowing when there is no logger keeps the old behavior for that window.

Notes, not blocking:

• The retry replays Block.OnBlockBroken from the top, so behaviors ahead of the thrower run twice per packet. A behavior that spawns the drops and then throws, with the same block id still in place, passes the guard and gets its drops a second time: a throwaway scenario showed it in 3 of 3 runs. That is the price of retrying, and a sentence in the description would help mod authors.
• With a tool in hand, when the block side throws once and the fallback succeeds, the block breaks and the tool takes no damage.
• Since ae9add5 the server acts on the return value of OnBlockBrokenWith, which vanilla discards. That is right for a real veto, but a mod that removes the block itself and then returns false now skips BlocksRemoved, the neighbour update and DidBreakBlock. Checking that the block is still there before returning false would cover it.
• No scenario pins the block code or the item code in the server-side log key: removing either leaves all of them green.
• On the Collectible side, every call from a mod subclass is logged as "during WalkBehaviors", and GetTemperature and AfterGetTemperature share one key.

[Zaldaryon]

Thanks for the review. I updated the recovery path and added regressions for the reported cases:

• Moved the survival player three blocks from the broken block so dropped items are not picked up before the count assertion.
• Made the retry guard use the same solid/fluid layer selection as the initial lookup, with a lake-ice case for ice stored in the fluid layer.
• When OnBlockBrokenWith returns false, the server now aborts only if the original selected block remains. If the callback already removed or replaced it, removal bookkeeping and events continue.
• A successful fallback now applies block-breaking tool wear once. The PR description also notes that callbacks can run again from the start during fallback, so side effects before a throw can repeat.
• WalkBehaviorsCore rethrows when the API logger is unavailable. Paired temperature callbacks have distinct log keys, and the protected two-argument WalkBehaviors signature remains intact.
• Added log-key coverage for item and block codes while preserving the rate cap.

Validation on rebased indev 66a8530: ItemBreakResilienceScenarios passed 16/16; both Release builds passed with zero errors (171 warnings unembedded, 4 embedded); smoke test passed inventory checks and reached WorldReady.

[Pixnop]

All three points from the last round are fixed, and each fix has a scenario that goes red when I revert it. The Survival scenario now breaks a block three blocks away: I ran the class 35 times on 818af9b and 34 runs were clean. The one bad run died on the chunkdbthread AllOnlinePlayers crash that #352 addresses, not on this scenario. Putting the offset back to +1 brings the flake back in 11 of 30 runs, with the same "must not spawn its drops twice" assertion. StratumGetSelectedBreakBlock gives the initial pick, the veto check and the retry guard one selection rule, and BlockBreak_Should_FallbackWhenSelectedBlockIsInFluidLayer goes red with the old solid-layer guard. WalkBehaviorsCore rethrows when api?.Logger is null, pinned by BlockBreak_Should_RethrowCollectibleBehaviorFailureWhenApiLoggerIsUnavailable. The notes are in too: fallback tool wear (exactly one point), the false return ignored once the block is gone, item and block code pinned in the server log key, and separate temperature keys. Removing any of them turns its scenario red. 66/66 on the full suite, 0 errors, and the same 171 and 4 warnings as indev.

Approving. Two things in the fallback are left, both for a follow-up or a last push here, your call:

• The fallback catch returns false even when the fallback already broke the block (patch line 478, generated ServerSystemBlockSimulation.cs:907-911). Since 818af9b the fallback try also runs GetDamagedBy and DamageItem (patch 466-471), after block4.OnBlockBroken has removed the block and spawned the drops. If the wear step throws, the method returns false. HandleBlockPlaceOrBreak then resyncs the client to air but skips TriggerNeighbourBlocksUpdate (sand or gravel above stays floating, attached blocks are not re-checked) and DidBreakBlock, and BlocksRemoved and the hotbar broadcast are skipped as well. Behavior exceptions cannot get there because WalkBehaviorsCore swallows them, but a DamageItem or GetDamagedBy override, a Harmony patch or the default action can. One fault is enough: a modded tool whose OnBlockBrokenWith override calls a throwing DamageItem before base throws before removal, the fallback breaks the block, and the same DamageItem throws again. The same throw on a break that needs no fallback returns true with full bookkeeping. Indev kicked the player for any throw here, so this is not worse than before, only inconsistent with the rest of the recovery. It is the veto note from last round again: return false in that catch only while StratumGetSelectedBreakBlock(blockPos).BlockId == block4.BlockId, as at patch line 445, or give the wear step its own try that only logs.

• A block that fails twice stays, and can be mined again for two sets of drops each time (same catch). Take a block whose break path spawns items, leaves its id in place and then throws every time. Vanilla already has the first half: SpawnDropsAndRemoveBlock calls the block entity's OnBlockBroken before GetDrops, and BEToolrack (the anvil, forge and helve hammer too) spawns its held stacks without clearing them. Add a mod BlockBehavior.GetDrops on that block that always throws. Each break packet spawns the tools, throws, passes the id guard, spawns them again, throws, and returns false. The client is reverted, the block and its inventory stay, and the player mines it again at normal speed, with no tool wear and two log lines per item, block and exception type for the whole run. On indev the first throw kicked the player on a dedicated server, so it was one set per reconnect. Returning false here was my request in round 2, so this follows from that choice. It still wants either a brake, for example remembering (player, position, block id) after a failed fallback and skipping the retry or rethrowing to the kick the next time the same key fails twice, or a sentence in the description stating it as an accepted risk.

Smaller:

• The description says "A fallback can invoke collectible callbacks again from the start". The fallback calls block4.OnBlockBroken directly and does not call OnBlockBrokenWith again, so what repeats is the block side: block behaviors ahead of the thrower, the block entity's OnBlockBroken, GetDrops. Mod authors will read that sentence, so it should name those.
• Collectible.cs.patch is not what scripts/extract-patches.sh produces. The GetTemperature and AfterGetTemperature change sits in two hand-split hunks (@@ -3324,7 and @@ -3332,6, patch lines 326 and 336) where git diff -U5 emits a single @@ -3324,18 +3352,20. git apply takes it and the generated code is right, but the next extract run on that file will rewrite those hunks in someone else's diff. Re-running extract-patches fixes it.

[Zaldaryon]

Rebased onto indev at a3966e2 to resolve the conflict with PR #353 in ServerSystemBlockSimulation.cs.patch.

Validation on commit 4dffb22:

• scripts/bootstrap.ps1 applied all patches with 0 errors.
• Both Release build passes (VintageStory.slnx and with -p:EmbedPatchedFiles=true) completed with 0 errors.
• ItemBreakResilienceScenarios: 16/16 passed.
• RandomTickPositionScenarios: 3/3 passed.
• OnlinePlayersCacheScenarios: 3/3 passed.
• scripts/smoke-test.ps1: passed inventory privacy and server reached WorldReady.