Skip to content
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

Threat refactor #432

Open
MikeJeffers opened this issue Nov 13, 2021 · 2 comments
Open

Threat refactor #432

MikeJeffers opened this issue Nov 13, 2021 · 2 comments
Labels
Difficulty: Easy This issue would be easy to do, and has little uncertainty in scope and implementation Priority: Medium This ticket is in scope for a future patch Refactor Code maintainability, style, or other non-functional changes

Comments

@MikeJeffers
Copy link

Threats and how they are valued, generated, and manipulated before sending to the threat manager of an NPC are all over the place
Some are here: https://github.com/OpenPerpetuum/PerpetuumServer/blob/Development/src/Perpetuum/Zones/NpcSystem/ThreatManager.cs#L12-L35
But then the relevant amounts and details live elsewhere like:
https://github.com/OpenPerpetuum/PerpetuumServer/blob/Development/src/Perpetuum/Modules/EnergyNeutralizerModule.cs#L51-L52
https://github.com/OpenPerpetuum/PerpetuumServer/blob/Development/src/Perpetuum/Modules/EnergyTransfererModule.cs#L49
https://github.com/OpenPerpetuum/PerpetuumServer/blob/Development/src/Perpetuum/Modules/EnergyVampireModule.cs#L60
https://github.com/OpenPerpetuum/PerpetuumServer/blob/Development/src/Perpetuum/Zones/NpcSystem/Npc.cs#L973

And a few other places (use find all references for ThreatType)

It would be a bit easier to track and manage all the bespoke values and modifiers of threat if they were in one place.
This will help developers reason about and communicate threat value calculation changes to designers and players.

@MikeJeffers MikeJeffers added Refactor Code maintainability, style, or other non-functional changes Difficulty: Easy This issue would be easy to do, and has little uncertainty in scope and implementation Priority: Medium This ticket is in scope for a future patch labels Nov 13, 2021
@Ramit110
Copy link

Ramit110 commented Sep 5, 2022

Would it make sense to have a ThreatBuilder for this? I'm thinking of making the class with a bunch of static methods for things like BuildThreatNeut(int value), BuildThreat(int value), BuildThreatVampire(int value) ect ect for all the places it's used, that way all the threat calculations are all in one class/file.

@Ramit110
Copy link

Ramit110 commented Sep 5, 2022

There's a small bit of duplication in ThreatManager.cs, mentioned in a comment, otherwise code should be acceptable.

56612db and f10d303 are the last commit I'll be doing until I get a response XD

@clouths clouths moved this to BACKLOG in Game Server Dec 17, 2023
@clouths clouths moved this from BACKLOG to Checkup in Game Server Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Easy This issue would be easy to do, and has little uncertainty in scope and implementation Priority: Medium This ticket is in scope for a future patch Refactor Code maintainability, style, or other non-functional changes
Projects
Status: Checkup
Development

No branches or pull requests

2 participants