Skip to content

Conversation

@HammerGS
Copy link
Member

Summary

This PR adds intelligent hidden unit reveal behavior to the Princess bot. Previously, hidden units would never voluntarily reveal themselves - they would stay hidden forever until an enemy stumbled into Point Blank Shot range. Now Princess evaluates firing opportunities and reveals when expected damage justifies breaking concealment.

Changes

Core Feature

  • New behavior setting: hiddenUnitRevealIndex (0-10 scale)
    • 0 = Never reveal (original behavior)
    • 5 = Moderate (default)
    • 10 = Always reveal if any target exists
  • Decision formula: reveal if (expectedDamage / selfPreservation) >= threshold
  • Princess calculates best firing plan for hidden units and reveals when the damage opportunity is worth it

Files Modified

File Changes
BehaviorSettings.java Added HIDDEN_REVEAL_VALUES array, field, getters/setters, XML serialization
BehaviorSettingsFactory.java Configured predefined behaviors (BERSERK=9, COWARDLY=1, CONVOY=0, etc.)
Princess.java Added shouldRevealHiddenUnit() method, modified calculateFiringTurn()
BotConfigDialog.java Added UI slider for hidden unit reveal setting
messages.properties Added i18n strings for UI

New Files

File Purpose
RevealCommand.java Chat command to force reveal all hidden units (testing)

Diagnostic Logging

Added detailed logging to help diagnose arc/facing issues:

  • Logs twist requirements, facing, canChangeSecondaryFacing, alreadyTwisted
  • Logs when TorsoTwistAction is sent
  • Warns if plan expects twist but no action generated

Predefined Behavior Values

Behavior hiddenUnitRevealIndex Reasoning
DEFAULT 5 Balanced
BERSERK 9 Very aggressive
COWARDLY 1 Very conservative
ESCAPE 2 Focus on escaping
RUTHLESS 7 Tactical but aggressive
PIRATE 8 Opportunistic
CONVOY 0 Never reveal, focus on moving

Testing

New chat command for testing:
Princess: reveal
Forces all hidden Princess units to reveal immediately.

Known Limitations

  • Reveals during firing phase rather than start of movement phase (slight deviation from strict RAW)
  • Future enhancement: "reveal to relocate" when enemies are too far away (deferred)

How to Test

  1. Create game with hidden unit rules enabled
  2. Deploy Princess-controlled unit as hidden
  3. Position enemy units at various ranges
  4. Observe Princess's reveal decisions based on behavior settings
  5. Use Princess: reveal command to force reveal for debugging
  6. Check unified_log.log for detailed decision logging

  Summary

  This PR adds intelligent hidden unit reveal behavior to the Princess bot. Previously, hidden units would never voluntarily reveal themselves - they would stay hidden forever until an enemy stumbled into Point Blank Shot range. Now Princess evaluates firing opportunities and reveals when expected damage justifies breaking concealment.

  Changes

  Core Feature

  - New behavior setting: hiddenUnitRevealIndex (0-10 scale)
    - 0 = Never reveal (original behavior)
    - 5 = Moderate (default)
    - 10 = Always reveal if any target exists
  - Decision formula: reveal if (expectedDamage / selfPreservation) >= threshold
  - Princess calculates best firing plan for hidden units and reveals when the damage opportunity is worth it

  Files Modified

  | File                         | Changes                                                                     |
  |------------------------------|-----------------------------------------------------------------------------|
  | BehaviorSettings.java        | Added HIDDEN_REVEAL_VALUES array, field, getters/setters, XML serialization |
  | BehaviorSettingsFactory.java | Configured predefined behaviors (BERSERK=9, COWARDLY=1, CONVOY=0, etc.)     |
  | Princess.java                | Added shouldRevealHiddenUnit() method, modified calculateFiringTurn()       |
  | BotConfigDialog.java         | Added UI slider for hidden unit reveal setting                              |
  | messages.properties          | Added i18n strings for UI                                                   |

  New Files

  | File               | Purpose                                                 |
  |--------------------|---------------------------------------------------------|
  | RevealCommand.java | Chat command to force reveal all hidden units (testing) |

  Diagnostic Logging

  Added detailed logging to help diagnose arc/facing issues:
  - Logs twist requirements, facing, canChangeSecondaryFacing, alreadyTwisted
  - Logs when TorsoTwistAction is sent
  - Warns if plan expects twist but no action generated

  Predefined Behavior Values

  | Behavior | hiddenUnitRevealIndex | Reasoning                     |
  |----------|-----------------------|-------------------------------|
  | DEFAULT  | 5                     | Balanced                      |
  | BERSERK  | 9                     | Very aggressive               |
  | COWARDLY | 1                     | Very conservative             |
  | ESCAPE   | 2                     | Focus on escaping             |
  | RUTHLESS | 7                     | Tactical but aggressive       |
  | PIRATE   | 8                     | Opportunistic                 |
  | CONVOY   | 0                     | Never reveal, focus on moving |

  Testing

  New chat command for testing:
  Princess: reveal
  Forces all hidden Princess units to reveal immediately.

  Known Limitations

  - Reveals during firing phase rather than start of movement phase (slight deviation from strict RAW)
  - Future enhancement: "reveal to relocate" when enemies are too far away (deferred)

  How to Test

  1. Create game with hidden unit rules enabled
  2. Deploy Princess-controlled unit as hidden
  3. Position enemy units at various ranges
  4. Observe Princess's reveal decisions based on behavior settings
  5. Use Princess: reveal command to force reveal for debugging
  6. Check unified_log.log for detailed decision logging
Copilot AI review requested due to automatic review settings December 18, 2025 04:02
@HammerGS HammerGS requested a review from a team as a code owner December 18, 2025 04:02
@HammerGS HammerGS added Princess/AI Issues or PR that relate to the current Bot AI In Development (Draft) An additional way to mark something as a draft. Make it stand out more. AI Generated Fix AI-generated fix. Requires human testing and review before merging. Needs Player Testing PR lacks actual play testing. labels Dec 18, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds intelligent hidden unit reveal behavior to the Princess bot, allowing hidden units to evaluate firing opportunities and reveal themselves when expected damage justifies breaking concealment. Previously, hidden units would never reveal voluntarily.

Key changes:

  • New hiddenUnitRevealIndex behavior setting (0-10 scale) with configurable thresholds
  • Decision logic that compares expectedDamage / selfPreservation against threshold to determine whether to reveal
  • UI slider in bot configuration dialog for adjusting reveal willingness

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
BehaviorSettings.java Adds HIDDEN_REVEAL_VALUES array, getters/setters, XML serialization, equals/hashCode for the new reveal index setting
BehaviorSettingsFactory.java Configures reveal index values for predefined behaviors (BERSERK=9, COWARDLY=1, CONVOY=0, etc.)
Princess.java Adds shouldRevealHiddenUnit() decision logic, modifies calculateFiringTurn() to check reveal conditions, adds diagnostic logging for twist/facing issues
BotConfigDialog.java Adds UI slider for hidden unit reveal setting with proper initialization and change detection
RevealCommand.java New chat command to force reveal all hidden units for testing/debugging purposes
ChatCommands.java Registers the new REVEAL command with abbreviation "rv"
messages.properties Adds i18n strings for UI slider labels, tooltips, and command description
log4j2.xml Changes bot logging level from error to debug (should be reverted before merge)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

HammerGS and others added 3 commits December 17, 2025 21:45
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
  | Issue                               | File                      | Fix                                                                           |
  |-------------------------------------|---------------------------|-------------------------------------------------------------------------------|
  | Critical: Logging level debug→error | log4j2.xml:61             | Reverted megamek.client.bot from "debug" to "error"                           |
  | Diagnostic logging info→debug       | Princess.java:1006,1124   | Changed twist/facing diagnostics to LOGGER.debug()                            |
  | Unused parameter (CodeQL)           | Princess.java:4061        | Removed unused shooter param from hasAnyValidTarget()                         |
  | Comment clarity                     | BehaviorSettings.java:129 | Changed "minimum damage ratio" to "(expectedDamage / selfPreservation) ratio" |
  | Comment clarity                     | BehaviorSettings.java:141 | Changed "if any damage possible" to "if any target
Copy link
Collaborator

@IllianiBird IllianiBird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One tiny comment, once resolved this looks good to merge

HammerGS and others added 5 commits December 18, 2025 13:09
  | Issue                                            | Fix Applied                                                       |
  |--------------------------------------------------|-------------------------------------------------------------------|
  | IllianiBird: Remove "(current behavior)"         | ✅ BehaviorSettings.java:131 - removed annotation                 |
  | Copilot: Debug logging left in                   | ✅ log4j2.xml:68 - changed megamek logger from "debug" to "error" |
  | Copilot: Twist/facing diagnostics at wrong level | ✅ Already at debug level - no change needed                      |
  | Copilot: Unused shooter parameter                | ✅ Method already doesn't have this parameter - no change needed  |
  Enhances Princess's hidden unit reveal decision with damage trend analysis:
  - Track expected damage history per hidden unit (last 7 rounds)
  - Analyze trend direction (increasing/peaked/declining)
  - Apply trend multiplier to reveal threshold:
    - 0.75x at peak (easier to reveal - best moment to strike)
    - 0.85x when declining (opportunity fading, don't wait)
    - 1.25x when increasing below 60% (wait for better shot)
    - 1.1x when increasing above 60% (getting close to peak)

  Debug logging improvements:
  - Add enemy detection logging in EnemyTracker (NEW/UPDATE/LOST)
  - Add visible enemies list at start of firing phase
  - Log trend analysis details (history size, peak, ratio, direction)
  - Include trendMultiplier in reveal evaluation output
  - Suppress verbose Precognition lock logging to reduce noise

  Files changed:
  - Princess.java: Trend tracking logic and reveal decision integration
  - EnemyTracker.java: Enemy detection logging
  - log4j2.xml: Logging configuration for Princess debug output
…Unit-Reveal' into Fix-Princess-Intelligent-Hidden-Unit-Reveal
  Changes Made:

  1. BotClient.java - Added PREMOVEMENT phase handling:
  case PREMOVEMENT:
      evaluateHiddenUnitRepositioning();
      sendDone(true);
      break;
  Plus a default empty implementation that subclasses can override.

  2. Princess.java - Added reposition evaluation logic:

  | Method                                    | Purpose                                                                                 |
  |-------------------------------------------|-----------------------------------------------------------------------------------------|
  | evaluateHiddenUnitRepositioning()         | Main entry point - evaluates all hidden units during PREMOVEMENT                        |
  | evaluateHiddenUnitForRepositioning()      | Evaluates single hidden unit for reposition potential                                   |
  | calculateDamageFromCurrentPosition()      | Gets expected damage from current position                                              |
  | generateRepositionCandidates()            | Generates candidate positions within movement range                                     |
  | calculateEnemyCenter()                    | Finds center of enemy positions for facing calculation                                  |
  | calculateDamageFromHypotheticalPosition() | Calculates damage from a hypothetical position using EntityState + Builder.buildGuess() |

  How It Works:

  1. During PREMOVEMENT phase, Princess evaluates each hidden unit
  2. Calculates expected damage from current position (with torso twist)
  3. Generates candidate positions within walk MP range
  4. For each candidate, calculates expected damage from that position
  5. If best reposition damage > 2x current damage, marks unit to reveal at movement start
  6. Unit reveals at start of MOVEMENT phase, then can move normally

  Debug Logging:

  Hidden unit Akuma reposition evaluation: currentDamage=3.50, bestRepositionDamage=45.00, threshold=2.0, bestPosition=(1805)
  Hidden unit Akuma will reveal to reposition: (2103) -> (1805) (damage: 3.5 -> 45.0)

  This completes the intelligent hidden unit reveal feature set for PR #7812.
Copy link
Member

@psikomonkie psikomonkie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests?

@HammerGS
Copy link
Member Author

Unit tests?

Not ready for prime time yet. Is working but needs more tweaks.

@psikomonkie
Copy link
Member

Unit tests?

Not ready for prime time yet. Is working but needs more tweaks.

We should probably set this back to Draft then so it doesn't get accidentally merged before the tests are ready.

@HammerGS HammerGS marked this pull request as draft December 20, 2025 02:49
@HammerGS HammerGS added the For New Dev Cycle This PR should be merged at the beginning of a dev cycle label Dec 21, 2025
@HammerGS HammerGS removed the For New Dev Cycle This PR should be merged at the beginning of a dev cycle label Jan 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Generated Fix AI-generated fix. Requires human testing and review before merging. In Development (Draft) An additional way to mark something as a draft. Make it stand out more. Needs Player Testing PR lacks actual play testing. Princess/AI Issues or PR that relate to the current Bot AI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants