Enable Rune Fencer Category 2 Merits, Implement Vivacious Pulse & Swordplay, Fix level for Tactical Parry III on RUN #1339
Conversation
You could use an issue for tracking. Github's markdown parsing makes it easy to make checklists and tables and such |
Good idea. That way it's not just sitting on my local computer. Plus fancier when I can directly reference the issue number. |
|
Transferred that PDF to here. |
| -- add starting tick bonuses if appropriate gear is equipped | ||
| if handsSlotID == 27018 then | ||
| power = 3 * tickPower | ||
| elseif handsSlotID == 27019 then | ||
| power = 5 * tickPower | ||
| elseif handsSlotID == 23218 then | ||
| power = 7 * tickPower | ||
| elseif handsSlotID == 23553 then | ||
| power = 9 * tickPower | ||
| end | ||
| end |
There was a problem hiding this comment.
These will need to be extracted out to an item mod, if you let us know what you need here we can push those changes straight into main so you don't get any conflicts while this branch is ongoing
There was a problem hiding this comment.
"Swordplay" item mod makes sense. I don't think anything else but this particular job specific gear boosts this currently though. These item IDs are all from Futhark Mitons/Futhark Mitons +1/Futhark Mitons +2/Futhark Mitons +3. We'd also need to get the extdata for Sleight of Sword if we replace that with an item mod, since we are no longer "guaranteed" to have the item equipped that will always have said augment. That, or the ID check stays but only for the Sleight of Sword augment bonus.
I have Futhark Mitons +3 in retail so i'll be able to dump the packet/data for the augment if I get instructions on how to do so. I assume NQ and all HQs all share the same augment...
There was a problem hiding this comment.
We -always- want to avoid checking item IDs when possible. And there's always the Chance SE adds something new that would use the mod.
We'd also need to get the extdata for Sleight of Sword if we replace that with an item mod,
That's not that bad. The part that may be a problem is if its in the segment of augment IDs we handle incorrectly atm (some don't display right, but its usually the type that use a the other items typing with the sockets or slotting). That said, I would bet on its ID being nearby the other job-ability enchanting augments already in our table. Once we have the ID its just a matter of filling in the modifier info for it in the aforementioned table.
There was a problem hiding this comment.
I'll see if I can find a cap or even dig through the values for it later.
There was a problem hiding this comment.
Turns out we already have the ID
INSERT INTO `augments` VALUES (1457,0,0,0,0,0); -- Enhances "Sleight of Sword" effectjust needs modifier hooked up where those zeroes are, in augments.sql
There was a problem hiding this comment.
yup, client agrees. I added that aug to an !additem call.
I will need mods for the following:
- Enhances "Battuta" effect
- Enhances "Elemental Sforzo" effect
- Enhances "Sleight of Sword" effect
- Enhances "Inspiration" effect
- Enhances "Elemental Sforzo" effect
- Swordplay
- Liement
- "Enhances "Valiance" and "Vallation" effects" (they are literally the same buff, but one is AoE. Only one mod is needed.)
- Pflug
- "Liement" effect extends to an area
- "Vivacious Pulse" potency (looks like the augment is 1310 according to windower resources, but I can't get Morgelai to display it. Probably some nonsense with the Path A/B/C extdata that I know very little about)
EDIT:
Also need - Augments "Vivacious Pulse"
There was a problem hiding this comment.
"Vivacious Pulse" potency (looks like the augment is 1310 according to windower resources, but I can't get Morgelai to display it. Probably some nonsense with the Path A/B/C extdata that I know very little about)
Windower's resource extractor pulls the dat file index, while our table uses the order the packet does - the 2 lists don't agree and we currently lack a way to match them up. Take a look in the augments.sql file and use the !additem GM command to try nearby IDs starting from where "Slight of Sword" was - its probably nearby.
| [xi.effect.LUX] = xi.mod.CHR, | ||
| } | ||
|
|
||
| local stat = runeStatMap[effect:getType()] |
There was a problem hiding this comment.
If the below is possible to be nil, this will throw an exception and break out of the function if getType does not exist in the above table.
There was a problem hiding this comment.
I adjusted this logic so that nil check is no longer necessary. Take a look!
| if type >= xi.effect.IGNIS and type <= xi.effect.TENEBRAE then | ||
| HPHealAmount = HPHealAmount + getRuneHealAmount(effect, target) | ||
| elseif | ||
| type == xi.effect.POISON or type == xi.effect.PARALYSIS or |
There was a problem hiding this comment.
Would suggest tabling up these items and looping over them as well instead of a very long conditional.
There was a problem hiding this comment.
I adjusted it to a table lookup, like the rune element mapping. tell me what you think
| local handsSlotID = target:getEquipID(xi.slot.HANDS) | ||
|
|
||
| -- add starting tick bonuses if appropriate gear is equipped | ||
| if handsSlotID == 27018 then |
There was a problem hiding this comment.
More things that could go into items.lua
There was a problem hiding this comment.
More things that could go into items.lua
nay, mods mods mods
There was a problem hiding this comment.
I'm punting magic number changes until we get the appropriate item mods into the main branch. It will get fixed in a future PR, and I will note it in the progress chart. They will exist here as reminders for what items need to have their sql entries updated. Don't need to duplicate work especially when we know the better way is on the horizon.
Xaver-DaRed
dismissed
their stale review
Points were adressed. Other points from othe rppl still need to
|
I'll do the mod number additions tonight when I finish work 👍 |
|
Added this PR: #1353 It isn't too bad to add new Mods, you just need to look at the bottom of the C++ list, find the next free number, add them, update the comment and copy things across to the Lua side 👍 |
zach2good
force-pushed
the
feature/runefencer
branch
from
2a25a07 to
72e66d2
Compare
|
Rebased |
|
Not entirely sure why it thinks that many of the commits duplicated... |
There was a problem hiding this comment.
LGTM, just some style nits, and the style checker is mad at you:
scripts/globals/status.lua:1219:5: (W314) value assigned to field INQUARTATA is overwritten on line 1578 before use
If don't want to fight your commits, we can just squash-on-merge, but I'm sure you probably want the credit for multiple commits :P
|
|
||
| xi.job_utils.rune_fencer.useVivaciousPulse = function(player, target, ability, effect) | ||
| return calculateVivaciousPulseHealing(player, target) | ||
| end |
There was a problem hiding this comment.
Newline at end of file please
|
Legitimately have no idea where that redefinition of inquartata came from... I just rebased with upstream/base and never touched that file to my memory. will dig into it |
Ah... I see. I added it myself because it had already existed in the c++ code but not status. Then came along #1353 which happened to merge without errors back into the same exact location. mystery solved. I will just have to remove it on my end. |
|
Content-wise, this looks good and it's ready to go in. But, we can't take all of those duplicate commits. I also don't want to cheat you out of a bunch of commits by squashing everything down into a single commit. Could you rebase and fix your commits? |
|
Agreed, I think it's a bit of a mess too. I will figure it out after work/this weekend. |
* Vivacious pulse is missing Morgelai etc aug bonus due to the aug value not being known at this time.
* Comment unneeded additions to scripts/commands/getstats.lua
* Adjust logic of Vivacious Pulse effect a little
* Actually apply healing in Vivacious Pulse and not just the log message.
WinterSolstice8
force-pushed
the
feature/runefencer
branch
from
d447b55 to
f91cd20
Compare
|
I rebased off upstream/feature/runefencer, then cherry picked the commits that actually were meant to represent the final product. |
Please see attached for this branches progress once merged.
RUN progress.pdf
I got a lot of work done and decided it was time to merge back in since it was getting a little beefy...
This PR implements
Edit: looks like I accidentally included a file in the last commit that modifies scripts/commands/getstats.lua to include Subtle Blow and Store TP in the display... though I suspect that won't be a problem, other than maybe amending the commit message.
I affirm: