Stop recording an NMR for a forced full disband - #1315
Open
johnpooch wants to merge 1 commit into
Open
Conversation
A nation with units and no supply centres must disband every unit it has. _adjustment_options emits a Disband option per unit whenever required_disbands() > 0, so has_possible_orders stayed True and _set_orders_outcome marked orders_outcome=nmr on zero submitted orders. The engine's EnforceDisbandLimits fills the disbands in regardless, so the resolution is identical either way, but the player was shown an NMR badge on their way out of the game and burned an NMR extension that pushed the deadline out for everyone. required_disbands equals the unit count exactly when the nation holds no supply centres, so derive the forced set from the counts already used for actionable_units and exclude those nations from the orders outcome, from NMR extensions, and from the deadline warning that would otherwise promise an extension they no longer get. A partial disband, where which units go is a real choice, keeps counting as an NMR. Leaving orders_outcome null matches how a phase state with no possible orders is already treated, so the phase drops out of commitment rating via the orders_outcome__isnull=False filter in get_rated_outcomes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RjyUirJePgFH9Q2zPWSp5Z
Contributor
|
Warning WIP limit exceeded. There are now 8 open pull requests — the project target is 5 or fewer. Please close or merge an existing PR before continuing with this one. |
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
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.
What this PR does
Fixes #1310. A player with units and no supply centres must disband every unit they have, and the engine fills those disbands in for them — but they were still recorded as having NMRed, shown an NMR badge on their way out of the game, and charged an NMR extension that pushed the deadline out for everyone.
_adjustment_optionsemits a Disband option per unit wheneverrequired_disbands() > 0(service/adjudicator/options.py:485-505), sohas_possible_ordersstayedTrueand_set_orders_outcomemarkedorders_outcome=nmron zero submitted orders.EnforceDisbandLimits/ApplyCivilDisorder(service/adjudicator/engine.py:154-168) fill the missing disbands in anyway, so the board resolves identically whether or not the player submitted anything.required_disbands()ismax(0, units - supply_centres)(service/adjudicator/types.py:1121-1124), so it equals the unit count exactly when the nation holds no supply centres. NewPhase.nations_with_forced_ordersderives that set from the same countsactionable_unitsalready walks, and three call sites now skip those nations:_set_orders_outcome— leavesorders_outcomenull rather thannmrmembers_that_require_nmr_extension— no extension consumedsend_deadline_warnings— no "you'll lose an extension" warning, which would no longer be trueA partial disband (3 units, 1 supply centre, where which two units go is a real choice) still counts as an NMR. Movement and Retreat phases are untouched.
Leaving
orders_outcomenull matches how a phase state withhas_possible_orders=Falseis already treated, so the phase drops out of commitment rating via theorders_outcome__isnull=Falsefilter inget_rated_outcomes(service/user_profile/commitment.py:49-53).Two things worth a reviewer's opinion
orderStatusandPhaseGuidancederive fromhas_possible_orders/ the SC-vs-unit delta, so a forced-disband player still sees "Disband 2 units" and an "Orders required" badge while the backend now asks nothing of them. Surfacing "forced" to the client means a serializer field plus codegen across both schemas, which felt like scope beyond the issue — happy to do it here or in a follow-up._retreat_optionsappends a Disband option for every dislodged unit regardless of whether a legal retreat exists (service/adjudicator/options.py:396-405), so a player whose dislodged units all have nowhere to go is in the identical position. Issue Forced full disband in an Adjustment phase is recorded as an NMR #1310 scopes to Adjustment, so I left it — but it's probably worth its own issue.Checklist
/review-pragainst this PR in Claude Code and addressed (or responded to) its findingsSeven tests added across
TestSetOrdersOutcome,TestNMRExtensionsNothingToOrderandTestSendDeadlineWarnings. The four that pin the bug fail onmainand pass here; full backend suite is green (2356 passed, 8 skipped).🤖 Generated with Claude Code
https://claude.ai/code/session_01RjyUirJePgFH9Q2zPWSp5Z
Generated by Claude Code