-
Notifications
You must be signed in to change notification settings - Fork 15
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
Issue 335: Repair cost as defined in .ini #391
Issue 335: Repair cost as defined in .ini #391
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the submission! In addition to the comments could we:
- Fix the SonarCloud issue thats popped up
- Link the issue this resolves in the PR
- Take a look at my comment about the get_ship_repair_cost() function, since this could make things much simpler.
CHANGELOG.md
Outdated
@@ -10,6 +10,9 @@ | |||
## 4.0.17 | |||
- Provided configs for 0x86AEC, 0x84018, 0xD3D6E and 0x58F46 in server.dll in crash catcher. These offsets were hardcoding several values that are often changed by mods manually relating to NPC spawn and scanner range. 0 values disables each respective hook. | |||
|
|||
## 4.0.17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be put at the top of the changelog and the version incremented please
plugins/autobuy/Autobuy.cpp
Outdated
|
||
if (valid) | ||
{ | ||
global->repairCosts.insert(std::pair<uint, int>(baseNickname, repairCost)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this be inside the else if and remove the need of the valid variable? It's been a while since I looked at any INI parsing stuff admittedly
plugins/autobuy/Autobuy.h
Outdated
@@ -50,6 +50,14 @@ namespace Plugins::Autobuy | |||
R"(..\data\equipment\weapon_equip.ini)", | |||
R"(..\data\equipment\prop_equip.ini)" | |||
}; | |||
|
|||
//paths to inis which should be read for repair cost | |||
std::vector<std::string> baseIniPaths |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't the admin need to add every single base ini if we did it this way? Could we not loop through the SYSTEMS folder and pull out any .ini thats ends in "base"? Wouldnt need to have this configurable variable then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might have found a better way. There is a "get_ship_repair_cost" function in the BaseData class in FLCoreCommon.h, I wonder if we could use that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested this but I think you could do:
float cost = BaseDataList_get()->get_base_data(baseId)->get_ship_repair_cost();
That would make this code much simpler since you wouldnt need to parse INIs or keep a map of ship repair costs. You could just call this whenever you need to calculate a ship repair cost.
plugins/autobuy/Autobuy.cpp
Outdated
@@ -67,7 +67,15 @@ namespace Plugins::Autobuy | |||
|
|||
void handleRepairs(ClientId& client) | |||
{ | |||
auto repairCost = static_cast<uint>(Archetype::GetShip(Players[client].shipArchetype)->fHitPoints * (1 - Players[client].fRelativeHealth) / 3); | |||
auto base = Hk::Player::GetCurrentBase(client); | |||
auto repairCost = static_cast<uint>((1-Players[client].fRelativeHealth) * Archetype::GetShip(Players[client].shipArchetype)->fHitPoints * 0.33); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're calculating this value even if its gonna get overwritten by the assignment in the if check. Could we put this in an "else" after the if check?
Quality Gate passedIssues Measures |
Fixed the issue that the original issue described.