raidboss: add support for The Merchant's Tale (Advanced)#1071
raidboss: add support for The Merchant's Tale (Advanced)#1071ForestSageSarah wants to merge 10 commits into
Conversation
JLGarber
left a comment
There was a problem hiding this comment.
Thanks so much for putting all this together. I don't know how much the regular maintainers here do the Variant/Criterion content, so we really appreciate anyone else willing to work on it. Do you know whether you're likely to be able to put all the timelines for Advanced together? I can check through my logs to see whether there's anything useful there.
Can you clarify the source of your translations? I can read enough French/German to be comfortable with those, but I don't read our other supported languages. If you speak all of them, that's perfectly fine, but if any are machine-translated, let's leave those out and let the community translators handle them instead.
We appreciate your contribution!
Thanks so much! I am planning on completing all three fights (Pari / Seamaid / Swordmaster) - would it have been better to wait until I had the baseline done for the other 3 to PR?
They are a mix of ML and directionals I grabbed from the Aloalo Island file I used as a template - definitely better for the community to translate instead then! |
Co-authored-by: Jonathan Garber <linkthevaliant@gmail.com>
Co-authored-by: Jonathan Garber <linkthevaliant@gmail.com>
Co-authored-by: Jonathan Garber <linkthevaliant@gmail.com>
Typically we do try to make non-Ultimate pieces of content one self-contained package, or "one trigger package/one timeline package" etc, but if you're actively planning to handle the other bosses it probably doesn't matter too much. This is especially true for Advanced Variant, where bosses are encountered in a non-fixed order. That being said, I'll tag in @xiashtra here to assist with clarifying, since this isn't a situation we come across as much and I don't want to give an answer that's my opinion but doesn't fully align with the docs. As for the translations, anything that's directly sourced from another file in the repository is fine, to be clear, so long as the mechanical handling matches exactly. (For example, if there's a specialized translation for a normal-mode raid's output strings, it is fine and expected to re-use that for Savage if it's unchanged mechanically.) What we generally want to avoid is machine translations for new text content. |
Either one PR for the entire instance or one PR per boss should be fine. It can be preferable to submit one PR per boss if it will take a while to finish all of them, so that they can be checked/approved and released incrementally. |
| netRegex: { effectId: '301' }, | ||
| condition: Conditions.targetIsYou(), | ||
| suppressSeconds: 2, | ||
| response: Responses.breakChains(), |
There was a problem hiding this comment.
| response: Responses.breakChains(), | |
| response: Responses.breakChains(), |
This will resolve the lint error. You should use the lint and test scripts locally to check your PR before submitting.
There was a problem hiding this comment.
To follow up on this, most linting errors can be fixed automatically with npm run lintfix. Other npm run <foo>fix targets exist for other linting types as well.
Adds initial raidboss support for Pari of Plenty in The Merchant's Tale (Advanced).
Included:
ToDo