Going back through the thread and the critical dice don't seem to work properly; even when it doesn't crit, the crit dice still apply.
Printable View
Going back through the thread and the critical dice don't seem to work properly; even when it doesn't crit, the crit dice still apply.
I'm not sure what your issue is from the posted image.Quote:
Originally Posted by metaldm007
The extension will 'roll' every dice it needs for the roll, and then when applied to a target the 'corrected' value is applied. ( I dont see you apply any damage in the image, just an non-target roll. )
Attachment 46571
See the attached example of a 'shortsword' that I have added a '1d8 slashing,critical' extra line of damage.
The first line shows d6+d8+3 = 3+1+3 = 7, this is the total roll result. No damage has been applied to a target.
The second line shows an 'Advanced Weapon Damage' with just the d6+3=3+3=6 box because this was not a critical roll.
The third line shows the 'Damage [6] -> cat', which is the correct damage from this total roll from the first line when 'applied' for the cat as a non-critical roll.
I've then added a forced critical roll...
The forth line shows a d6+d8+g6+3=1+4+5+3=13, which you can see the '1d8 critcal' dice was not 'double rolled' but the d6 is correctly double rolled (g6).
The 5th line shows the total 'damage[13]->cat'.
If you have a specific case I'm happy to take a look, but I will need to see a roll and it applied to a target.
NOTE: the extension will roll every dice is needs because at the 'roll' point it does not know what the target is. If you dont have a target you get the first line, you can that drag that box from chat to CT/map to apply to a target at which point it will do as my targeted example shows and calculate which sections of the damage line apply. If you get an 'advanced weapon damage' line this is because 'some' of the lines were not applied and you get a report of the dice it is going to apply. ( Hence line 2, but no such report line for the critical attack because all lines were applied to the cat. )
Just wondered if you are aware of any conflicts with other exts. When I run the ext in a clean campaign with just this ext running its working fine, when I use it in another campaign in which I use a lot of exts it is adding both damages together (both campaigns I used hew with the same character).
Thats a new one to me, You will have to try and work out which extension it is conflicting with. I can not own and check every possible extension conflict. But I do try and fix them when they are detected and reported.Quote:
Originally Posted by Leprekorn
If you could try a binary chop of extensions, ie copy the campaign and then turn off half the extensions.. if it still has the issues then repeat until you find which extension conflicts.
Sorry I can not help you reduce the number, but I have no idea which extensions you are using, so I just can not help without know which conflicts for you. Its normally reasonable to fix and get them working together. ( most of the time. )
Just because I dont know of any at the moment, I do have patches for a few, and they might have made changes that break those patches. But it does require a user to report and narrow it down.
You might have multiple extension that cause issues. They are a pain because of how the ruleset defines the character sheet, so a lot of extensions end up changing the 'powers' page which has the spell and weapon buttons. So it happens a bunch to get conflicts.
Let me know if you find which one it is, I'll then try and get hold of it and replicate the issue and update to between the extensions who ends up fixing it. ( Sometimes the fix ends up in the other extension and not mine. )
Thanks, Pete
No problem I will report back.
Just a quick note....Quote:
Originally Posted by Leprekorn
Have you checked if it is applying multiple damages to the target ? ( Or just listing it in the chat ? )
Advanced weapon damage will list the 'full' dice roll and then a listing for what was applied to the creature.
So if you have a weapon, '1d6' with a second damage line '1d6 type(giant)',
When you roll, you will get a chat line for,
1. Advanced Weapon Damage 1d6 + 1d6 = roll total...
2. (for a non giant ) It will then list a 'Damage 1d6', with the 'wolf damage (1d6=3)' type line in chat.. and 3 damage applied to the wolf...
3. if you had multiple targets and the other was a giant, you would also get the lines for 'giant 1d6+1d6=7', 'giant takes (7) damage' with the resistances etc..
This might be the case you are seeing if you are new to the extension, or it could be a real double damage application.
See awd.jpg attached for a 'type(giant)' damage, with 11 damage going to the non-giant, and the full 17 damage going to the giant.
It's Def the combined damage of the max and non max rolls.
Its something to do with constitutional Amendments ext. I used hew against a plant creature and it calculated the damage max + dice roll when CA was active but not when it wasn't.
Is there a conflict with max?
I tested in a new campaign with just CA and AWDE and SRD loaded ragged both lightbringer and hew item into the actions tab from the AWDE mod. Added a plant based creature and undead to the CT, tried lightbringer against the undead worked properly, tried lightbringer vs plant creature worked properly. Tried Hew against plant creature combined the max and the rolled damage worked incorrectly. Tried hew against the undead worked properly only applying the normal damage not maxed. Added type plant to the undead tried hew again worked incorrectly combining the max and the normal damage. I think it has something to do with the max damage trait.
I was thinking this one, we have had 'issues' with conflicts in the past. Most were sorted out.Quote:
Originally Posted by Leprekorn
It sounds like we both use 'max' for different things. I AWD 'max' forces that line dice rolls to the 'max' value for the dice. While CA I think calculates a total max against a creature.
Which version of CA are you using ? ( Is it on the free forums, forge or dmsguild... I dont remember.. I was free if I recall. )
Forge
Ive grabbed the forge version, My guess is they way the word 'max' is been used.. I'll take a look but I'm doing spell tokens code today.
Have you reported it on CA forum side ?
Yes I did, but they don't own your extension so can't work on it. Maybe you could send them a copy?. BTW I am running 2.0.4 version of their ext and 2.1 version of advanced weapon damage.
I had a quick look, and I dont see a solution.Quote:
Originally Posted by Leprekorn
Basically they have a 'max' damage type which is the same as my 'max' damage type. So both will get processed. My extension will see CA max on the damage line as AWD max on the damage line, but both are different things.
One extension would have to use a different name to resolve the conflict. I did not check for code conflicts but had a quick look at the use of 'max' in CA. So this was a quick glance at the code.
Ok I hope a solution can be found as both are very useful in there own way and had I realised the conflict was there I wouldn't have purchased from DmsG.
Depending on the time you purchased AWD, you could always refund it.Quote:
Originally Posted by Leprekorn
But do not be upset with my extension, CA is at fault for this one.
FG uses 'max' at a specific roll point to increase the values on the dice to the max value for the dice type. AWD uses that same point to do the same thing on a per line bases. They work together without conflict because AWD follows the FG specification for what 'max' is designed to do. ( attached max.jpg, shown in a spell token campaign because i'm only showing the modifier max in FG )
CA goes off and conflates what 'max' does in FG, they have the issue. And should not be using the term 'max' or change the [MAX] specification of FG. Because they are not processing 'max' as changing the dice roll value early to max value, they are using it to damage the max health of a create. ( if I recall the usage. )
CA is in conflict with FG, they probably have to 'fudge' the ruleset code to make what they want to work. It is up to the CA team to sort this conflict out, as they are breaking FG specification. ( Not AWD )
I've linked the creator to your post
So looked at AWD, I use 'maxdice', so maybe CA is just spotting 'maxdice' as 'max' depending on how they are doing the match ?Quote:
Originally Posted by Leprekorn
I'll take a better look with some test cases in a campaign at some point, as just quick looks at code I have not updated for a while and my memory of these things. So not 100% sure on the exact interactions of what is going off with this.
Thanks I appreciate it.
Max, as a modifier, is indeed built in to FG. As a damage type though, it is very much so not. And CA does in fact succeed at accomplishing one of its core competencies; it is not completely failing to function when loaded on its own within the 5e ruleset. Further, I have taken great pains to ensure that not a single line of ruleset code is excluded nor function duplicated when handling the "max" damage type; CA is a strict addition of functionality in this regard. Lastly, CA does not experience any difficulties when used in conjunction with a fairly substantial number of other extensions; the only other conflicts occur occasionally when interacting with two Wild Shape extensions, as a completely separate pool of health and potential replacement of ability scores is involved.Quote:
Originally Posted by bratch9
I have verified that when using CA by itself in a campaign it does not conflate "maxdice" with "max".Quote:
Originally Posted by bratch9
So I've looked into this some more, and its the same issue I had with the extension a long time ago.Quote:
Originally Posted by Leprekorn
Basically they 'store' an early version of the 'decodeDamageText' results in a local variable 'decodeResult' near the start of the damage process. This then 'holds' the pre-AWD values or any modifications for the damage stack that happen between then and the 'apply damage' section. Which they then pull back and use later in the damage process when they display the 'messagedamage' function.
So the issue is the CA extension is 'trying to avoid' doing a bunch of extra decode of damage type stuff and holding onto incorrect values that could change in the damage function call process. Causing the incorrect results to end up at the damage point.
See attached 'values.jpg', in the green box I have shown the results of AWD on its own. You can see the initial dice #13 get changed into the #4 result and been used correctly and passed to the 'messageDamage' function.
In the 'red' box with both extensions, you can see the #13 dice values been passed into the AWD side and reduced to the correct #4 values, which suddenly 'becomes #13' for the 'call messageDamage' because during the 'applyDamage' from the 5e ruleset it has to call the 'decodeDamageText' and this is when CA 'magic replaces' the damage line with the one they 'stole' at the start of the damage roll process. Basically replacing the correct valid adjusted at the correct points in the damage process values created by AWD, with the non-processed 'stashed' out of date values from the start of the damage roll.
I reported this as a problem when they started the extension, it seriously invalidates the damage roll process and all the calling systems for modifications. I do not understand 'why' they store this early value and re-inject it later in the damage. But its a total mess that needs to be replaced, and I said this to them last year ( or even the year before then. )
CA is seriously breaking the damage roll process with using the stored old decoded values, this is why it breaks. Infact it would break any valid extension that changes values in the damage process stack using the damage modification stack function provided by FG.
The CA extension needs re-writing without this 'decodeResult' local stashed value. I'm sure they have a good reason, maybe it holds some values they they need later. But they should not re-inject it back at a later stage. They should process out of the old stored value the items they need, and merge that with the 'current' version the damage stack is using at the time they want to modify.
-pete
Thanks botha for looking at this.
While I can't speak much to the process of testing here given that it all exists behind closed doors, if one takes a look at the CA code it will be seen that indeed CA does hold on to the decode results from the ruleset. However, the assertion the old values are re-injected is simply inaccurate as there is no re-injection, merely inspection. The conclusion that CA breaks the damage modification stack is also, unfortunately, demonstrably false; when using CA alone with the 5e ruleset all CA damage types combine perfectly with all damage modifiers. I'd be happy to discuss in more detail the motivations for the use of a local variable for value inspection, though at a high level this is simply the best way to use the result of the decode calculations later in the ruleset damage application logic without overwriting, or re-inventing, ruleset logic. The alternatives would be highly likely to conflict, as "doing a bunch of extra decode of damage type stuff" drastically reduces compatibility with other extensions that are designed against the ruleset itself.Quote:
Originally Posted by bratch9
Sadly, I get the feeling given the flow of the conversation thus far that finding a solution is of secondary priority to assignment of fault. As such I will be backing out of this thread going forward, and earnestly await a favorable decision to seek a resolution for this conflict.
@bratch9 I'm sure there is a way this could be resolved without so much finger pointing both of these extensions provide excellent functionality how can we move this forward?
Not with the local variable, because they do back force it. Even if they think they do not.Quote:
Originally Posted by Leprekorn
Basically the 'local decodeResult' start off as 'nil', and so on the 'first' call to decodeDamageText, the 'if not decodeResult' will flow into the 'if' and capture the decodeDamageTextOriginal values. From this point on ANY call to 'decodeDamageText' used by the ruleset will return the original first decoded values held in 'decodeResult'. This includes the latter call to 'decodeDamageText' in the 'applyDamage' from the ruleset which is why my extensions modified values get overridden. Instead of correctly decoding the current on the fly values in the code flow, the ruleset if 'forced' to use the old stale value. ( Because decodeResult now has a value, it just skips over the decode and returns the old values when the ruleset code asks to re-decode the current values. )Quote:
function decodeDamageText(nDamage, sDamageDesc)
if not decodeResult then
decodeResult = decodeDamageTextOriginal(nDamage, sDamageDesc);
if string.match(sDamageDesc, "%[HEAL") and string.match(sDamageDesc, "%[MAX%]") then
decodeResult.sTypeOutput = "Maximum hit points";
end
end
return decodeResult;
end
Only when the 'applyDamage' ruleset function later calls the 'messageDamage' call is the decodeResult set back to nil and releases the held values. ( And waits for the next 'decodeDamageText', to store the values. )
Its clear from the image I provided that the CA 'stored' value is getting back into the results, and is why the correct values AWD generates get 'lost'. If this is not supposed to be the case, then its still a bug in CA.
Ive shown than my extension changes the values during the process, which it is allowed to do. I've shown that multiple calls to 'decodeDamageText' happens because CA captures it at the start and holds on to his until the message happens. Which happens in the 'applyDamage' after further 'decodeDamageText' call so the ruleset expects this structure to 'change' over time in the damage processing. ( Otherwise the ruleset would 'hold' the value as well, instead of multiple decodeDamageText calls. )
My extension does not hold onto the values, it can not 'magic' back the wrong values into the results. What you are seeing when the values do not return as expected is the net result of the 'stored' values in 'decodeResult' getting re-applied when the ruleset 'applyDamage' function later calls 'decodeDamageText' to process the 'Current' state, but in fact CA returns the old stored pre-decoded values. Which is how the wrong values gets re-injected back into the 'applyDamage' and the incorrect values end up as a result.
CA needs to remove the 'local decodeResult' and assume the structure been passed around into functions designed to change them, actually changes them, it can not and should not 'store' the values.
Clearly from the 'response' above, CA team does not 'expect' the values to get re-injected... they seem shocked that I'm telling that is the reason. So maybe they need to look into this 'bug'.
Its very clear in the 'red box' in the image I provided that when the CA extension is active the stored #13 decode results gets re-used in the later 'applyDamage' ruleset function. ( And you see it in game with the incorrect values been applied. ) As I've output the values at different stages of the damage function flow.
Finding a solution is why we are talking about this, the solution is to stop the bug that is re-injecting old values caused by the CA code.
I'm very willing to take on fault, when its in my extension, the fact the fault is in CA and they are not accepting the issue.
CA say 'assertion the old values are re-injected is simply inaccurate as there is no re-injection', but this is clearly shown to be happening in the images, and I've very clearly stated which parts of the CA code is causing this and how it happens.
I dont know how I'm getting painted as the 'bad guy', I'm going out of my way to debug an extension I do not own, I'm providing the feedback and reasons and how this issue is been caused.
This is not my bug, its an issue in CA.
To be clear I'm not painting anyone as the bad guy. Inferring tone is very difficult online.
I was not referring to you. ( But to CA,Quote:
Originally Posted by Leprekorn
, which imply they think its a bug in my code and its more important for me to blame them for the bug... But it is their bug, and yep I'm pointing the issue at them as their bug. )Quote:
Sadly, I get the feeling given the flow of the conversation thus far that finding a solution is of secondary priority to assignment of fault.
If you take a look of this image with just CA loaded, you can see 2x calls to 'decodeDamageText'.
Attachment 49158
This is a normal weapon roll, you get 2 calls to decodeDamageText. In this case 'nothing changed' between the early and later calls, and all the results match up.
-------------------------------------------------------------------------------
With CA and AWD, and an hew hit, the 2x decodeDamageText is clear.
Attachment 49159
I've marked on the issues on the second call. In the early call that CA 'store' the #13 damage, the later call with the modified results of AWD shows that the function is called with a request for '#4' damage, and the dynamic decodeDamageText call shows the original ruleset decodeDamageText decoded results 'nVal=#4', because that is the value that is requested in the 'applyDamage' 5e function that calls the second decodeDamageText, with the values AWD passes in.
I've marked with the 'red box' the point CA returns the 'old stale' values from the first call, and this causes the 're-inject' of the old values back into 'applyDamage' and it then applies '#13' damage instead of the '#4' that is called for in the function.
I've attached the changes to the CA 'manager_action_damage_ca.lua' ( renamed txt to make it upload ), to show the shown outputs.
-------------------------------------------------------------------------------
I hope this makes it clear that CA is holding and re-injecting 'old out of date' decodeDamageText, and it is totally the fault of this that is causing the incorrect damage result you are seeing.
The ruleset calls the decodeDamageText twice, once in the early stages of the damage functions, then the 'modification' can be applied, then it calls it again in the 'applyDamage' function to resolve the requested damage. It is totally within the ruleset for AWD to 'change' the initial early values, with the more specific per-NPC before the call to 'applyDamage'.
The fact CA are 'holding' and 're-injecting' the old early values is not compliant with the ruleset. They should not hold this value, and the ARE re-injecting it into later results as shown. ( Which they claim they are not. )
Except it doesn't. The text "decodeDamageText" exists exactly twice in the entirety of the ruleset code: once to declare the function, and one call made in applyDamage.Quote:
Originally Posted by bratch9
Per above, it totally isn't. While I wouldn't call the flow necessarily unreasonable, it is notably outside the assumed flow of the ruleset, and therefore would be a high risk of conflict with other extensions.Quote:
Originally Posted by bratch9
Yes, CA itself does make an additional call to decodeDamageText prior to applyDamage to pre-calculate healing "damage" that should increase the target's maximum hit points. And due to the fact the the ruleset only makes this call once, CA ensures that the precalculated value is used during the ruleset call as well. While I grant that "pre-calculation" and "re-injection" may be considered semantic differences from one point of view, a vast amount of software development is intrinsically defined by semantic distinction.Quote:
Originally Posted by bratch9
We did indeed have a conversation about compatibility in January. At which time I explained the rationale and invited feedback for alternatives. The last I heard, the conflict had been resolved on the AWDE side of things...Quote:
Originally Posted by bratch9
And then out of the blue this is stated quite publicly and pre-emptive of any real investigation.Quote:
Originally Posted by bratch9
This is sorta the crux of the issue, and there are two key points here.Quote:
Originally Posted by bratch9
I'll start with the less important one: when two extensions work when on their own and conflict when together, assignment of fault could not be so trivial. Neither extension has bugs of its own. Both extensions experience a bug together. It is a shared thing. Either extension would be capable of resolving the issue.
The really important part though is that fault simply does not matter. The overwhelming majority of extension developers in the community recognize this. We work together to make the best experience we can for the users. And in the event that we deem the effort of compatibility too great to justify, we don't waste anyone's time assigning blame or putting each other down. We simply own the fact that sometimes extensions are incompatible in their goals.
Personally, I do not think that CA and AWDE have hit the point of concluding incompatibility. However, given that CA works on its own and with roughly 100 other extensions of varying capabilities including additions to the modifier stack and dammage application, if I am to be able to address the present conflict in CA I require either more constructive guidance than being advised to rewrite it, or a better understanding of exactly how AWDE deviates from the ruleset.
Ok fixed your extension for you... And yes I changed your decodeDamageText which was causing the issue and injecting incorrect values. Now it does not.
I've not checked any of your 'CA function' changes, so maybe the CA weapon features have some issues. But I dont think this will be the case.
The only 'issue' I can think that conflicts with AWD is the 'transfer' option when you swap the source/target. As AWD will do things like effect 'size/type' checks which will not be against the source and not against the target. So with 'transfer' it will 'turn' the AWD specific damage selections onto the source. So if your character was a 'plant' and the weapon had '1d6 slashing,type(plant)' as part of the AWD then that 1d6 extra would be applied to 'you' as the 'you'/source is now the target. This is 'probably' what you want with the 'transfer' option anyway. ( basic flip the weapon on its owner ? )
Because of the way 'transfer' is checked, if it was on an AWD line with a check ie '1d6 slashing, type(plant), transfer'. You extension would always do the swap for the 'transfer' but in an interaction with AWD it probably should only apply the 'transfer' if the target was 'type(plant)'. But this would be complex to sort out and probably such a low chance not worth the worry. But you might want to note that 'transfer' use when interacting with AWD options, should not have 'transfer' damage type on any 'special' AWD lines that are basically 'optional' applied by AWD. It depends on how you want to think about the process of what it is trying to do.
-pete
AWD interacts with the applyDamage function that you also change, which breaks the 'assumption' in your extension that you can hold the decodedDamageText as static. Because it is not.Quote:
Originally Posted by MeAndUnique
Since applyDamage is calling the decodeDamageText the inputs to applyDamage can be 'adjusted' in extension. Like you do in your extension. I just dont assume that it is 'static' or that no other extension could possibly be in the calling stack of overrides for applyDamage call chain.
The issue is that your extension 'assumed' that once you had processed 'applyDamage' and then called down into the 'chain' of possible other extensions that could overload this function, that they would not also change something in the call process. ( Which since you wrap this function to apply your own changes, you can not assume other will not also apply changes before it hits the actual ruleset function. ) Your extension is holding the value from your extension level call, and then passing it back into the 5e ruleset version and assumes nothing will ever happen to it or be between you and the ruleset function.
AWD also scans the damage lines before calling the ruleset applyDamage, and can change the lines. Which makes this a dynamic item. Your extension should not be assuming it is static which it will be if no other extension is in the stack that access/changes the applyDamage function.
I dont think the suggestion change I've posted will cause you any issue, because AWD does not change the decoded outputType which is what you are holding from the stored value. You could reduce what you store in the local value to just that one item. Then not have caused this conflict.
The reason I can not fix it in my extension, is because yours sits on top of mine and is pushing back in the stored values. Without changing my load priority I can not resolve it. As far as I know my extension does not break your extra codes as they pass through my extension. But in theory we could have some extra interaction like this. ( Like the 'transfer' damage code I already talked about. )
I also do not think you should be doing 'extra' work in the 'messageDamage', as people will not be expecting this. I know you do it so you do not have to adjust the applyDamage function in a mid code point, which causes a possible massive conflict point. So I understand why you do this. But I suggest you look to see if you can do this work before the applyDamage or post the applyDamage function. But this would be a more of the re-write I think you should do.
I do think you should just store the 'decodeResult.sType' which is all you use outside the applyDamage function, then you could store this at the point you 'decodeDamageText' in your applyDamage function, and then totally remove your access to the 'decodeDamageText'. Since you link into messageDamage you can use this to change your sTypeOutput to "maximum hit points'. I've not looked into the 'effectmanage/actiondamage' calls in applyDamage so it might not be possible to remove the decodeDamageText 'sTypeOutput' change.
I'm not sure if this has been addressed, but when I use this extension, ongoing damage (for example, "DMGO: 1d4 fire") always becomes untyped damage. I've tested this with only this extension on and it still happens. Has there been a fix for this, or has anyone else had this problem?
I can replicate this, and will take a look. It should not be causing an issue like this.Quote:
Originally Posted by BlazingAzureCrow
EDIT: I've got a fix for this issue working, but want to look at what I can sort with the CA extension above before doing a new release.
v2.2 is now uploaded, should fix both CA and DMGO issue.
Great to hear thanks
The issue is fixed! Glad to see that resolved. I noticed it happening months ago but didn't track down which extension it was until this week. Glad it could be fixed so quickly!
Whoops, posted in wrong thread. My bad!
Not sure on this, Advance Weapon Damage, does not interact with character max HP.... so it changing these on a couple of characters is probably not the extension. ( Might be some odd interaction between extensions. )Quote:
Originally Posted by RedmondStache
Did you have these characters at 5th level before adding AWD ? ( Or create them with AWD )
Did you already have CA enabled before AWD ?
If CA is thinking these are 'adjusted', I'm not familiar with CA to know what mechanism it used to detect this. ( I think some extension I read about, allow for per level dice roll entry and manual entry and 'standard' value, which might be CA.. I seem to recall you could open some part of the character sheet to see what it thought and correct these... ie go back and say for level 2 'I rolled 5' for my HP increase.. )
When I get chance I'll take a look, but character history and order of when extensions were added etc... would be good information to know about.
Any extra information you can provide would be good to help re-produce this.
Thanks, Pete
[ I assume you posted on CA thread as well ? ]
Followed a link from the CA thread to see about an incompatibility, wasn't aware I was being taken to a different thread. My bad!
Not an issue, easy done.Quote:
Originally Posted by RedmondStache
( The link was probably when CA and AWD was miss calculating. Which should be fixed. So was worried that it might have come back . )
-pete