Docstrings: core spells - #89
Merged
Merged
Conversation
Rewrite every docstring in src/osrlib/core/spells.py for a developer who reads the published reference and never opens the source. Every model field, constant, and public function now says what it does, what you need first and where to get it, what to call next, and when not to use it, and every public function carries an example that runs under the docs examples harness. Claude-Session: https://claude.ai/code/session_01GL26QnA6dCrvUc3WmhzFSa
Move every model's field prose out of its Attributes: section and into a PEP 224 attribute docstring, so the rendered field rows carry the prose instead of showing bare names under a table. Correct four claims against the code: cast_from_scroll now says which steps run at the scroll's caster level and which at the reader's own, SpellTemplate.intro says the mode's prose is the full text rather than a repeat, _resolve_dispel keeps only what the dispellable flag supports, and TurnUndeadResult.hd_pool says the second roll happens on a destroy as well as a turn. Drop the numeric count of dual-page spells, say the catalog's id order is a property of the shipped data, and reflow one broken sentence. Claude-Session: https://claude.ai/code/session_01GL26QnA6dCrvUc3WmhzFSa
Rewrite every `#` comment in spells.py the way the docstrings were rewritten: no dashes, no semicolon splices, one idea per sentence. Each "pinned" becomes the choice it stood for, stated as what the code does, so a maintainer reads the rule rather than a marker. The import-direction note drops its reference to an earlier review and keeps the constraint. Claude-Session: https://claude.ai/code/session_01GL26QnA6dCrvUc3WmhzFSa
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A reader who has only the published reference can now prepare a caster's spells, cast one, read a scroll, disrupt a declared cast, pop a mirror image, and turn undead without opening the source. Every model field in
osrlib.core.spellsused to render as a bare name; each one now carries its own docstring saying what it holds, where it comes from, and what reads it, so the prose lands on the field row the renderer generates. Every public function carries a runnable example with its real output.Docstrings I corrected because the code disagreed with them:
cast_from_scroll(src/osrlib/core/spells.py:2328) said the spell resolves at the levelminimum_caster_levelgives. Only the resolution does.validate_castis called with the raw reader, so the legality checks run at the reader's own level, and that splits two things a reader would expect to follow the scroll: the number of targets a mode demands, and the reach of a range that grows per level. The docstring now describes both halves.SpellTemplate.intro(src/osrlib/core/spells.py:802) was documented as the lead-in above a multi-usage page and empty otherwise. Every spell has one, and on a single-usage page it is the opening of the description while the mode'sproseis the full text, which usually runs longer._resolve_dispel(src/osrlib/core/spells.py:3141) claimed magic items are exempt from dispelling. Nothing in this module implements that. The docstring now states only what thedispellableflag supports, and that the flag defaults to False.TurnUndeadResult.hd_poolsaid the second roll happens only when a kind was turned. It happens when any kind came out turned or destroyed.SpellCatalog.spellsandby_listsaid the order is id order. The model's validator checks id uniqueness only, so that is a property of the shipped data; a catalog you build yourself keeps the order you gave it.One code defect, which I did not fix. A scroll read validates at the reader's level and resolves at the scroll's. A 6th-level magic-user reading a magic missile scroll is required to supply three targets, because
validate_castcomputes the missile count from the reader's level, and the resolution then strikes all three, even thoughminimum_caster_levelputs the scroll at level 1. Two spells whose printed range grows per level have the same split in the range check.Every example runs under the docs examples harness. None is marked
no-run, and no member is undocumented on purpose.https://claude.ai/code/session_01GL26QnA6dCrvUc3WmhzFSa