fix(engine): emit the legacy tuple for PerSource damage thresholds

[matthewevans]

Follow-up to #7476, addressing CodeRabbit's Major finding on that PR. #7476 merged
through the queue before the finding could be addressed there.

The gap

#7476 gave damage_amount a scope axis and added deserialize_damage_amount_compat
so pre-axis data still loads. The migration was one-way — nothing preserved the
write side.

PerSource serialized as {"comparator":"GE","threshold":6}, where every build
predating the axis emits and expects ["GE", 6]. Eliding the PerSource default
keeps the object minimal, but it does not preserve the shape, so two comments
claiming pre-axis cards "serialize byte-identically" were false — and the assertion
15 lines below one of them pinned the object form.

Why it matters

The shape reaches persistence with no version gate to reject it early:

• PersistedSession.state is a PersistedGameState (crates/server-core/src/persist.rs:23)
• GameObject.trigger_definitions is a serialized field

So a build carrying the axis writes rows an older build fails to deserialize, taking
the whole session restore with it. The three consumers are the ones already named on
deserialize_damage_amount_compat: browser IndexedDB saved games, the phase-server
session store, and the shared card fixture.

The fix

serialize_damage_amount_compat, the write-side counterpart. PerSource emits the
legacy tuple; only WholeEvent emits the object — and no pre-axis reader can
represent that pole anyway. Information-lossless, and it makes the byte-identical
claim true rather than deleting it.

Modeled on the serialize_multi_target_min / deserialize_multi_target_min pair in
the same file, which collapses a Fixed variant to a bare int the same way.

Also pins #[serde(default)] on DamageAmountThreshold::scope with an object row
carrying no scope key — CodeRabbit's nitpick. That shape is reachable rather than
hypothetical: any build between the axis landing and this serializer wrote exactly it.

Verification

Tested on a worktree carrying the identical edit:

• cargo fmt --all -- --check clean
• full engine suite 24,294 / 24,294 passed
• revert-to-red: removing serialize_with fails the assertion with exactly the
  break in question —
  left: Object {"comparator": "GE", "threshold": 6} vs right: Array ["GE", 6]

Summary by CodeRabbit

• Bug Fixes

  • Improved compatibility when saving and loading damage amount thresholds.
  • Legacy tuple-formatted thresholds are now supported alongside the current object format.
  • Scopeless threshold objects now default correctly, while whole-event thresholds retain their object representation.

• Tests

  • Added coverage verifying compatibility in both serialization directions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 16c6a350-3b2e-4265-a805-0c45999ef96c

📥 Commits

Reviewing files that changed from the base of the PR and between 566888a and dab2d89.

📒 Files selected for processing (1)

• crates/engine/src/types/ability.rs

Included review availability: Your plan includes up to 2 reviews per rolling hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The damage_amount field now uses a compatibility serializer. PerSource values serialize as legacy tuples, while other values retain object serialization. Tests cover legacy input, scopeless objects, and WholeEvent.

Changes

Damage amount compatibility

Layer / File(s)	Summary
Compatibility serializer and field wiring crates/engine/src/types/ability.rs	The damage_amount field uses a serializer that emits PerSource thresholds as ["GE", threshold] and preserves object serialization for other values.
Serialization and deserialization coverage crates/engine/src/types/ability.rs	Tests verify legacy and object input, default PerSource scope for scopeless objects, tuple serialization for PerSource, and object serialization for WholeEvent.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to dab2d

This localized serialization fix is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

Suggested reviewers: jacobwoodson, mike-thedude

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: serializing PerSource damage thresholds as legacy tuples for compatibility.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch engine/damage-amount-legacy-tuple-serializer

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Generated for head dab2d89cae138e5af62da23fa54b685c46289f4f.

Parse changes introduced by this PR

✓ No card-parse changes detected.

[matthewevans]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.