zeel01
v3.6.x Bugfixes - See issue list and target this branch for related PRs
This is a tracking PR for various high-priority issues:
Planned fixes:
- #197
- #206
- #215
- #231
- #232
- #233
- #230 / #241
- #242
- #243
Unplanned (but maybe):
- #65
Here's a quick rundown from some local testing in my main three instances (v11/v3, v12/v3, v12/v4).
Probably stale:
- #197: could not repro;
- #231: could not repro;
- #233: could not repro (should be already fixed);
Partially stale:
- #206: could not repro, but it looks like inputExpression is still accessing
actor.data, which is giving errors in v12 (handler.js:47). - #215: could not repro in v12/v3 or v12/v4. In v11/v3, setting a condition immunity caused it to be displayed as
[object set]and then unable to modify;
Current:
- #232: verified current issue;
- #241: verified current issue.
I didn't dive into the commit history yet to check which ones of these "could not repro" have actually been specifically targeted and fixed already, but I thought this would give at least some more insight into the current status and/or prioritization. I'll add info when I have more.
Much appreciated! I'll comment on the ones you couldn't reproduce and ask the reporters if they are still having the problem in the new version.
I feel like #233 wouldn't have fixed itself, I even have a comment in the code there because I knew it was going to be a problem. But there's a good chance one of you guys did fix it because it should have been a big problem.
#206... Input Expressions... right...
#215 if this is resolved for v12 but not for v11 I'm okay with that.
Fairly sure #233 was addressed by @erizocosmico in their v12/v3 branch (that I included).
EDIT: Thought it would also be worth detailing how. Since enrichHTML can't be used asynchronously anymore, it's not in a Handlebars helper. It's instead used to prepare the enriched text in getData().
While I look at a couple other issues, question about Input Expressions and math.js. There's a simple PR on IE's repo that would solve our issues, but I'm also curious if you still want to keep both the git submodule and the math.js Foundry module dependency. Since IE already includes the math.js script in its folder, we could load that one directly and remove the dependency. Let me know if that sounds reasonable.
Merged https://github.com/zeel01/input-expressions/pull/11
The weirdness of the git submodule and dependency is that these modules date back to before dependencies were a thing in Foundry, so they had to include Math.js themselves. When dependencies were introduced, I switched them over to that but the copy of the math.js script I never removed from the other module for some reason.
Weirder still, the functionality of Input Expressions is something I originally created for Monster Blocks, but realized was useful for any sheet so I spun it off on its own. But because IE effects more fields than just Monster Blocks (the default sheet and HUD too) I didn't feel like installing Monster Blocks should automatically install IE and enable it causing side-effects outside of MB.
So instead, I just include the same code into MB as a submodule so that it's bundled in but doesn't activate outside of MB, but you can install Input Expressions to have it in other fields.
It's really not ideal, but I'm not sure if there is a truly better way to handle it. I like using the Foundry dependency system for Math.js (which I really need to update too), but Input Expressions isn't a proper "library" module so making it a dependency would be weird.
Yeah, it's a bit tricky. But also, it's fair to say that it's not something that actually needs to be changed, so we're good (I do recommend updating the Math.js manifest, at least, to avoid the annoying warnings in the setup screen).
If you're ok with it, I think this branch is in a good spot for a new release, unless you want us to look into that #65 first. I personally suggest postponing it just because the module is currently not working on dnd5e 4.3.x, and I've seen a few users anxious for an update already. We could push this one out to get in a stable place, and maybe close up a few old issues before going for enhancements.
By the way, never worked with git submodules. I suppose IE needs to be updated here somehow to point to the new commit?
Yes, we need to update the submodule too, I don't even remember how that's done. https://git-scm.com/book/en/v2/Git-Tools-Submodules naturally, it's overcomplicated.