Feature/command fully randomize

[HansGR]

This feature modifies the processing of the -com flag to allow two new modes: fully random commands and fully random unique commands. These modes can replace the common commands Fight, Magic, and Item with skill commands (steal, rage, etc.) with a probability determined by the flags.

• -com fr F.M.I: Fully randomized commands. F, M, and I are integers in the range {0, 100} and correspond to the percent probability that any given character has the common commands Fight, Magic, and Item.
• -com fru F.M.I: Fully randomized unique commands. After rolling each characters' common commands, the algorithm distributes unique commands to the unfilled command slots via a snake draft, replenishing when it runs out.

This enables several challenge modes: for example, 0.100.100 makes all characters like Gau (no fight, two commands); 33.33.33 gives each character approximately one of {Fight, Magic, Item} on average, with the rest backfilled with skills; 0.0.0 would give characters skills only (no magic or items).

Notes on the implementation:

1. -com fru is not truly unique: it is not possible to have truly unique commands as there are too many slots to fill. However, this minimizes repeats to the extent possible.
2. These uses respect excluded commands.
3. Command order is sorted to preserve the order: "Fight" --> "Skill(s)" --> "Magic" --> "Item".
4. When Natural Magic User = random, the algorithm attempts to find a character who is capable of using magic. If there are none, the natural magic use is still granted (as spells can still be used out of battle so it may still be useful).
5. Morph can only be granted to one character because of its mechanics.
6. Gau's probability of getting Fight is always half that of the other characters (e.g. if F = 80, Gau's probability of having Fight is 40%). This custom handling is intended to preserve Gau's identity a bit.
7. Gogo and Umaro are not affected. Gogo can equip any commands that the party has unlocked, as per usual.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[wrjones104]

PR #170 Review — Feature/command fully randomize

1. PR Summary & Grade

• Overall Grade: B (Minor Tweaks Needed)
• Executive Summary: This PR adds probability-driven command randomization (-comfr/-comfru/-compr/-compru), makes -com composable with those flags, consolidates the six -recN exclusion flags into a single dot-list -rec (keeping the legacy forms), and fixes the Skills/Espers submenu so esper-capable characters aren't locked out when they lack the Magic command. The implementation is well-structured, heavily commented, and backed by strong invariant tests; CI is green on Python 3.9–3.13. It earns a B rather than an A because declaring Morph as a probability command can hand Morph to multiple characters, violating the "only one character gets Morph" invariant the rest of the code enforces, and the PR description's Gau/Fight rule contradicts the shipped behavior.

2. Must Change (Critical / Blockers)

Morph can be assigned to multiple characters in probability mode

• Location: [data/commands.py](data/commands.py) — roll_probability_commands (diff lines 536–566) called per-character in the loop at mod_probability_random_commands (diff lines 630–635).
• Issue: Morph (id 3) is a valid probability command (PROBABILITY_EXCLUDED_NAMES doesn't exclude it), and roll_probability_commands runs independently per character with no cross-character Morph guard. The morph_placed logic (diff 616–625) only removes Morph from the backfill pool (available); it does nothing to stop the per-character rolls from each producing Morph. So -compru 3 100 gives every non-Gau character Morph, and -compru 3 50 gives it to ~half the party.
• Why: data/characters_asm.update_morph_character explicitly # NOTE: this assumes only 1 character has morph and charges the morph gauge for only the first holder — every other Morph command is non-functional/undefined, exactly the mechanic the design says to avoid (PR note #5). This is a documented, tested feature path (PR_MORPH_INVARIANTS uses -compru 3 50), so it's user-reachable.
• Note on the test gap: PR_MORPH_INVARIANTS only asserts cmds.count(MORPH) <= 1 per character — it never checks the party-wide total, which is why the bug passes CI.
• Suggested Fix: Track Morph placement across the per-character roll loop and stop rolling it once placed. For example, thread a shared mutable flag into roll_probability_commands:

# in mod_probability_random_commands, before the roll loop
morph_rolled = morph_placed  # already-explicit morph counts as placed

for character in characters:
    capacity[character] = (...)
    rolled[character], morph_rolled = self.roll_probability_commands(
        character, capacity[character], set(explicit[character]), morph_rolled)

def roll_probability_commands(self, character, capacity, held, morph_rolled):
    morph_id = name_id["Morph"]
    ...
            if command == morph_id and morph_rolled:
                continue
            if random.randrange(100) < percent and command not in held:
                rolled.append(command)
                if command == morph_id:
                    morph_rolled = True
    return rolled, morph_rolled

Then extend PR_MORPH_INVARIANTS to assert sum(MORPH in chars[i].commands for i in c.full_random_characters()) <= 1.

3. Could Change (Low Impact / Optional)

• Gau/Fight description mismatch (please reconcile before merge). PR note #6 says "Gau's probability of getting Fight is always half that of the other characters," but the code (roll_probability_commands, diff 561–562) fully skips Fight for Gau — he can never roll it. The test comment agrees with the code ("gau never gets fight from a probability roll, even at 100%"). This is almost certainly a stale description rather than a code bug, but it's user-facing, so either update the PR/README text or implement the halving. (Flagged here rather than as a blocker since the code is internally consistent and tested.)
• Explicit -com Magic/Item picks crash in composed mode. In mod_probability_random_commands (diff 609–614), an explicit -com command that is Magic/Item (accepted by _process_character_commands, which only checks command in id_name) falls to raise ValueError, an uncaught exception during generation rather than a clean parser.error. Consider validating explicit picks against the allowed set at parse time so the user gets a friendly message.
• options() can now emit more than six random_exclude_commandN rows (diff 336–343) and new command_probability_* / Random Mode keys. Worth confirming the flag-selection UI/menu consumer tolerates >6 exclusion rows and the new keys; low impact for ROM generation itself.
• menus/skills.py ASM is not covered by the test suite (the unit tests parse args / use fake characters and never assemble the ROM). Since it's an in-place 59-byte rewrite gated by Reserve(0x34d3d, 0x34d77, …), an overflow or logic error would only surface on a real build — see QA below.

4. General Notes & QA Check

• Praise:

  1. Excellent test discipline — the DRAFT_INVARIANTS / PR_* / COMPOSED_INVARIANTS scripts exercise the snake-draft refill, cap, empty-slot, and composition logic across thousands of trials without needing a ROM. That's exactly the right way to pin down probabilistic behavior.
  2. The menus/skills.py rewrite is beautifully documented — the comment explains the vanilla routine, the exact bug (esper row keyed to the Magic command), and how the byte-for-byte in-place rewrite preserves the table. Refactoring the moogle logic out into mod_moogle_commands for reuse is clean.

• Manual QA Testing Plan:

  1. Morph blast radius: Generate a seed with -compru 3 50 (and -compru 3 100), then inspect the spoiler Commands section — confirm how many characters end up with Morph. This reproduces the blocker above.
  2. Espers submenu fix: Seed with -comfr 0.0.0 (no Magic command for anyone), start a game, and verify a natural-magic learner can still open Skills → Espers and equip an esper, while Gogo and Umaro remain greyed out of the Espers row. Confirm the ASM fit in the reserved space (a build error would show at generation time).
  3. Composition + menu order: -com 05999999999999999999999999 -compr 28 100 -comfr 50.50.50 — verify Terra keeps her explicit Steal, everyone holds Possess, Fight/Magic/Item appear in the correct menu order, and no character exceeds four slots.
  4. Backward compatibility: Confirm a legacy flagstring using -rec1 28 -rec2 27 still round-trips (re-emitted as -rec 28.27) and produces an identical seed to before this PR for a plain -com … (no probability flags).

• Questions for Contributor:

  1. Is Morph intended to be declarable via -compr/-compru? If yes, the multi-character issue must be fixed; if not, it should be added to PROBABILITY_EXCLUDED_NAMES.
  2. For Gau + Fight: is the intended behavior "never" (as coded) or "half chance" (as documented in note #6)? Which one should the release notes reflect?
  3. Was the change of -com to nargs="*" verified against every place that reloads/round-trips a saved flagstring (e.g. presets/seed replays), given it now yields a list before process() normalizes it back to a string?