-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Add Double Battles #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sudo-owen
wants to merge
39
commits into
main
Choose a base branch
from
claude/double-battle-switch-combos-jvJGO
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+6,640
−457
Conversation
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
Milestone 1 of doubles refactor: - Add GameMode enum (Singles, Doubles) - Extend BattleData with slotSwitchFlagsAndGameMode (packed uint8) - Add p0Move2, p1Move2 to BattleConfig for slot 1 moves - Add constants for activeMonIndex 4-bit packing and switch flags - Update Engine startBattle to initialize doubles fields - Update getBattleContext/getCommitContext to extract game mode - Add gameMode field to Battle, ProposedBattle structs - Update matchmaker and CPU to pass gameMode - Update all tests with gameMode: GameMode.Singles All changes are backwards compatible - singles mode works unchanged.
Fixes compilation error from Milestone 1 - BattleConfigView was missing the new doubles move fields.
Milestone 2 progress: - Add getGameMode(battleKey) - returns GameMode enum - Add getActiveMonIndexForSlot(battleKey, playerIndex, slotIndex) - For doubles: uses 4-bit packing per slot - For singles: falls back to existing 8-bit packing These getters allow external contracts to query the game mode and active mons for specific battle slots.
Milestone 3: Doubles Commit Manager - Create DoublesCommitManager.sol with: - commitMoves(): commits hash of both slot moves - revealMoves(): reveals and validates both moves at once - Same alternating commit scheme as singles - Update Engine.setMove() to handle slot 1 moves: - playerIndex 0-1: slot 0 moves (existing behavior) - playerIndex 2-3: slot 1 moves (stored in p0Move2/p1Move2) Hash format: keccak256(moveIndex0, extraData0, moveIndex1, extraData1, salt)
- Add helper functions for doubles-specific active mon index packing - Add _computeMoveOrderForDoubles for 4-move priority sorting - Add _handleMoveForSlot for executing moves per slot - Add _handleSwitchForSlot for handling slot-specific switches - Add _checkForGameOverOrKO_Doubles for doubles KO tracking - Add _executeDoubles as main execution function for doubles mode - Add comprehensive tests for doubles commit/reveal/execute flow
- Fix getActiveMonIndexForBattleState to be doubles-aware - Fix getDamageCalcContext to use correct slot unpacking in doubles - Add DoublesTargetedAttack mock for testing slot-specific targeting - Add comprehensive doubles boundary condition tests: - test_doublesFasterSpeedExecutesFirst - test_doublesFasterPriorityExecutesFirst - test_doublesPositionTiebreaker - test_doublesPartialKOContinuesBattle - test_doublesGameOverWhenAllMonsKOed - test_doublesSwitchPriorityBeforeAttacks - test_doublesNonKOSubsequentMoves
- Add _handleSwitchCore for shared switch-out effects logic - Add _completeSwitchIn for shared switch-in effects logic - Add _checkForGameOver for shared game over detection - Refactor _handleSwitch to use shared functions - Refactor _handleSwitchForSlot to use shared functions - Refactor _checkForGameOverOrKO to use _checkForGameOver - Refactor _checkForGameOverOrKO_Doubles to use _checkForGameOver This reduces code duplication between singles and doubles execution paths.
Add validatePlayerMoveForSlot to IValidator and DefaultValidator: - Validates moves for specific slots in doubles mode - Enforces switch for KO'd mons, allows NO_OP if no valid targets - Prevents switching to mon already active in other slot - Update DoublesCommitManager to use new validation
Test scenarios for validatePlayerMoveForSlot: - Turn 0 only allows SWITCH_MOVE_INDEX - After turn 0, attacks are allowed for non-KO'd mons - Can't switch to same mon or other slot's active mon - One player with 1 KO'd mon (with/without valid switch targets) - Both players with 1 KO'd mon each (both/neither have targets) - Integration test for validation after KO - Reveal revert test for invalid moves on KO'd slot
When a player has a KO'd mon with valid switch targets, only they act next turn. Changes include: - Add _playerNeedsSwitchTurn helper to check if player needs switch turn - Update _checkForGameOverOrKO_Doubles to set playerSwitchForTurnFlag - Fix _handleMoveForSlot to allow SWITCH on KO'd slots - Fix _computeMoveOrderForDoubles to handle unset moves in single-player turns - Update tests for new turn flow behavior
Add tests covering all combinations of switch turn scenarios: - P1-only switch turns (mirrors of P0 tests) - Asymmetric switch targets (one player has target, other doesn't) - Slot 1 KO'd scenarios - Both slots KO'd with reserves - Game over when all mons KO'd - Continuing play with one mon after KO with no valid target Key principle tested: Switch turns trigger when a player has a KO'd mon AND a valid switch target. If no valid target, NO_OP is allowed.
When both slots are KO'd and only one reserve exists, both slots attempt to switch to the same mon. Previously this resulted in both slots having the same mon. Now the second switch is treated as NO_OP at execution time - the slot keeps its KO'd mon and the player continues playing with just one active mon. This handles the edge case without additional storage by checking if the target mon is already active in the other slot before executing the switch.
- Update validateSwitch to check both slots in doubles mode When a move forces a switch, it now correctly rejects targets that are already active in either slot for the player. - Add tests for forced switch validation in doubles: - test_forceSwitchMove_cannotSwitchToOtherSlotActiveMon - test_forceSwitchMove_cannotSwitchToSlot0ActiveMon - test_validateSwitch_allowsKOdMonReplacement These tests verify that validateSwitch (used by switchActiveMon for move-initiated switches) properly handles doubles mode.
Add two tests to verify MappingAllocator storage reuse works correctly when transitioning between different game modes: - test_doublesThenSingles_storageReuse: Complete a doubles battle, then verify a singles battle can reuse the freed storage key correctly - test_singlesThenDoubles_storageReuse: Complete a singles battle, then verify a doubles battle can reuse the freed storage key correctly Both tests execute actual combat with damage to ensure storage is properly written to and that mode transitions don't corrupt state. Also adds singles battle helper functions: - _startSinglesBattle: Creates and starts a singles battle - _singlesInitialSwitch: Handles turn 0 initial switch for singles - _singlesCommitRevealExecute: Commit/reveal flow for singles turns - _singlesSwitchTurn: Single-player switch turn handler for singles
Add a new Engine function `switchActiveMonForSlot` that correctly handles forced switches in doubles mode by using the slot-aware storage format. The existing `switchActiveMon` uses singles-style storage packing which corrupts the activeMonIndex in doubles mode. The new function: - Takes a slotIndex parameter to specify which slot to switch - Uses `_handleSwitchForSlot` for correct doubles storage handling - Uses `_checkForGameOverOrKO_Doubles` for proper KO detection Also adds: - DoublesForceSwitchMove mock for testing force-switch in doubles - Import for the mock in DoublesValidationTest
Add tests verifying the new switchActiveMonForSlot function works correctly: - test_switchActiveMonForSlot_correctlyUpdatesSingleSlot: Verifies force- switching slot 0 updates only that slot without corrupting slot 1 - test_switchActiveMonForSlot_slot1_doesNotAffectSlot0: Verifies force- switching slot 1 doesn't affect slot 0's active mon Also fixes DoublesForceSwitchMove to use Type.None instead of Type.Normal.
When both slots are KO'd and there's only one reserve mon, slot 0 may claim that reserve, leaving slot 1 with no valid switch target. Added validatePlayerMoveForSlotWithClaimed to IValidator to account for what the other slot is switching to when validating moves. This allows slot 1 to NO_OP when slot 0 is claiming the last available reserve, rather than incorrectly rejecting the NO_OP because the validator didn't account for the pending switch. Also adds tests verifying: - Both slots can't switch to same mon (reverts) - KO'd mon's moves don't execute - Both opponent slots KO'd mid-turn handled correctly
Summarizes all changes made for doubles support including: - Core data structure changes - New files added (DoublesCommitManager, tests, mocks) - Modified interfaces (IEngine, IValidator, Engine, DefaultValidator) - Client usage guide with code examples - Future work and suggested improvements
Adds test_singlePlayerSwitchTurn_withAttack to verify that during a single-player switch turn (when one slot is KO'd), the alive slot can attack while the KO'd slot switches. This confirms the implementation handles mixed switch + attack correctly.
AttackCalculator._calculateDamageFromContext was returning bytes32(0) when the accuracy check failed, but should return MOVE_MISS_EVENT_TYPE so callers can properly detect and handle misses.
…tions - Create BaseCommitManager.sol with shared commit/reveal logic - DefaultCommitManager and DoublesCommitManager now extend BaseCommitManager - Unify _hasValidSwitchTargetForSlot with optional claimedByOtherSlot param - Create _validatePlayerMoveForSlotImpl to share logic between slot validators - Add _getActiveMonIndexFromContext helper to reduce ENGINE calls - Add Engine.setMoveForSlot for clean slot-based move setting - Fix AttackCalculator to return MOVE_MISS_EVENT_TYPE on miss - Update CHANGELOG with refactoring details and known inconsistencies - Net reduction of ~200 lines of duplicated code
The _runEffects function was using _unpackActiveMonIndex which only returns slot 0's mon index. In doubles battles, effects on slot 1's mons would incorrectly look up effects for slot 0's mon instead. Changes: - Add _runEffectsForMon with explicit monIndex parameter - _runEffects now delegates to _runEffectsForMon with sentinel value - Update _executeDoubles to pass correct monIndex for each slot - Simplify baseSlot calculation (both branches did the same thing) Test: - Add DoublesEffectAttack mock for targeting specific slots - Add test_effectsRunOnBothSlots verifying effects run for both slots
Singles now uses the same 4-bit-per-slot packing as doubles, defaulting to slot 0. This removes redundant functions and simplifies the codebase: - Remove deprecated _packActiveMonIndices, _unpackActiveMonIndex, _setActiveMonIndex functions - Have _handleSwitch delegate to _handleSwitchForSlot with slot 0 - Update all singles code to use _unpackActiveMonIndexForSlot(..., 0) - Simplify external getters by removing mode-specific branching
Singles code now calls _handleSwitchForSlot directly with slot 0, eliminating the unnecessary _handleSwitch wrapper function.
This commit fixes several critical issues in the doubles implementation: Issue #1: Switch effects now pass explicit monIndex - _handleSwitchCore passes currentActiveMonIndex to switch-out effects - _completeSwitchIn passes monToSwitchIndex to switch-in effects Issue #2: Move validation now receives slotIndex - validateSpecificMoveSelection accepts slotIndex parameter - Uses _getActiveMonIndexFromContext to get correct mon for validation Issue #3: AfterDamage effects pass explicit monIndex - dealDamage now passes monIndex to effect execution Issue #4: OnUpdateMonState effects pass explicit monIndex - updateMonState now passes monIndex to effect execution Also adds: - Overloaded _runEffects that accepts explicit monIndex - EffectApplyingAttack mock for testing - MonIndexTrackingEffect mock for testing - Tests: test_afterDamageEffectsRunOnCorrectMon, test_moveValidationUsesCorrectSlotMon
- p0ActiveMonIndex → p0ActiveMonIndex0 - p1ActiveMonIndex → p1ActiveMonIndex0 - p0ActiveMonIndex2 → p0ActiveMonIndex1 - p1ActiveMonIndex2 → p1ActiveMonIndex1 This maintains consistent 0-indexed naming across the codebase.
Add attackerSlotIndex and defenderSlotIndex parameters to: - getDamageCalcContext (IEngine and Engine) - AttackCalculator._calculateDamage - AttackCalculator._calculateDamageView This fixes a bug where damage calculations in doubles mode always used slot 0's mon stats regardless of which slot was attacking or being attacked. All callers updated to pass slot indices (singles use 0, 0). Adds DoublesSlotAttack mock and tests to verify: - Attacking slot 1 uses slot 1's defense stats - Attacking from slot 1 uses slot 1's attack stats
101bb6c to
ef54e4d
Compare
…rclock These tests validate two bugs where global effects only affect slot 0 mons in doubles mode: 1. StaminaRegen.onRoundEnd() - uses getActiveMonIndexForBattleState() which only returns slot 0 mons, so slot 1 never gets stamina regen 2. Overclock.onApply() - same issue, only applies stat boost to slot 0 mon Both tests are expected to FAIL until the bugs are fixed.
Both effects were using getActiveMonIndexForBattleState() which only returns slot 0 mons. Fixed to iterate over both slots in doubles mode: - StaminaRegen.onRoundEnd(): Now regenerates stamina for all active mons (both slots in doubles, slot 0 only in singles) - Overclock.onApply(): Now applies stat boosts to all active mons of the player who summoned Overclock - Overclock.onRemove(): Now removes stat boosts from all active mons when the effect expires Uses getGameMode() and getActiveMonIndexForSlot() for proper slot handling.
- Move StaminaRegen doubles test to DoublesValidationTest.sol - Move Overclock doubles test to VolthareTest.sol - Add doubles helper functions to BattleHelper.sol for reuse - Delete DoublesEffectBugsTest.sol (tests now in appropriate files)
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.
Changelog
Double Battles Implementation
This document summarizes all changes made to implement double battles support.
Core Data Structure Changes
src/Enums.solGameModeenum:Singles,Doublessrc/Structs.solBattleArgsandBattle: AddedGameMode gameModefieldBattleData: AddedslotSwitchFlagsAndGameMode(packed field: lower 4 bits = per-slot switch flags, bit 4 = game mode)BattleContext/BattleConfigView: Added:p0ActiveMonIndex2,p1ActiveMonIndex2(slot 1 active mons)slotSwitchFlags(per-slot switch requirements)gameModesrc/Constants.solGAME_MODE_BIT = 0x10(bit 4 for doubles mode)SWITCH_FLAGS_MASK = 0x0F(lower 4 bits for per-slot flags)ACTIVE_MON_INDEX_MASK = 0x0F(4 bits per slot in packed active index)New Files Added
src/DoublesCommitManager.solCommit/reveal manager for doubles that handles 2 moves per player per turn:
commitMoves(battleKey, moveHash)- Single hash for both movesrevealMoves(battleKey, moveIndex0, extraData0, moveIndex1, extraData1, salt, autoExecute)- Reveal both slot movesIValidator.validatePlayerMoveForSlotBothSlotsSwitchToSameMonerror)test/DoublesCommitManagerTest.solBasic integration tests for doubles commit/reveal flow.
test/DoublesValidationTest.solComprehensive test suite (30 tests) covering:
test/mocks/DoublesTargetedAttack.solMock attack move that targets a specific slot in doubles.
test/mocks/DoublesForceSwitchMove.solMock move that forces opponent to switch a specific slot (uses
switchActiveMonForSlot).Modified Interfaces
src/IEngine.solNew functions:
src/IValidator.solNew functions:
src/Engine.solKey changes:
startBattleacceptsgameModeand initializes doubles-specific storage packingexecutedispatches to_executeDoubleswhen in doubles mode_executeDoubleshandles 4 moves per turn (2 per player), speed ordering, KO detection_handleSwitchForSlotupdates slot-specific active mon (4-bit packed storage)_checkForGameOverOrKO_Doubleschecks both slots for each playersrc/DefaultValidator.solvalidateSwitchnow checks both slots when in doubles modevalidatePlayerMoveForSlotvalidates moves for a specific slotvalidatePlayerMoveForSlotWithClaimedaccounts for cross-slot switch claiming_hasValidSwitchTargetForSlot/_hasValidSwitchTargetForSlotWithClaimedcheck available monsClient Usage Guide
Starting a Doubles Battle
Turn 0: Initial Switch (Both Slots)
Regular Turns: Attacks/Switches
Handling KO'd Slots
NO_OP_MOVE_INDEX)Future Work / Suggested Changes
Target Redirection (Not Yet Implemented)
When a target slot is KO'd mid-turn, moves targeting that slot should redirect or fail. Currently, this can be handled by individual move implementations via an abstract base class.
Move Targeting System
TargetTypeenum and standardizingextraDataencoding for slot targetingSpeed Tie Handling
Timeout Handling
DefaultValidator.validateTimeoutneeds reviewMixed Switch + Attack Turns
Ability/Effect Integration
UI/Client Considerations