Active modules now typically consume capacitor charge on initial cycle #293
Conversation
Reviewer's Guide by SourceryThis pull request addresses an issue where active modules were not consuming capacitor charge on their initial activation cycle. The implementation adds capacitor consumption checks and deductions for module activation, ensuring that modules now properly consume capacitor charge from the first activation. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @charles-m-knox - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| if (m_modRef->HasAttribute(AttrCapacitorNeed)) { | ||
| float remainingCapacitorCharge = m_shipRef->GetAttribute(AttrCapacitorCharge).get_float(); | ||
| float requiredCapacitorCharge = GetAttribute(AttrCapacitorNeed).get_float(); | ||
|
|
||
| float newCap = remainingCapacitorCharge - requiredCapacitorCharge; | ||
|
|
||
| if (newCap < 0) { | ||
| m_shipRef->GetPilot()->SendNotifyMsg( | ||
| "This module requires %.0f GJ, but your capacitor only has %.0f GJ remaining.", | ||
| requiredCapacitorCharge, | ||
| remainingCapacitorCharge | ||
| ); | ||
|
|
||
| AbortCycle(); | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| float shipTotalCapacitor = m_shipRef->GetAttribute(AttrCapacitorCapacity).get_float(); | ||
|
|
||
| m_shipRef->SetShipCapacitorLevel(newCap / shipTotalCapacitor); | ||
| } |
There was a problem hiding this comment.
suggestion: Consider extracting the capacitor check logic into a separate method
The capacitor check logic is duplicated in both DoCycle() and CanActivate(). Extracting this into a separate method would improve maintainability and reduce the risk of inconsistencies.
bool CheckCapacitorRequirement() {
if (!m_modRef->HasAttribute(AttrCapacitorNeed)) return true;
float remainingCapacitorCharge = m_shipRef->GetAttribute(AttrCapacitorCharge).get_float();
float requiredCapacitorCharge = GetAttribute(AttrCapacitorNeed).get_float();
float newCap = remainingCapacitorCharge - requiredCapacitorCharge;
if (newCap < 0) {
m_shipRef->GetPilot()->SendNotifyMsg("This module requires %.0f GJ, but your capacitor only has %.0f GJ remaining.", requiredCapacitorCharge, remainingCapacitorCharge);
return false;
}
m_shipRef->SetShipCapacitorLevel(newCap / m_shipRef->GetAttribute(AttrCapacitorCapacity).get_float());
return true;
}
|
|
||
| if (newCap < 0) { | ||
| m_shipRef->GetPilot()->SendNotifyMsg( | ||
| "This module requires %.0f GJ, but your capacitor only has %.0f GJ remaining.", |
There was a problem hiding this comment.
suggestion: Consider rephrasing the error message to use percentage values
Using percentage values instead of absolute GJ values in the error message could be more intuitive for players who may not be familiar with the unit 'GJ'.
"This module requires %.1f%% of your capacitor, but you only have %.1f%% remaining.",
(requiredCapacitorCharge / m_shipRef->GetCapacitorCapacity()) * 100,
(remainingCapacitorCharge / m_shipRef->GetCapacitorCapacity()) * 100
|
|
|
lgtm ❤️ |
Currently, you can repeatedly activate and deactivate a module over and over and it will not consume any capacitor charge. From my testing, this applies to:
It probably applies to all currently supported modules in evemu.
It is important to note that repeating a module's active cycle does consume the expected capacitor amount. It's only the initial activation that doesn't seem to consume charge.
This pull request fixes this issue by simply calculating the charge required in order to activate and deducting it from the ship's capacitor as expected.
Thank you for taking the time to review all of my pull requests 🙂
Summary by Sourcery
Ensure active modules consume capacitor charge on initial activation by checking and deducting the required charge from the ship's capacitor, preventing activation if insufficient charge is available.
Bug Fixes: