fix: Update of auto apply persistent damage and recovery roll #1395
Conversation
Current pf2e codebase allows this to be simplified. Instead of hooking renderChatMessage, hook preCreateChatMessage. This way we don't have to worry about running on old messages that were scrolled back to. And preCreate is run just on on the client of the user sending the message, not all clients, so there's less performance impact. The extra server update to set a flag when the message is handled isn't needed, since preCreate is only run once. Give the argument a type, so typescript can do type checking. Rather than look in the flavor text for certain html to find persistent damage, use `isDamageRoll` plus a damage instance with `persistent` and `options.evaluatePersistent` set. Instead of searching conditions flavor html to find the condition by translated name, it can be found directly from the `origin.uuid` flag. The All/GM/Players/None and per-user settings do not work as they are described. It turns out they didn't really work before, but no one seemed to noticed, likely because the part that didn't work wasn't widely used. How it actually worked is that the "All" or "GM" choice will use the GM's checkbox for all actors. The non-GM users' settings are (almost) always ignored. The "Players" choice will effectively turn it off. Only in the unlikely case of no GMs being logged in, and a player ending their own turn, would the Players setting and a player user's checkbox get used. However, it wouldn't necessarily be the same user's checkbox as the one who clicked next turn. It could be a different user who logged in first. This commit changes it so that in addition to All and GM using the GM's setting, so will Players, instead of Players meaning always off. Probably would be nice to fix this at some point.
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
Merged manually, so closing PR. (Persistent healing was double-applying for some reason so I fixed that locally and made it work more like you've handled persistent damage.) |
|
I checked the way I found persistent damage would always find it, and not find other damage, or the initial persistent damage card when the player first gets it. But I didn't think to check healing, it's probably getting detected as persistent "damage". I think the persistent healing and damage code likely can be merged, they are probably identical other than which setting is checked. |
|
Last time I tried there was a problem with using applyDamage for healing when passing in a roll. Also, persistent doesn't get set for healing. But, yeah, they could certainly be brought even closer than now. |
Current pf2e codebase allows this to be simplified.
Instead of hooking renderChatMessage, hook preCreateChatMessage. This way we don't have to worry about running on old messages that were scrolled back to. And preCreate is run just on on the client of the user sending the message, not all clients, so there's less performance impact.
The extra server update to set a flag when the message is handled isn't needed, since preCreate is only run once.
Give the argument a type, so typescript can do type checking.
Rather than look in the flavor text for certain html to find persistent damage, use
isDamageRollplus a damage instance withpersistentandoptions.evaluatePersistentset.Instead of searching conditions flavor html to find the condition by translated name, it can be found directly from the
origin.uuidflag.The All/GM/Players/None and per-user settings do not work as they are described. It turns out they didn't really work before, but no one seemed to noticed, likely because the part that didn't work wasn't widely used.
How it actually worked is that the "All" or "GM" choice will use the GM's checkbox for all actors. The non-GM users' settings are (almost) always ignored. The "Players" choice will effectively turn it off. Only in the unlikely case of no GMs being logged in, and a player ending their own turn, would the Players setting and a player user's checkbox get used. However, it wouldn't necessarily be the same user's checkbox as the one who clicked next turn. It could be a different user who logged in first.
This commit changes it so that in addition to All and GM using the GM's setting, so will Players, instead of Players meaning always off.
Probably would be nice to fix this at some point.
follow https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines.)
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this
PR?)
Other information: