Refactor Hooks into atomic hooks
Description of feature
At present we have all out Hooks in files which match the hook type and have multiple unrelated "functions" being called within that single hook.
This is IMO suboptimal as it means the following:
- Doing multiple things in a single hook is fragile, if the first thing fails then the 2nd and 3rd won't trigger
- Long files doing multiple things are hard to keep in your brain
Example (snipped for brevity):
const actorHooks = () => {
Hooks.on("preUpdateActor", async (doc, updatedData) => {
LOGGER.trace("preUpdateActor | actorHooks | Called.");
// #1.
if (updatedData.system?.stats?.emp) {
...
}
// #2.
if (updatedData.system?.stats?.emp && doc.system.foobar) {
...
}
// #3.
if (updatedData.system?.stats?.emp || updatedData.system?.stats?.luck) {
...
}
});
}
In this example if #1
errors out for any reason #2
and #3
will never trigger.
These should files should be split into more specific files to provide both readability and separate concerns. An updateActor
hook where we run something for BlackICE should not be in the same file as a hook where we update a Mook for example.
For example in MR !1249 we added a new hook for updating tracked armor when an armor item is edited. This should have probably gone in hooks/item.js
but we currently have code in there which does stuff to Role items so the new functionality was created in hooks/items/update-tracked-armor.js
. This seperates the armor code from the role code for this hook event meaning no overloading hooks and making sure that the modifyTrackedArmor
functionality will always run whether or not the Role code runs successfully.