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.

Sign up for GitHub

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

Jump to bottom

Add ailment threshold #628

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Add ailment threshold #628

wants to merge 10 commits into from

Conversation

OrderedSet86
Copy link
Contributor

@OrderedSet86 OrderedSet86 commented Jan 27, 2025 •
edited
Loading

Old PR: #434 (not the same PR due to fork shenanigans)

No issue fixes yet, might help with #178 though. Pushed early to avoid work duplication conflicts with other devs.

Description of the problem being solved:

  • Previous elemental ailment calc would always give 0% for non-crit and 100% for crit to apply ailment. Seems to be just PoE1 calc and missing values in PoE2
  • Ignite and Shock calcs should use Enemy Ailment Threshold for % chance to apply, and were not
  • Order of ailment breakdown in Calcs tab was confusing, values referenced in one hover breakdown were not consistently coming from above data (you had to read back and forth)

Steps taken to verify a working solution:

  • Compare to in-game tooltips for implementation reference

Implementation notes / stuff to still be done in future patches

  • Add standardized terminology for types of ailments in PoE2:
"Flat chance ailments"
[Ailments that require chance and ignore threshold]
- Bleed (damaging)
- Poison (damaging)

"Scaling threshold ailments"
[Ailments that use enemy ailment threshold (1% chance per 4% ailment threshold dealt)]
- Ignite (damaging)
- Shock (non-damaging)

"Minimum threshold ailments"
[Ailments that "require a minimum threshold" (unknown number) but then always apply]
- Chill (non-damaging)

"Buildup ailments"
[Ailments that use buildup mechanic]
- Electrocute (non-damaging)
- Freeze (non-damaging)
- Stun (non-damaging - note that I think the game doesn't consider this an "ailment")
  • Bleed, Shock, Poison, and Ignite chance values should be computed correctly now. My chance values for Shock and Ignite use the average hit applied to the enemy ailment threshold. [In the future for maximum correctness, these should actually say the chance range (since now ignite/shock chance scales with the hit damage roll).]
  • Move damage functions above ailment chance code, as now this is required for Ignite and Shock.
  • Added Enemy Ailment Threshold to ailments it matters for (ignite and shock), and added to breakdown
  • For Chill threshold I just guessed 60% of Enemy Ailment Threshold, the tooltip is quite vague:
    image
  • Not sure how freeze/stun/electrocute (buildup ailments) should be implemented. I set freeze and electrocute chance on hit variables temporarily to 0, they are buildup ailments anyway so I'm not sure how much "chance on hit" would be meaningful
  • I sorted the calc tab ailments in "calculation order", ie which values have to be known in which order to compute the final value. This matches how damage calculations are sorted.
  • Removed logic for various legacy ailments, like PoE1 "alt" ailments (scorch, sap, brittle)
  • Update knockback calculation (there was some knockback calculation in ailment calc for some reason)
  • Fix logic error in canDoAilment causing calcAilmentSourceDamage to unconditionally return 0 damage.

Link to a build that showcases this PR:

I made an Ailment Demo project which applies the following effects:

  • Poison
  • Bleed
  • Ignite
  • Chill
  • Shock
eNqtW-tz2zgO_1z_FRrP3M3dnOPo_cglu-M4SZNeHl47abv7pcNYtK1GllyJyqM7_d8PICVZdkxZTrs7k8oifgAIAiAI2oe_P89D5ZEmaRBHR22tq7YVGo1jP4imR-2727M9t_37b63DAWGzm8lxFoQ4ov_WenfIPyghfaQhAFUAjkOSptdkTo_anwI2nrUVko5p5PeX74_DOPaVKzKlbYWRZErZx0K2-kUDFjOSkDGjySXy7WUsvop9wE1ImAJkToJoFI8fKHufxNkC5LaVx4A-CaKLq8HN8LYNyr07HITkhSYjRpiSwp-jdg8mCWJPyJwLfyRhBhBb87q2ZVi24anwp71fCz7OkpS9jcNoQalfgrSuZpsy0kFCTycTOmbBI-0nAevPSDReCrRkuF1pr7KQBYswoElJr8voz1-xxhWXEN_GjIQng1FJ6-haV3dsx3QN19bselzMSpxUwnEI1qxKMNSursMyGI50yuCSs1c4x9K6lmrorq4ZNZpdTKOA0SpQs7uO6ZmWZdfJe41zDLer6rauGl6dKQZxkMbRikCna3me7daJe41Cca5jm3ozy1ehttm1HMu2PUPuRVkYQjqooqRLNqQpTR4JC1YVlNL34_l9EK2tlmt1XUs3NcepW61eQsnNRMTQkPhBll5RltB06eZdafBdkYj045RVzFBHOqAJZDi2glC3AEZ0HENSrEIcs2t6dgNBm-FSiZfBhDacDZLuNJscsPNsKoJ2nM3pqCndzozfptAQNoJmlKM4CxtSsmU6tqX5-IQ-L71Zd-RB922FUpMyvIiWuumWXOxjjBG8fRo8o5yeD5b5y_TcrqXrGm4DjlTjwewlDcYkvCLPwTybw75zSx7oUqBm6Zrcs6YzFkFCkoJVV-qWZ0FC34Lrx6H_JtyMxKl8llpNNgiicxL5vfE4g1rpZQlyTb0u6Krc1Zo6ZXyA1BfReAnQ6vjeRQnP7dX6pkYCQoYQZFhS3Ye0MWYpJg_WJqWIEDalUS7xpVleu6R0PHsPhh4SRpsl9pLKdOuNi8RV49Zy3WDbGv6riB3MhMDNZoLqVq9D7Wip0SKAyrOJSoJyk281x-xggdOIJtOX0Sygod_A76vkhc36ZNEAyX2gCm_kC6vydvLnKnTH1cL9bVdpjyStbhCGp9dbQtBXjaDL3ZRC_Q4In64dRlT5WSr-iiepcDdYL5nHWdLEnWEKgrjRMha7mzhDDqmfjZttp-WBEA7Q44em0yhRoGcY7gTtMUbGDyexP21sNC5kJ8SqfqNssYDIRXdoygA3bThYBJXCas9sQH0DzlwNV7Vud28uYEndWEBZszSXsgZpPhesONbENCFuLKBczitIFnNIvLzpcRVX-x6q9AByBofJRidDTrh6VpUHXPwEys-w05TuRg2l2TKJSVVJaPT9pTH_FfJGAk4jH4o8iIXGMtYRm8QcZ5NJqozhhE3YJazwUbut3MO74hkKy5TmHwTiNphD7k3TE8KI4uengI8kCUjEdN6GSylJxjMEnZEwvIfkgZyWb_kn3ro7C0JGkxN4h2riVNY5asgRBR_u8y4jPl3MF3HCFPqM_wxIwl6KriAn5G-AT8qCiDcZIIWFYVsZzeKnnv-Ikm7jOEzLViJZLGjkr_C4TShVSJGQxqgEnzx-UOYkBa1fhI-nOJtKc_PCx96ZEsWgADzZuqt3dNtx7Y7muvBsWXD86ZiwGRodOBpbTsdWLdXq2IalA42qGVpH1zzX7NgAg1HNVIHS8Rwb_hpeR7NNW-1opmnaHUN1VRdIDE0HcsN1O4bngijTNVWtY-iObQAlKAEsdVftwPlQM0AFHYQAA8tCGqDUTNtCPqbmdRxd7xiW5zodzXIcq4PNIhhyDbNjeZbhdnTT1a0OsENBoLDDRTgdy7YtrWO5mup1LA0GLdW0PdQUhjzD6lgg0AGJrgdjhmYC2gYC3bPQLp6BauqoguWqoAToZJkgx_P0jmNbNhCqwNpwNRMMYjgwfwsm4QDIAiVAWU0FE1kwK9tVwUTw3gDbw1w6rqlZsACmivJBS8_UwT6qimgPjAsCdJyZCX9cwwEwyELbGaaKU7VUUNAFpSxHBStrnqaC-rYLysIigcF003DRx7FjQJKX3qpDRAH4HwOfWmtxF-MG9653h3fDS_7wbsbYIj3Y3396euouCJvFE_oMlUsXAnV_ASDwy730IQjDPeS634P_jqdX_TudfCP_-_DhifnsmzU43qPPf-31_4wetM9T7_rzo-6w-DS-7t2ezDJ69vVxOrhR567tnX7-4lw7f91enAVgyj22MPa8m7vLR_vpcvT1w5UXvbyP3T-T-GF-Yt3fk-nNLXWMu8fgy3DYfx7d9R6urUgb6r3p8W0_Tj9-_5pNgo_WIFgkn8n4rufb52fDcLD3zQwnmvnJtp6ev_1x6WmRFZxMhnvjq_Tl9tr_6H3_8Gxc9f86ztSH2LnpDdkDc3ujb33dfF6ok-RPmOXRETfUfmGpQ3EHkO6LT7hBJQGEnjDjIdQuSXCfMVoMQF54vs5jky981adFPAqfXolK4d_cs7mjVINSxKPwdRGPwvFEDAonrsag8FyREnLfwRAUbsedUyQBEZM8GoWfonMK5xcOKTxbJAIRqkUiwLwhsg134LYSRCyfNPgoS_JnESsiY_B4zKNPRCIPNhFUInyFu4sgWnaQYS1WjH64jzmSJ2xMovhwHTMxhi-LD4cjdN8UNoKEvafz9PgFdvEzPJmsNXzzLIzUI8rEBlPFHLVZklHcOCYkC_H9HxkJA9wU1OrbS3E9FcXJvOwpASvYFLDaFBxvXxZomN7lZZ7uc6lK4BdbQP6S3z_1lqr1SThOuXJBNA4zn15EeRVTbjMhuUcF8FYNux1-ofgap1LQu0PQiG_loMFFBI6bJxKBfx_G9yTUCy757Zuuro5rxfij2E8x25RVYy9J4iewAYp9PSDqibbyrWLOKZ0j4RVlxIf9f_-CwTLs41rsc-XRzmvcIzLP99ByRMmH1gwxjrMoX-DKpHPL4syFWWAFxDAf4TTKFOocRn2xJcsmEggb5udxdFVQ9-2m1rZb-hMlizg6DXkJXJz1cner2D1_s5F611UofXmQBODocCIlc974WK6DGFGWQ7uuQxH4fMl_UVCkcHDGEhCTxgFu1788TgqJ9atXCQbsF84jWMp00yJU_XpClYL0F1gzfwRSnkZFosRHDuIUF9EiY1yFo_Y8SMdfsGbHq2-e3_mN_enZ2Wn_9uLjaV63VyF8kl-ibH6PsxL_loFxOKK8FaGk2X0qHo_aHwP6xBU5AacLwhTnFYZkkdKycOZJMtc8BFwNN051HpQX6Jt5LQnknE6faQJ1_vQTHC6SgEr1Kse3KCUEYocLzzkybngzLWckGiZ9OCWI3prEUvxrAHIueC8vnQ4O1mDhhERCqeR8dIslGO6F4N_BJBhjwq1fctw5BVWNXcprCul6590eOQ9-bS9jIAblYHEpLkPnozVW5Vf4UquKUTn8hI6JdO5iUA4u27VxdI5d881cSqoaTtdxxJ0cd8cgxA1AurKwFZUkcoY3bEaT_DAs43QFOaogqQ0cUa1L-VQoamzFb7gkFsIxOVRc30jmgGM1QZPfZ8iiPagP2dVbB8l6VGnkrESvTmrDvO9Xswx5y1uyBGK0ZiZF318yiXy4JlB4Du49xoEver-SkFkjq0sacEj8eTa8of3zbNY73D_P8Swk6YN0vfNROfyOBVjVbOAiKphGTDCwfo4DxtfPccA-5_zN6OF6KbLEDuuLkLIjuxFcjNbFft6ofTMH0U5-M5x3u9-M5hvACZ1QmEHtDlDS1AQHy6ITMAarCYyGrLham7PIcnY78RJ74caZ7sxRRHf-XY-NjKoUW_jAXn5eUy0241Te2ZxTEuI3COPw5xi--krLzzDDe-dsQSK_YHezqUbf2XoxS4Env1o4wevtn7VhROcvGxjJ9TrcL051_NIBz1n5VcmIwZkMeEA58D2O538etU3dcbqG6lma63mWJ97nfS3NyJtZUISfBLCUCffAQjRSfj5q7xlW1zN1U7U1wzDEBdwhbx_kfTZ8LttscnZZSsW340TPgiOEscRZE7gUfRB-HVLpnw3hNXs5UO6uL_64O20NoFplNPknsEn_myoj_GZoxFq3cRIpcDR_pGlLlD2KqHsOFE1r5ZdDB8ogoYra1bqVV_0sAV1ZSxxCqa-UI3or7wzCo9qCU1AYjCFqDhS19Xeu7IH242_IxFN6oHatH__5l7Wnqf_-h8JiBW-MFVE6YTppgMAr4N0QywbZJphehWGRrxTQeKKUXSNFOJhCUuX0mSVEqCFevpXXOktui1_Lcjn1nK9hbAb24wgCH9e1MPEugKrmjQCv9MJWzVXsD3GqCp_wURtmXLh43syRUuhbKYytFOZWCmsrhb2VwtlK4W6l8PJUyTuUy8wgDLGSDYa94WnrGnIfErROnxewAsoJhKtoZynH8VOrn5AJLMuBgjmoBaE_CZ4PlNzrtB-X8ZiEPd-nPrqFWC5N_fKlpISTL139MMomTT5U00Z-33SgjPD_1jCLaE61-ZGn5yH9dqA4bjXnaK2_GZmmB4Q3i37A_BTRN0qVCTopiRTi-wFuK7xXm4ABYHKpYhvol5614vy_xivX1gkvPuSJfvVOJIyZEgBmcH83vMR-rmg0igze5kMXVVVkACFG0ZYQfRuEF_ZVhFoi-Fpd-EDOL-RxkN9kF9AP9ImGSv6qynqLyN48CynbJFEC4CcTxdgZoe2AEHbQG9qBf8dg1Q75qx3skK-W3tj2puat256_2kHmMQ13sXzhUMroCb-41Bg3xKyj7wA4jv0XpWid7Kqe_ib1tJ39aZcJnVM47rCGS8u_a7K6tPmrXZY2jln6SqDISuI2BItk4kNKwgL5E8XNORVVNC9i-SVJHE2CaV7Oig95Qcvx5RuFBSyk2HnlN8Sr-ax6WUKxpr9Ij-M0Xd6uYG4vpjMIyZjOIBvTpArJf-dX3K44qroFUP3VXoGy5JjRDNZhxLeMV3c4tTKKW5cCY9QolgbTILyZ8AYByOJdjgayRuOYn2B2Uy7_Liwcm_g2VbWdHDkvf7uIv3uDc4rPxfbx2g3OAJOKeGuL_NXv5FYWwXG3IIu4LxCe6m4TVrZJy0nqphzSn-G92mZ7whGycGpxoOSffmsd7r_6re7_AU6W75c=

Before screenshot:

image
image

After screenshot:

image
image

@OrderedSet86
Add notes for poe2 ailment threshold implementation, remove poe1 scorch
ff2e094
@OrderedSet86
Partial progress towards ailment threshold
86b763d 
- canDoAilment is currently unconditionally returning false - possible
  bug with calcAverageSourceDamage ?
@OrderedSet86
Fix bugs in last commit, reorganize calc tab, temporarily remove per …
189cc9f 
…stack value (didn't match breakdown values)
@OrderedSet86
Cleanup for PR
0fab122
@OrderedSet86
Remove leftover ConPrintf values
81cc48d
@OrderedSet86
Implement @deathbeam feedback
ce2ce4d 
1. Make effective DPS visible again
2. Make damage per stack visible again (renamed to "Dmg./<ailment>
   Stack" for clarity)
@OrderedSet86 OrderedSet86 force-pushed the add-ailment-threshold branch from 9a226b4 to ce2ce4d Compare January 27, 2025 03:42
Nightblade
Nightblade reviewed Jan 27, 2025
View reviewed changes
src/Modules/CalcSections.lua Outdated
{ label = "Ailment modifiers", modName = "AilmentChance", cfg = "skill" },
}, },
{ label = "Magnitude Effect", { format = "x {2:output:IgniteMagnitudeEffect}", { modName = "AilmentMagnitude", cfg = "ignite" }, }, },
{ label = "Enemy Ail. Thresh.", { format = "{0:output:EnemyAilmentThreshold}", { modname = "EnemyAilmentThreshold" }, }, },
Copy link
Contributor

Choose a reason for hiding this comment

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

modname -> modName

src/Modules/CalcSections.lua Outdated
{ label = "Off hand modifiers", flag = "weapon2Attack", modName = { "EnemyShockMagnitude", "ShockAsThoughDealing" }, cfg = "weapon2" },
{ label = "Enemy modifiers", modName = "SelfShockEffect", enemy = true },
}, },
{ label = "Enemy Ail. Thresh.", bgCol = colorCodes.SHOCKBG, flag = "shock", { format = "{0:output:EnemyAilmentThreshold}", { modname = "EnemyAilmentThreshold" }, }, },
Copy link
Contributor

Choose a reason for hiding this comment

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

modname -> modName

@OrderedSet86
Copy link
Contributor Author

OrderedSet86 commented Jan 27, 2025 •
edited
Loading

The functional part is done for now, should be mergeable as is. I removed the draft tag.

Changes I'd like to do later (all breakdown or non functionality source code improvements):
image

@OrderedSet86 OrderedSet86 marked this pull request as ready for review January 27, 2025 04:31
@OrderedSet86
Copy link
Contributor Author

Updated PR description to accurately reflect changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nightblade Nightblade Nightblade left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@OrderedSet86 @Nightblade