raidboss: updates to UCOB #5395
Conversation
| let bahaDirIdx; | ||
|
|
||
| for (const mob of data.combatantData) { | ||
| const mobDirIdx = Math.round(4 - 4 * Math.atan2(mob.PosX, mob.PosY) / Math.PI) % 8; |
There was a problem hiding this comment.
positionTo8DirOutput returns the direction name from dirNumToName, but we actually need the array index (or, more precisely, the direction number), because we use the direction number in naelDirIdx and bahaDirIdx to calculate the safe spot (based on opposite-Bahamut, possible adjustment for Nael).
If we wanted to use positionTo8DirOutput here instead of directly calling Math.atan2, we could grab the direction name, but then we'd have to do an indexOf lookup to get back to the direction number, which feels clunky.
There was a problem hiding this comment.
As an aside, given that we seem to use relatively common variants of positionTo8Dir now in various fights, perhaps these (and, optionally, their 4Dir and 16Dir variants?) would be good candidates to add to a function library, so we're not repeating this each time? If you agree, I'd be happy to submit a PR, but not sure where would be the right place for them to live.
There was a problem hiding this comment.
Maybe it'd be better to have positionTo8Dir instead of positionTo8DirOutput and you could look up from there and use it in all places? Personally I think it's fine to re-do the number=>string lookup, but the math part would be nice to keep in one spot.
Agreed that it'd be good to put these in common locations. I think a good common location could just be util.ts. We could make some new file for "common raidboss functions" or "raidboss helpers" too (since watch combatants might be a better fit in there than util). One problem is that a couple of fights have conflicting ideas of what 0 means (there's like ~2-3 that want to have 0=NW and most are 0=N) and so those would need to be refactored/renamed as well.
There was a problem hiding this comment.
Sounds good. Let's pause this PR for now, and I'll open up a new one to add some functionality to util. If & once you're good with it and land it, I can revise this PR to use the new functions and then do a follow-on PR for the other fights.
|
Marking as draft for now to work on landing some position-related stuff to |
@quisquous Per our discussion on #5395 / discord, here's some direction-related helpers to be added to `util.ts`. I opted to export all of the functions in a single `Directions` object, rather than to export each function individually. Let me know if you'd prefer to have them live in `Util{}` or some other preference.
…lpers (#5402) @quisquous Per our discussion on #5395 / discord, here's some direction-related helpers to be added to `util.ts`. I opted to export all of the functions in a single `Directions` object, rather than to export each function individually. Let me know if you'd prefer to have them live in `Util{}` or some other preference. 0c55ccc
|
@quisquous This is now updated to use the new After some further thought, I moved the three arrays of output strings ( |
resources/util.ts
Outdated
| export type DirectionOutput8Map = { [key in DirectionOutput8]: OutputStrings }; | ||
| export type DirectionOutputCardinalMap = { [key in DirectionOutputCardinal]: OutputStrings }; | ||
| export type DirectionOutputIntercardMap = { [key in DirectionOutputIntercard]: OutputStrings }; | ||
| export type DirectionOutput16Map = { [outputString: string]: OutputStrings }; |
There was a problem hiding this comment.
One last change request for me is that I don't know that these types are that useful anymore. It'd probably be worth just moving the { [outputString: string]: OutputStrings }; down to the various output8Dir types, e.g. const output8Dir: { [outputString: string]: OutputStrings }; = [ and removing these exports at this point.
There was a problem hiding this comment.
Yeah... I liked the idea of explicit typing, but it didn't hold up when it came time to actually use them in the outputStrings{} in triggers. Alas.
I do think it's worthwhile still to keep the explicit typing on the lookup arrays (e.g. output8Dir: DirectionOutput8[]), but I agree it makes sense to remove them on the outputStrings objects like you suggest. If that works for you, this is good to go.
|
Thanks for all the changes and the helper functions! |
Added some additional triggers for tank cleaves/busters, some minor cleanup, and new triggers for QM, Blackfire, Heavensfall, and Grand Octet. Also added clockwise/counterclockwise to resources/outputs, since it tends to come up reasonably often. c231d22
Added some additional triggers for tank cleaves/busters, some minor cleanup, and new triggers for QM, Blackfire, Heavensfall, and Grand Octet. Also added clockwise/counterclockwise to resources/outputs, since it tends to come up reasonably often. c231d22
Added some additional triggers for tank cleaves/busters, some minor cleanup, and new triggers for QM, Blackfire, Heavensfall, and Grand Octet. Also added clockwise/counterclockwise to resources/outputs, since it tends to come up reasonably often. c231d22
Added some additional triggers for tank cleaves/busters, some minor cleanup, and new triggers for QM, Blackfire, Heavensfall, and Grand Octet. Also added clockwise/counterclockwise to resources/outputs, since it tends to come up reasonably often. c231d22
Added some additional triggers for tank cleaves/busters, some minor cleanup, and new triggers for QM, Blackfire, Heavensfall, and Grand Octet. Also added clockwise/counterclockwise to resources/outputs, since it tends to come up reasonably often.