update to 0.8.6 #13
update to 0.8.6 #13
Conversation
make the module work for 0.8.6 and not throw deprecation warnings. Ideally it is advised to rework some of the parts to work more natively with 0.8.x
src/about-face.js
Outdated
| const scene = token.object.scene; | ||
| if (!AboutFace.sceneEnabled) return; | ||
| token = (token instanceof Token) ? token : canvas.tokens.get(token.id); | ||
| if (!token.children.includes(AboutFace.tokenIndicators[token.id].c)) { |
There was a problem hiding this comment.
I don't know if we want to use that kind of coding standard, but I would move this condition into a named method so we can easily know what is being checked.
I don't know if it's because I am still new to module development, but I have no idea what is being checked here other than we check for a specific children.
There was a problem hiding this comment.
Yeah, I didn't want to use something like that as well, but for some reason it was removing the indicator from the children of the token, I wasn't able to understand why, but I really wanted the module to work, so I did this. It would be great if you could find a better solution.
There was a problem hiding this comment.
You're closer to the solution than I am. I won't be able to suggest a better solution than this without going as deep into the code as you did.
However to make life easier for the next person coming across this code, the condition !token.children.includes(AboutFace.tokenIndicators[token.id].c) should be moved into a function or const with a name similar to tokenChildrensIncludes###something### and the use that const inside the if. I would do it, but I do not know what AboutFace.tokenIndicators[token.id].c represents.
There was a problem hiding this comment.
I moved it to AboutFace.tokenIndicators[token.id].c to a separate variable and added some comments.
|
Any news on this pull request? Seems like some much needed updates.. |
make the module work for 0.8.6 and not throw deprecation warnings. Ideally it is better to rework some of the parts to work more natively with 0.8.x