PDA

View Full Version : Damage not flagged as melee or ranged



thejeff79
December 10th, 2021, 14:05
Good Morning,

I have a player that's running a Barbarian / Wild shaping druid, so basically NPC beasts with Rage effects. I've found an bug around applying the additional damage effect to Melee attacks.

The Damage formula in the NPC Actions is not picking up the proper damage type from the Attack portion of the action definition. This is using Fantasy Grounds Unity v4.1.12 Ultimate, using the 5E ruleset and Monster Manual NPCs

We have the Effect from the Wiki: Rage;ADVCHK:strength;ADVSAV:strength;DMG:2 melee;RESIST:bludgeoning,slashing,piercing; Friends ADV 5ft that we're applying to the NPC in the combat tracker

If I, as the GM, invoke the attack from the Combat Tracker action, everything works as expected: [DAMAGE (M)] Beak [EFFECTS +2] [TYPE: slashing (1d8+4=6)]

Now, what's interesting is if I make the same Attack Hit and Damage roll from the NPC character sheet, which is what my Player will have to do: [DAMAGE] Beak [TYPE: slashing (1d8+2=4)]


In this case, the actual damage roll is no longer flagged as being a Melee attack, so the effect does not proc. If I remove the melee specific requirement of the effect, and just do a DMG: 2 then the effect procs as expected.


50276

Moderator: Moved to House of Healing

Zacchaeus
December 10th, 2021, 14:19
Yes, someone else brought up the point that damage wasn't flagged in NPCs as being melee or ranged. Not sure if this is a bug or whether it is expected behaviour.

SilentRuin
December 10th, 2021, 17:01
Good Morning,

I have a player that's running a Barbarian / Wild shaping druid, so basically NPC beasts with Rage effects. I've found an bug around applying the additional damage effect to Melee attacks.

The Damage formula in the NPC Actions is not picking up the proper damage type from the Attack portion of the action definition. This is using Fantasy Grounds Unity v4.1.12 Ultimate, using the 5E ruleset and Monster Manual NPCs

We have the Effect from the Wiki: Rage;ADVCHK:strength;ADVSAV:strength;DMG:2 melee;RESIST:bludgeoning,slashing,piercing; Friends ADV 5ft that we're applying to the NPC in the combat tracker

If I, as the GM, invoke the attack from the Combat Tracker action, everything works as expected: [DAMAGE (M)] Beak [EFFECTS +2] [TYPE: slashing (1d8+4=6)]

Now, what's interesting is if I make the same Attack Hit and Damage roll from the NPC character sheet, which is what my Player will have to do: [DAMAGE] Beak [TYPE: slashing (1d8+2=4)]


In this case, the actual damage roll is no longer flagged as being a Melee attack, so the effect does not proc. If I remove the melee specific requirement of the effect, and just do a DMG: 2 then the effect procs as expected.


50276

Moderator: Moved to House of Healing

Does not matter if you run the NPC sheet from host or player (client), it will not recognize the (M) melee damage. Only the CT entry will parse damage correctly. I would for sure consider this an outright bug - it does happen in a raw 5e campaign with any NPC sheet loaded in CT.

thejeff79
December 10th, 2021, 20:59
Actually, this looks to be an easy fix.

The code in question is in the 5e.pak file, in /scripts/manager_power.lua
Starting at parseNPCPower() on line 1827, ultimately calling parsePower() on line 1861
In my example, I'm looking at the Axe Beak, which has the 'Beak' action. This would make the variables:
sPowerName = "Beak"
sPowerDesc = "Melee Weapon Attack: +4 to hit, reach 5 ft, one target. Hit: 6 (1d8 + 2) slashing damage."

The description gets sanitized and split, then the Name and array[Words] get passed to parseAttacks() and parseDamages() on lines 1815 and 1816

parseAttacks() sets the range correctly, parseDamages() does not.

The reason for this is the conditional on line 988 within parseDamages. Specifically, elseif sPowerName:match("^[Mm]elee") then
Now, sPowerName here is "Beak", so that definitely doesn't match... However, if I add a new action to the Axe Beak named 'Melee Beak', the range gets set correctly, as demonstrated in the attached screenshot.

I think the fix is to simply look at the first word in aWords
Line 988
if sPowerName:match("^[Rr]anged") then becomes if StringManager.isWord(aWords[0], "ranged") then

Line 990
elseif sPowerName:match("^[Mm]elee") then becomes elseif StringManager.isWord(aWords[0], "melee") then

50278

Moon Wizard
December 13th, 2021, 21:33
I took a quick look; and the suggestion from @thejeff79 seemed like a reasonable solution that should catch most situations. I'll add to the current beta testing.

Regards,
JPG

thejeff79
December 13th, 2021, 21:41
Great, Thanks for looking into it

rhagelstrom
December 15th, 2021, 21:24
[5E] NPC sheet action damage phrases not picking up melee/ranged trait. Fixed.

I tried this and I still see this as this issue not working.

SilentRuin
December 15th, 2021, 21:26
[5E] NPC sheet action damage phrases not picking up melee/ranged trait. Fixed.

I tried this and I still see this as this issue not working.

Looking at the above fix - I don't see anything about fixing the chat output. But I don't remember how to actually test the functionality as presented in the above code. So if someone knows how to truly test it (not just see chat has [M] or not) that would be the telling point.

Moon Wizard
December 15th, 2021, 22:04
Just pushed hot fix for this.

Regards,
JPG

SilentRuin
December 15th, 2021, 22:10
Just pushed hot fix for this.

Regards,
JPG

Just ran with it -
I used the legit "DMG:2 ranged" straight out of dev doc for 5e effects and it did not seem to respect that when I switched to "DMG:2 melee" or the ranged. The CT action for the same crossbow, light on the NPC however worked and respected if it was ranged or melee appropriately.

SilentRuin
December 15th, 2021, 22:14
Just pushed hot fix for this.

Regards,
JPG

50342

Fix failed.

Moon Wizard
December 15th, 2021, 22:16
I'm talking specifically around the fact the the (M) and (R) were missing from the damage rolls from the NPC sheet, using the Axe Beak as the specific example.

I'll need more information if that data is not showing up in other records. (i.e. exact NPC affected, whether it displays the data, and what kind of test you are doing)

Regards,
JPG

Moon Wizard
December 15th, 2021, 22:17
Also, please disable other extensions before testing.

JPG

SilentRuin
December 15th, 2021, 22:19
I'm talking specifically around the fact the the (M) and (R) were missing from the damage rolls from the NPC sheet, using the Axe Beak as the specific example.

I'll need more information if that data is not showing up in other records. (i.e. exact NPC affected, whether it displays the data, and what kind of test you are doing)

Regards,
JPG

I'll try axe beak but really - don't see difference between one NPC and another. Regardless of the the "weapon" text in the NPC main page that gets translated into the CT action tab. So far as I and another have tried - does not matter what on NPC main page is used the M/R is not taken into account (effect DMG:2 ranged or DMG:2 melee is easiest most simple test) but is on the CT action entry as shown in my picture from earlier post.

SilentRuin
December 15th, 2021, 22:30
I think you did fix it, it is completely my problem. When I added in my weapon I did not add in the proper syntax as the text is defined - but the parse does find the M/R and puts it into the CT action tab correctly ( you can see it in my image) - so I stand corrected - if the NPC main page syntax is in a form it can grab the [R] or [M] it works - if not it does not.

Kelrugem
December 15th, 2021, 22:38
I can confirm that the chat messages now work :) (and therefore the effects like DMG: 2 ranged now also work, since their code checks the chat message for the attackfilter if I am not wrong. Regardless of that, I tested the effect and it works, too :) )

SilentRuin
December 15th, 2021, 22:43
Also, please disable other extensions before testing.

JPG

I did with no extensions the last test. And I have a related question as I don't understand how the parser into CT actions can understand the Hit is ranged or melee but the text parser for NPC main does not.

Look at any spear or javelin (say on an orc) for the ranged hit and you'll see its still a M when it processes which is incorrect. Same for my custom weapons - if I put Melee Weapon Attack: x followed by a Hit: y the hit will not be found as M or R. If I do an out of the box orc with a javelin which has a Melee attack and ranged attack with different hits - both hits will be interpreted as M.

I need to know the exact legitimate syntax on the text side of things to make this stuff work. What is it? Because the CT action representation works always - for my stuff or the orc out of the box - but neither of those work from the text side.

I think you can understand there is still a problem here.

Kelrugem
December 15th, 2021, 22:49
I did with no extensions the last test. And I have a related question as I don't understand how the parser into CT actions can understand the Hit is ranged or melee but the text parser for NPC main does not.

Look at any spear or javelin (say on an orc) for the ranged hit and you'll see its still a M when it processes which is incorrect. Same for my custom weapons - if I put Melee Weapon Attack: x followed by a Hit: y the hit will not be found as M or R. If I do an out of the box orc with a javelin which has a Melee attack and ranged attack with different hits - both hits will be interpreted as M.

I need to know the exact legitimate syntax on the text side of things to make this stuff work. What is it? Because the CT action representation works always - for my stuff or the orc out of the box - but neither of those work from the text side.

I think you can understand there is still a problem here.

I attached an image of the NPC I am testing which is working for me:


https://www.fantasygrounds.com/forums/attachment.php?attachmentid=50343&stc=1&d=1639608523

Kelrugem
December 15th, 2021, 22:50
But I can confirm that the orc javelin is faulty. Probably because the javelin first lists a melee entry. Even if I click on the ranged javelin damage in the NPC sheet, it designates it as melee damage :) (speaking of the orc in the 5E basic monster module)

SilentRuin
December 15th, 2021, 22:56
But I can confirm that the orc javelin is faulty. Probably because the javelin first lists a melee entry. Even if I click on the ranged javelin damage in the NPC sheet, it designates it as melee damage :) (speaking of the orc in the 5E basic monster module)

Yeah the syntax I'm using for a single weapon (like club in my image) is not picking it up either. But of course is in my NPC CT actions entry. I need a guaranteed syntax that keyed in by hand will be parsed correctly on the NPC sheet side. My current take away is only the CT actions entry is the only valid place to execute any damage for an NPC.

And I tried keying in exactly what club has for out of the box and still was not picking up damage. Though I have newlines in mine. IS that the issue? I can't have newlines ever?

Kelrugem
December 15th, 2021, 23:00
Yeah the syntax I'm using for a single weapon (like club in my image) is not picking it up either. But of course is in my NPC CT actions entry. I need a guaranteed syntax that keyed in by hand will be parsed correctly on the NPC sheet side. My current take away is only the CT actions entry is the only valid place to execute any damage for an NPC.

And I tried keying in exactly what club has for out of the box and still was not picking up damage. Though I have newlines in mine. IS that the issue? I can't have newlines ever?

I tested the orc javelin with and without a newline before the ranged information, and it never worked correctly :) so, doesn't look like that this is the culprit, but I do not know the code :) not sure about your club though

So far I can at least confirm that there is still a faulty behaviour for weapons with a melee and ranged option in the info box :)

SilentRuin
December 15th, 2021, 23:07
ARRGGGGGHHH... Its not newline. It's if there is ANY character before Melee Weapon Attack it won't pick the damage up no matter what.

Which also accounts why the orc javelin fails. It basically parses the first one no matter what your doing.

To fix my NPC main page weapon syntax I enter I'd have to insure NO TEXT (can have as many newlines as I want) occurs before it hits line with Melee Weapon Attack in it. So in my case as I'm putting out raw DB titles and entries of the actual data means that attack syntax has to come first in order for Main Page execution to work. Parsed CT action stuff will always work. I would hope this stuff would work regardless - but evidently not. To get both NPC main page and CT actions to execute properly that is what you have to do.

Before I change all my syntax to conform - I'd like to know if I really have to - or if instead a legit fix that works based on the block of text that the Hit syntax is associated with regardless of what precedes it is coming out soon.

Right now its like luck. And of course things with dual purpose will never work for the hit in NPC main page as its written to parse right now.

thejeff79
December 15th, 2021, 23:24
Actually, Is it an OS specific issue with newlines?

Looking at ParsePower, the String split line is:

local aWords, aWordStats = StringManager.parseWords(sLocal, ".:;\n");

So, '\n' is being passed in as a delimiter. However, in Windows, a new line can actually be Carriage Return, New line: '\r\n'

The parsing logic looked pretty positionally dependent, so it that Carriage Return isn't being properly skipped, it could account for an unexpected word and hence things not parsing as expected.

rhagelstrom
December 15th, 2021, 23:29
Just pushed hot fix for this.

Regards,
JPG

Works. Thanks for the super speedy support

SilentRuin
December 15th, 2021, 23:50
Actually, Is it an OS specific issue with newlines?

Looking at ParsePower, the String split line is:

local aWords, aWordStats = StringManager.parseWords(sLocal, ".:;\n");

So, '\n' is being passed in as a delimiter. However, in Windows, a new line can actually be Carriage Return, New line: '\r\n'

The parsing logic looked pretty positionally dependent, so it that Carriage Return isn't being properly skipped, it could account for an unexpected word and hence things not parsing as expected.

All I can speak for is windows. I can have as many new lines in it as I wish - but one character in it and the Hit can't tell M or R in NPC main page. And of course multiple uses (like spear or javelin) never work for the 2nd defined use.

thejeff79
December 16th, 2021, 02:05
Having the Melee and Range options in the same description certainly complicates the parsing, and in that case looking at the first word in aWords won't give the proper answer for the second item. We need the last occurring instance of the term.

As you iterate over i in parseDamages() you'd need to track the last observed occurrence of "<range> [weapon,spell] attack". Then that value can be used as the range on the attack.

local lastObservedRange = ""
while aWords[i] do (line 971)
if StringManager.isWord(aWords[i], "attack") then
lastObservedRange = aWords[i-2];
end
-- MAIN TRIGGER ("damage")
(line 992)
if StringManager.isWord(lastObservedRange, "ranged") then
rDamage.range = "R";
elseif StringManager.isWord(lastObservedRange, "melee") then
rDamage.range = "M";
end

SilentRuin
December 16th, 2021, 05:43
Having the Melee and Range options in the same description certainly complicates the parsing, and in that case looking at the first word in aWords won't give the proper answer for the second item. We need the last occurring instance of the term.

As you iterate over i in parseDamages() you'd need to track the last observed occurrence of "<range> [weapon,spell] attack". Then that value can be used as the range on the attack.

local lastObservedRange = ""
while aWords[i] do (line 971)
if StringManager.isWord(aWords[i], "attack") then
lastObservedRange = aWords[i-2];
end
-- MAIN TRIGGER ("damage")
(line 992)
if StringManager.isWord(lastObservedRange, "ranged") then
rDamage.range = "R";
elseif StringManager.isWord(lastObservedRange, "melee") then
rDamage.range = "M";
end

@MoonWizard that would work for everything I have a problem with right now I think.

Moon Wizard
December 16th, 2021, 06:21
It’s not something I feel comfortable with putting a quick fix out for.

The reason why the CT stuff works is that it only pulls the range data from the attacks; and then the CT rolls pull from the overall line after the fact. The damage range field is not even used at all in the CT conversion.

There is no linkage in the basic parsing of the data between attack and damage rolls in the core parsing.

If you put Melee/Ranged first in the description like the 5E official material; it should work fine.

JPG

Moon Wizard
December 16th, 2021, 06:21
Also, it has been this way for years, so this is not something that was changed in this release.

Regards,
JPG

SilentRuin
December 16th, 2021, 08:22
Also, it has been this way for years, so this is not something that was changed in this release.

Regards,
JPG

???
Why would it matter how long it’s been out there? Things like spear and javelin flat out don’t work and this fix might fix it. Honestly, most bugs I find were “always there” which has no bearing on them needing to be fixed.

Kelrugem
December 16th, 2021, 08:36
I think you speak about different things; SilentRuin, you speak now about javelins, while Moon Wizard was commenting the newline stuff. So, let me post an example of what is not working right now and what SilentRuin meant :) This is not about the newline stuff commented above

Moon Wizard, an Orc has the following javelin text in its sheet:

Melee Weapon Attack: +5 to hit, reach 5 ft., one target. Hit: 6 (1d6 + 3) piercing damage. Or Ranged Weapon Attack: +5 to hit, range 30/120 ft., one target. Hit: 6 (1d6 + 3) piercing damage.
Observe it has two attack options, one melee and one ranged. If I use the damage text, then this parses as melee damage, not ranged. The ranged attack however works as ranged :)

LordEntrails
December 16th, 2021, 15:19
???
Why would it matter how long it’s been out there? Things like spear and javelin flat out don’t work and this fix might fix it. Honestly, most bugs I find were “always there” which has no bearing on them needing to be fixed.
In that since it has been this way for awhile it doesn't justify the risk of a Hot Fix. It doesn't mean it doesn't justify getting fixed.

SilentRuin
December 16th, 2021, 15:59
In that since it has been this way for awhile it doesn't justify the risk of a Hot Fix. It doesn't mean it doesn't justify getting fixed.

Fix did not fix it completely. Hot fix though fixing it a bit more - still did not fix it completely.

I guess I've lived in a different programming world, including what I program here. I don't distinguish between a bug and the type of fix I provide. I fix it unless there is a technical risk/reason not to do it and live with the bug.

That is not the case here IMHO. You can disagree - but for sure I don't accept these arguments on the type of bug or fix having any bearing on things. Bug = kill it. Or give a reason why it technically cannot or should not be fixed.

In the end, this is all low priority to me. If so - just say that. Don't make up these ridiculous reasons based on what you call the bug or fix. Personally, I can't tolerate a known bug that I personally consider a bug not being addressed (no technical risk/reason to not do it). So I really don't understand the reasoning I'm being given. And I've worked in way more complex and bug ridden legacy code than this, so being told "I don't know what it's like" is not going to fly.

Low priority and busy - fine.

But if fix is simply sitting there ready to try out provided by a community member? Try it. Or reason out why the risk is to great to do it.

Either way - I'm just going to change my stuff to get syntax in the magic (no text preceding) so that at least things with single entries (not spear/javelin types) work as best they can. A bug to be easily squashed that I can do on my side of things with minimal risk. So I do it.

LordEntrails
December 16th, 2021, 17:21
Remember, I'm not a SW developer and certainly can't speak for Moon. But what I can do is try to help foster understanding based upon my experience (both on this board and in real life as a business analyst and scrum master for a software development team) in the conversations here on the forum.

What I see is that SmiteWorks addresses issues (bugs, features, enhancements, etc) in two ways. One is through their 'regular' development cycles. These are often made available in the Test Channel for public beta testing, but not always. These are the releases that increment their version numbering. These go through whatever they do for QA internally.

Then they occasionally release "Hot Fixes" that do not increment version numbers, are almost never available in the Test Channel and are things that do not require anything more than unit testing. i.e. things like typos or very minor changes where the impacts of the change are obvious and the risks of impacting other parts of the code are considered trivial. These are often identified and resolved in hours.

In this case, Moon as stated

It’s not something I feel comfortable with putting a quick fix out for.
So I don't think Moon is saying or implying that this is something they will not fix, but rather that it is something that he will not Hot Fix. So it means it goes into the 'regular' development channel for fixing.

SilentRuin
December 16th, 2021, 17:26
Remember, I'm not a SW developer and certainly can't speak for Moon. But what I can do is try to help foster understanding based upon my experience (both on this board and in real life as a business analyst and scrum master for a software development team) in the conversations here on the forum.

What I see is that SmiteWorks addresses issues (bugs, features, enhancements, etc) in two ways. One is through their 'regular' development cycles. These are often made available in the Test Channel for public beta testing, but not always. These are the releases that increment their version numbering. These go through whatever they do for QA internally.

Then they occasionally release "Hot Fixes" that do not increment version numbers, are almost never available in the Test Channel and are things that do not require anything more than unit testing. i.e. things like typos or very minor changes where the impacts of the change are obvious and the risks of impacting other parts of the code are considered trivial. These are often identified and resolved in hours.

In this case, Moon as stated

So I don't think Moon is saying or implying that this is not something they will fix, but rather that it is not something that he will Hot Fix. So it means it goes into the 'regular' development channel.

I agree with what your saying.
However, my personal experience is that when the spotlight is taken off a problem, it becomes like a wishlist thing, and IMHO that means this is going to happen (sigh) ;) 50355