feat: add Helminth abilities #643
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request enhance the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
SlayerOrnstein
force-pushed
the
helminth_abilities
branch
from
398c8e0 to
e1575a2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
build/scraper.mjs (1)
96-104: LGTM with a suggestion for improvementThe addition of the 'Helminth' object to the 'Warframes' category aligns well with the PR objectives. The structure of the object matches the description in the PR summary, which is good for consistency.
However, I suggest adding a null check for
raw.ExportAbilitiesto improve error handling:Consider adding a null check for
raw.ExportAbilities:if (category === 'Warframes') { const helminth = { uniqueName: '/Lotus/Powersuits/PowersuitAbilities/Helminth', name: 'Helminth', - abilities: raw.ExportAbilities, + abilities: raw.ExportAbilities || [], }; data.push(helminth, ...raw.ExportWarframes); }This change ensures that
abilitieswill always be an array, even ifraw.ExportAbilitiesis undefined.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- build/scraper.mjs (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
build/scraper.mjs (2)
Line range hint
105-111: LGTM: Improved data structure for 'Upgrades' categoryThe addition of the
typeproperty to themodSetsitems enhances the data structure and aligns with the PR objectives. This change provides more detailed information about the upgrades without affecting the existing logic for other categories.The modification is clean and straightforward, improving the overall data representation for the 'Upgrades' category.
Line range hint
96-111: Overall assessment: Focused enhancements to data structureThe changes in this file are well-focused and align closely with the PR objectives. They enhance the data structure for both the 'Warframes' and 'Upgrades' categories without introducing significant risks to the existing functionality.
Key points:
- Addition of 'Helminth' data to the 'Warframes' category.
- Improved structure for 'Upgrades' category with the new
typeproperty.These modifications provide more detailed and structured data, which should improve the overall functionality of the scraper.
The changes are minimal, focused, and improve the data representation without altering the core logic of the
Scraperclass.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
test/index.spec.mjs (2)
26-26: LGTM. Consider adding Helminth-specific tests.The addition of 'Helminth' to the
namedExclusionsarray is appropriate, as it ensures that Helminth is not treated as a standard warframe in the component reflection test. This aligns with the PR objectives of introducing new Helminth abilities.Consider adding specific test cases for Helminth abilities to ensure they are correctly represented in the data structure. This could include verifying the presence of the new
imageNameattribute for Helminth abilities and checking the overall structure of the Helminth data.
Line range hint
1-283: Enhance test coverage for new Helminth abilitiesWhile the existing test suite is comprehensive, it doesn't explicitly cover the new Helminth abilities introduced in this PR. Consider adding the following test cases:
- Verify the presence of Helminth in the 'Warframes' category.
- Check the structure of Helminth abilities, including the new
imageNameattribute.- Ensure that Helminth is correctly marked as non-masterable.
- Validate the integrity of Helminth-related data, such as item drops and tradability.
Here's a suggested structure for these new tests:
describe('Helminth', () => { let helminth; before(async () => { const items = await wrapConstr({ category: ['Warframes'] }); helminth = items.find(item => item.name === 'Helminth'); }); it('should exist in the Warframes category', () => { assert(helminth, 'Helminth should be present in the Warframes category'); }); it('should have correctly structured abilities', () => { assert(Array.isArray(helminth.abilities), 'Helminth should have an abilities array'); helminth.abilities.forEach(ability => { assert(ability.abilityName, 'Each ability should have a name'); assert(ability.description, 'Each ability should have a description'); assert(ability.imageName, 'Each ability should have an imageName'); }); }); it('should be marked as non-masterable', () => { assert.strictEqual(helminth.masterable, false, 'Helminth should not be masterable'); }); it('should have correct item drops', () => { assert(helminth.drops, 'Helminth should have drops'); // Add more specific checks for drops based on expected data }); it('should be non-tradable', () => { assert.strictEqual(helminth.tradable, false, 'Helminth should not be tradable'); }); });These tests will help ensure that the new Helminth data is correctly structured and integrated into the existing system.
There was a problem hiding this comment.
i'm gonna have to go w/ @coderabbitai on this one, please add helminth-specific tests, since this seems to be unique to helminth
SlayerOrnstein
force-pushed
the
helminth_abilities
branch
from
0098616 to
5fae883
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
test/index.spec.mjs (3)
6-6: Reorder imports for consistency.Consider moving the
chaiimport before the../build/dedupe.mjsimport to maintain a consistent import order. This aligns with the static analysis suggestion and improves code readability.Apply this diff to reorder the imports:
import assert from 'node:assert'; import { resolve } from 'node:path'; import { createRequire } from 'module'; +import { expect } from 'chai'; import dedupe from '../build/dedupe.mjs'; -import { expect } from 'chai';🧰 Tools
🪛 GitHub Check: Lint
[failure] 6-6:
chaiimport should occur before import of../build/dedupe.mjs
282-287: LGTM: New test case for Helminth properties.The new test case correctly verifies Helminth's unique structure. However, consider enhancing the assertions for improved clarity and error messages.
Consider updating the assertions as follows:
- assert(typeof helminth.component, undefined); - assert(typeof helminth.drops, Array); + assert.strictEqual(helminth.component, undefined, 'Helminth should not have a component property'); + assert(Array.isArray(helminth.drops), 'Helminth drops should be an array');This change provides more descriptive error messages and uses more specific assertion methods.
288-294: LGTM: New test case for Helminth abilities count.The test case effectively verifies the expected range of Helminth abilities. The comment explaining the upper limit is helpful for future maintenance.
Consider a minor adjustment for consistency:
- it('helminth should more the 4 abilites', async () => { + it('helminth should have more than 4 abilities', async () => {This change corrects the grammatical error in the test description and improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- test/index.spec.mjs (3 hunks)
🧰 Additional context used
🪛 GitHub Check: Lint
test/index.spec.mjs
[failure] 6-6:
chaiimport should occur before import of../build/dedupe.mjs
🔇 Additional comments (1)
test/index.spec.mjs (1)
27-27: LGTM: Helminth added to namedExclusions.The addition of 'Helminth' to the
namedExclusionsarray is consistent with the PR objectives and accommodates the unique nature of Helminth in the game.
SlayerOrnstein
force-pushed
the
helminth_abilities
branch
from
6e57453 to
7d7fcd2
Compare
Didn't want to break any exisiting projects by making the nullable so opted to just default to 0
SlayerOrnstein
force-pushed
the
helminth_abilities
branch
from
7d7fcd2 to
acc3d24
Compare
|
🎉 This PR is included in version 1.1265.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What did you fix?
Add Helminth abilities into scraper process. I took a partial from one of the ability
uniqueNamesto form the baseBefore:
{ "ExportWarframes": [...], "ExportAbilities": [ { "abilityUniqueName": "/Lotus/Powersuits/PowersuitAbilities/HelminthEfficiencyAbility", "abilityName": "Energized Munitions", "description": "Increase the efficiency of your ammo consumption." }, { "abilityUniqueName": "/Lotus/Powersuits/PowersuitAbilities/HelminthCompanionAbility", "abilityName": "Master's Summons", "description": "Heal your companion and call it to your side." }, { "abilityUniqueName": "/Lotus/Powersuits/PowersuitAbilities/HelminthEnergyShareAbility", "abilityName": "Voracious Metastasis", "description": "Consume Energy to heal yourself and grant matching Energy to each ally." }, ... ] }After:
{ "abilities": [ { "uniqueName": "/Lotus/Powersuits/PowersuitAbilities/HelminthEfficiencyAbility", "name": "Energized Munitions", "description": "Increase the efficiency of your ammo consumption.", "imageName": "energized-munitions-d5df009f96.png" }, { "uniqueName": "/Lotus/Powersuits/PowersuitAbilities/HelminthCompanionAbility", "name": "Master's Summons", "description": "Heal your companion and call it to your side.", "imageName": "master's-summons-b2337980b6.png" }, ... ], "category": "Warframes", "drops": [ { "chance": 1, "location": "Entrati (Son), Associate", "rarity": "Common", "type": "Helminth Segment Blueprint" }, { "chance": 1, "location": "Entrati (Son), Family", "rarity": "Common", "type": "Helminth Invigoration Segment Blueprint" } ], "isPrime": false, "masterable": true, "name": "Helminth", "tradable": false, "type": "Warframe", "uniqueName": "/Lotus/Powersuits/PowersuitAbilities/Helminth" }Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores