wai Scrapper Bone Smasher Large PvE crit marked PvP only

[aethereal]

https://cod.uberguy.net./html/power.html?power=scrapper_melee.energy_melee.bone_smasher&at=scrapper

 

In CoD, the crit line for "lieutenants and above," the one with the base chance of happening at 10% crit rate, is marked as PvP only.  Not sure if that flag is actually respected for a child effect, but thought it was worth mentioning.

 

Pretty sure it's not just a CoD problem: no other large crit line on a half dozen other powers I checked was flagged like that 

[Arc-Mage]

Great catch.

[Booper]

10 hours ago, aethereal said:

https://cod.uberguy.net./html/power.html?power=scrapper_melee.energy_melee.bone_smasher&at=scrapper

 

In CoD, the crit line for "lieutenants and above," the one with the base chance of happening at 10% crit rate, is marked as PvP only.  Not sure if that flag is actually respected for a child effect, but thought it was worth mentioning.

 

Pretty sure it's not just a CoD problem: no other large crit line on a half dozen other powers I checked was flagged like that 

Have you tested the power in game to confirm you never get a critical hit against critters when using Scrapper's Bone Smasher?

 

There is a good chance it's a display logic bug in CoD. Without knowing what powers you compared against, I can't really say if what you looked at was an apples-to-apples comparison. But one thing I noticed was the 10% crit effect in Bone Smasher does not show a requirement for "not player", while all the other crit effects I did look up did have a "not player" check. This makes sense to me because on Live there was no such thing as Effect Groups, so a crit effect had to include all requirements checks. However, Bone Smasher uses effect groups and it is nested. The first effect checks to make sure the target is a critter, then the crit effect within it checks for a set of critters to not include (the ones that are included in the 5% chance). There is no need to check if the target is a player because the first effect already ruled that out. I believe CoD is using string matching to determine some of the PvP or PvE flags, and in this case might be getting it wrong because it does not show "not player" in the effect like most others do.

 

So I would recommend testing it in game just to see if it's a CoD problem or a Live problem.

[aethereal]

5 hours ago, Booper said:

Have you tested the power in game to confirm you never get a critical hit against critters when using Scrapper's Bone Smasher?

 

I have not.

 

5 hours ago, Booper said:

 

There is a good chance it's a display logic bug in CoD.

 

This isn't impossible, but it's not likely, I think.

 

What seems more plausible is that the specific flag that CoD checks for when it shows an effect group as "PvPOnly" is non-operative for child effects (ie, the parent effect group's flag supercedes the child flash), in which case it's not a CoD bug, but it's harmless (unless say someone ever copies that data out of the child effect and into a top level effect).  But even if that is the case, it should probably still be fixed.

[aethereal]

I checked and confirmed that you CAN crit against Lieutenants and above in PVE with Bone Smasher -- the flag is ignored.  Should still be fixed!

 

I don't really know anything about the native format of powers data, but in the JSON that CoD uses as an intermediate format names the key for this particular data item as is_pvp and the possible values are PVP_ONLY, PVE_ONLY, and EITHER.  This effect (the one with tag CritLarge) is value PVP_ONLY, and regardless of the fact that the code is currently ignoring the flag, it's clearly wrong and should be corrected!

 

Edited February 26, 2023 by aethereal

[Bopper]

1 hour ago, aethereal said:

I checked and confirmed that you CAN crit against Lieutenants and above in PVE with Bone Smasher -- the flag is ignored.  Should still be fixed!

 

I don't really know anything about the native format of powers data, but in the JSON that CoD uses as an intermediate format names the key for this particular data item as is_pvp and the possible values are PVP_ONLY, PVE_ONLY, and EITHER.  This effect (the one with tag CritLarge) is value PVP_ONLY, and regardless of the fact that the code is currently ignoring the flag, it's clearly wrong and should be corrected!

 

That is a CoD issue, not a Homecoming issue. What you see on CoD is not always "raw", but rather interpreted so it is easier for a user to read. In this case, CoD is not recognizing a requirements check correctly and is selecting to display PvP only when it's actually PvE only.

 

TLDR; it's not broke and there is nothing to fix on Homecoming. It's a CoD bug.

[aethereal]

5 hours ago, Bopper said:

That is a CoD issue, not a Homecoming issue. What you see on CoD is not always "raw", but rather interpreted so it is easier for a user to read. In this case, CoD is not recognizing a requirements check correctly and is selecting to display PvP only when it's actually PvE only.

 

TLDR; it's not broke and there is nothing to fix on Homecoming. It's a CoD bug.

 

I mean, I'm not an expert in either RubyRed's parsing code or the underlying CoH codebase, and it's possible that I'm off base here.  Perhaps @UberGuy can weigh in.

 

But I spent ten minutes looking over the parsing code, and it looks to me like there are two underlying straightforward binary flags, one for "pvp only," one for "pve only," and all the parsing code does is collapse them into a synthetic ternary field (PvPOnly if one flag is set and the other isn't, PvEOnly of the other flag is set and the one isn't, and neither if no flag is set).  I didn't see anything that suggests text parsing or the creation of a massively synthetic field here.

 

Again, maybe I'm wrong: there's a limited amount of trying to work my way through an unfamiliar codebase that I'm going to do here.  But I'll note that the corresponding "small" crit line (the one for minions and below) doesn't show the same data error, and it seems hard to explain that if you imagine that this is entirely a synthetic field issue in the intermediate json data format.

[UberGuy]

This is a CoD data processing bug.

 

Ruby's original parser code handles only some what CoD does. There is a lot of post-processing done on certain aspects of the resulting data. The PvP/PvE-ness of an effect is one of those things, and it is one of the more complex post-processing bits of logic I added.

 

Some power effects are explicitly tagged when they are PvP-specific effects, and when that tagging is present, CoD honors that and does not do additional inspection. That process is simple and I wish it was all that was ever required.

 

However, the vast majority of effects do not have this tag, and the "PvP/PvE" nature of an effect has to be inferred from expressions attached to the effect. When we can see that an effect says "if target is a critter and target is not a friend", this taken to mean that the effect is expected to happen in PvE. Conversely, having "if target is a player and target is not a friend" expression is taken to mean that the effect is PvP specific. (Note that the expression may not need to explicitly ask "is target (not) a friend" as the power has targeting info that (usually but not always) makes this obvious.)

 

This conditional interpretation of powers' PvE/PvP nature is usually robust, but can and does fail because CoD does not have all the context that the game's combat engine does. The "if player / if critter" expression logic can be part of a much more complex expression, and other conditions in such an expression are simply collapsed to true or false based on some usually reasonable but potentially false assumptions, Also the interaction of "if player / if critter" with targeting constraints occasionally becomes very complicated.

 

Most of this is simply to explain why the processing is complicated to start with. What's happening here, though, is a fairly simple breakdown in that logic.

 

The expression here checks for whether an effect applies to any of a list of critter "AT"s. The test is "if not (critter or critter or critter or critter)", which CoD's parser turns into "if not (PvE or PvE or PvE or PvE)". So the parser is evaluating that as "not (PvE)" which means "PvP".

 

This is clearly wrong, but because the expression is so explicit, given the data and assumptions the parser has to work with, it's an understandable outcome. It would be more accurate for the parser to realize that saying an effect only applies to "not some critters" still means it could apply to other critters, but given the way the parser is structured, looking at each part of the expression in a vacuum, adding that will be (very) challenging.

 

This doesn't happen to the "large crit" expressions in Scrapper powers more often because, looing at most powers, they have this expression instead.

 

if !((target>arch eq 'Class_Minion_Grunt') || (target>arch eq 'Class_Minion_Pets') 
  || (target>arch eq 'Class_Minion_Small') || (target>arch eq 'Class_Minion_Swarm') 
  || (target>enttype eq 'player'))

 

That translates to "if not (target is critter or target is player)", which collapses to "if not (PvE or PvP)" which just flips the two states. It becomes "PvP or PvE", which CoD (correctly) displays as "Any".

 

The parsing  described here is done on the fly for each attribmod in a vacuum, but in the Bone Smasher example, there's a pretty unambiguous condition expression on the containing effect group that says "if target is a critter". CoD doesn't handle this nesting of conditions in its PvP/PvE logic. If it did so, this bug would be fixed. That's probably how I will try and address this particular issue.

 

Edited February 26, 2023 by UberGuy

[UberGuy]

Odds are good this isn't going to be fixed for a some time.

 

CoD takes powers data in in a kind of streaming way. As each discrete chunk of data comes in, it's passed to logic that parses that particular kind of thing, with zero context. All logic that builds links between things that should know about one another (like a power knowing what AT's can use it) is done by later, whole-dataset passes that analyze everything and stitch the relationships together.

 

To give you an example of what I mean, consider a power that is invoked only by redirection. It will often not be in either a power category or powerset associated with an AT (or critter). The only way to know what ATs will use it is to analyze every power (and every AT), find direct links between powers/powersets and ATs, and then detect indirect links between powers. If Power A is a power Blasters can use by direct AT/powerset relationship, and power A redirects to power B, then power B is also a power Blasters can use, even if there's no direct relationship.

We can't do that kind of relationship analysis until everything has been loaded and a first pass on the non-relational parts of it done. So I can't know about the relationship between Blasters and Power B until I've loaded all the data for ATs, powers and powersets.*

 

The PvP and PvE parsing of expressions is currently done in each attribmod as it comes in. The code that does this knows nothing about the enclosing effect group or the power which encloses that. All the code knows about is the expression it received, and a bunch of zero-context rules about whether a "piece" of an expression indicates PvP, PvE, or both/either. 

 

Fixing this bug requires better parsing of "not some critters" in a vacuum, or a second pass on all expressions to add context to PvP/PvE evaluation. Either one is non-trivial - adding "maybe PvP" to the context free parser would require a dramatic rewrite of it. Code for a "second pass" analysis of all the expressions doesn't exist and would need to be written. (And to be clear, a context-aware expression pass could get this wrong unless I improve the expression handler - imagine if this expression was on the effect group instead of something inside it.)

 

Given how much of the time the current logic is "good enough" and I am working on other features, I don't have a huge motive to put in either version of that work to fix one effect in a single power. I'll noodle over alternatives that would take less effort, though. Someday I would like to improve this whole process and will likely rip and replace large parts of it, but probably not now.

 

 

* This power-to-AT code in the original Rust code was buggy by the way, as it did not consider multi-step redirects between powers, which do exist in some player powers. Since the processing order of related powers is effectively random**, building a relationship mapping between powers would sometimes miss links because a power in a relationship chain was not yet seen. For example, if Power A calls Power B calls Power C, and you process C before you see B or A, then you will assume there's no relationship between C and A. I noticed this problem because I put the site's JSON data in source control, which let me see that the AT relationships for some powers were changing between runs of the extract code when nothing else was changing. Fixing this required added logic to perform the relationship analysis in multiple passes.

 

** Powers are stored in a hashmap as they are loaded, and most hashmap implementations make no guarantee about ordering.  Even if you sort them, you don't know that you're sorting in an order that means you will process them in order of their relationships.

Edited February 27, 2023 by UberGuy

[UberGuy]

So I think I fixed this, but my analysis is kind of shallow at this point. After some reflection and digging into what it was really doing to have them in the PvE consideration, I simply removed all of the "partial critter only" criteria. So far in spot checking that seems to do the right thing. The example above for Scrapper Bone Smasher is now showing the correct "PvE" flagging in CoD, as are a couple of other problematic things like the two effects granted by Tanker Gauntlet.

 

If you see any more wonky things like this, feel free to post about it over in the main CoD thread in the tools subforum.