Implement Seismic / Lightning Spire Trap DPS #5212
Conversation
Added the basic seismic trap mechanics and also added some average shotgunning against configurable enemy size.
|
|
This still fails to factor cooldown recovery rate and duration as far as I can tell as dps is affected by the average number of traps on the ground. |
|
@Edvinas-Smita send me a friend req/message request on Discord so I can invite you to our PoB dev Discord. Will make it easier to discuss stuff with this PR LocalIdentity#9871 |
and copy pasted most of its logic to lightning spire trap and a bit to flamethrower trap
src/Data/Skills/act_dex.lua
Outdated
| statMap = { | ||
| ["base_skill_show_average_damage_instead_of_dps"] = {}, | ||
| ["phys_cascade_trap_base_interval_duration_ms"] = { | ||
| skill("repeatFrequency", nil), |
There was a problem hiding this comment.
| skill("repeatFrequency", nil), | |
| skill("PulseCooldown", nil), |
Most mods are capitalised like this and this is the time between pulses not the frequency
Pulse rate should probably be changed to be the inverse of how you currently have it or relaballed to pulse interval or similar. As it is not exactly clear where the hit rate of the skill comes from as you cannot see the same number elsewhere.
There was a problem hiding this comment.
Good catch with the frequency. Though the capitalization looks a bit bit weird: in these skill files when it's mod("... it's capitalized (mostly) and when it's skill("... it's lowercase (mostly). Not sure which should be the case.
There was a problem hiding this comment.
Yeah fair enough on the capitalisation pick which ever then.
|
I feel like this might be able to be made more readable with some renaming / shuffling as it is fair hard to read. Not sure the best way to tackle this. I am fairly happy with the math the only math that could use justification is the circle math as how that formula works and produces the correct hit rate. |
|
Is there a way to factor these functionally equivalent functions for both traps so there is less code to maintain in the future? |
Expanded, reworded and shuffled information to make it more readable (hopefully) + some small fixes
Shuffled it around a bit such that going top to bottom everything that goes into DPS Multiplier is defined sequentially. Might help a bit
Looks like it could be deduplicated by moving it into CalcOffence, but it would be terribly out of place there. Other than that, I don't know enough about the codebase and/or the template + combined data files to see a good prospect. Regarding the circle calculations - unifying multi-part skill configuration sounds amazing, but also sounds like the scope would be quite a bit too big for me. I reckon it's worth leaving the seismic circle calculations in, at least while they're still relevant in 3.20. Though I will not mind if the decision is to remove the skill parts with average waves that automatically include these calculations. |
|
I think it is fine in calc offense we already have explosive arrow implemented in there and soon to be #5049 sustainable trauma stacks (hopefully) |
|
Your config names are a bit long for this box it probably effects other skills but if it not to hard it might be nice to fix here as well. |
LocalIdentity
changed the title
3 participants
Added the basic seismic trap mechanics and also added some average shotgunning against configurable enemy size.
Fixes #3150.
Description of the problem being solved:
Seismic trap mechanics were not being calculated
Steps taken to verify a working solution:
Link to a build that showcases this PR:
Before screenshot:
After screenshot: