Closed mgrzegor closed 6 months ago
Thanks so much for the contribution @mgrzegor and apologies for the delay in getting to it! It looks almost perfect, I just have a few little tweaks to request:
values:[...]
for the agzresist
attribute if we don't have to, since that requires special case handling in the code for translation etc. If the effect is guaranteed full protection (within the radius and duration) then let's make it unit:'%', min:0, max:100, default:0, scale:0
and give the weapons agzresist:100
, or if it's a fuzzier kind of "it helps a lot" effect, then omit unit
, make max:1
, and give the weapons agzresist:1
.88293
to 88299
, and remove its namekey
attribute (it defaults to the module id).88393
to 88399
, and change its namekey
to 88299
.edsy.js
, revert the VERSIONS = [...]
change; since nothing else changed in that file, it doesn't strictly require eddb.js
to be version 3.8.17.7.lang-en.json
, change module-88293
and module-88293-abbr
to 88299
\u2060
changes; I agree that wrapping unit labels is a problem, but I think it'd be better to fix that at the CSS presentation layer (which I'll do along with this update).If you're able to make these adjustments then I can just merge and release. It's no problem if you can't get to it, I can also take care of it after merging, I think it just makes the commit history a little tidier if it's done in your branch first.
Thanks again!
OK — I’ll make the changes and update the PR.
I have to add I am not quite happy with the agzresist
attribute being displayed as either “100%” or “1”. In-game, this stat is displayed simply as “active” (and means simply that the module is fully resistant to the anti-Guardian effect). I was attempting to reproduce that in EDSY, and yes, doing it properly would require a special case in the translation code.
If FDev introduces more stats of this kind perhaps it will make sense to implement them as a boolean (“yes”/“no”) unit, but that is not something we can predict 😉
Hm, okay. I guess I'd been assuming that the resistance was something mentioned in the module flavor text or some other special-case kind of context, but having found a screenshot of how the module attributes are listed in-game, I see what you mean that it's literally shown as "ANTI GUARDIAN ZONE RESISTANCE" ... "ACTIVE" as an attribute right below "AMMO CLIP SIZE" and "AMMO MAXIMUM."
So okay, I agree that it's good to match in-game displays whenever possible. Let's restore that change, and just append , // hextp
to the end of the line to allow future attribute commits to only have to include their own added line(s), and to notate which module group makes use of the attribute.
Then I think it's all set! Thanks for your thoughtful discussion of these finer details. :)
Done :-)
Also I changed the spelling of the multiplier unit (used by the dmgmul
attribute) from "x" (a.k.a. “poor man’s multiplication sign”) to the actual multiplication sign "×"; I think it looks better.
Thanks for your work on this!
Hi taleden, this pull request is to add support of the Guardian Nanite Torpedo Pylons, added to the game a couple of days ago.
I have copied the stats from the module info in the right-hand panel, which includes the zero values for damage and distributor draw, then verified that the values in outfitting are the same (no precision difference). I have also verified that loadouts containing these modules import correctly, both from a journal extract and from the EDMC dump. The prices are taken from Inara and match my own Loadout journal events (taking discounts into account).
The new stat "Anti Guardian Zone Resistance" will need a proper translation of its value "Active", but right now I am too tired to figure this out ;)
Additionally, commit c0f490e fixes a tabs-vs-spaces indentation issue (very visible with tabs set to 8 spaces) and commit e350ea6 fixes an annoying problem with units containing a slash (like m/s) being line-wrapped by Firefox. (I used explicit Unicode escapes because U+2060 a.k.a. WORD JOINER is normally invisible.)
Best regards MG