fix: profit taker rewards weren't under the correct level #89
Conversation
WalkthroughThe changes add a new variable Changes
Sequence Diagram(s)sequenceDiagram
participant P as Parser
participant H as Header Checker
participant B as BountyReward Object
P->>H: Pass header text
alt Header contains "Completion"
H-->>P: Return text as completion value
else Header does not contain "Completion"
H-->>P: Ignore and use default stageText
end
P->>B: Assign stage = completion ?? stageText
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
lib/bountyRewards.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-prettier". (The package "eslint-plugin-prettier" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-prettier" was referenced from the config file in ".eslintrc.json » @wfcd/eslint-config". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (
|
![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
🧹 Nitpick comments (1)
lib/bountyRewards.js (1)
11-11: New variable addition without reset mechanism.A new variable
completionis introduced to track headers containing "Completion" text. However, unlike therotationvariable that gets reset when encountering new table headers (line 25), there's no mechanism to resetcompletion. This could lead to incorrect stage tagging if multiple reward sections are processed sequentially.Consider adding a reset for the
completionvariable similar to how rotation is handled:if (elem.name === 'th') { const tmp = parseRotation(text); // Reset rotation when encountering a new table header. rotation = undefined; + // Reset completion when encountering a new table header. + completion = undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/bountyRewards.js(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
lib/bountyRewards.js (12)
lib/blueprintLocations.js (1) (1)
text(12:12)lib/dropByAvatar.js (1) (1)
text(13:13)lib/enemyModTables.js (1) (1)
text(12:12)lib/miscItems.js (1) (1)
text(14:14)lib/keyRewards.js (1) (1)
text(15:15)lib/enemyBlueprintTables.js (1) (1)
text(12:12)lib/missionRewards.js (1) (1)
text(15:15)lib/sortieRewards.js (1) (1)
text(12:12)lib/modLocations.js (1) (1)
text(12:12)lib/relics.js (1) (1)
text(12:12)lib/transientRewards.js (1) (1)
text(13:13)lib/syndicates.js (1) (1)
text(81:81)
🔇 Additional comments (3)
lib/bountyRewards.js (3)
29-31: Logical handling of "Completion" headers.The added condition correctly identifies and stores header texts containing "Completion" (such as "First Completion" or "Subsequent Completions") in the
completionvariable. This effectively addresses the issue mentioned in the PR description where different rewards for first and subsequent completions needed to be properly categorized.
63-63: Improved stage assignment logic.The use of the nullish coalescing operator (
??) is a good choice here. It prioritizes using thecompletiontext when available (for headers like "First Completion"), falling back tostageTextotherwise. This change ensures the rewards are correctly categorized under the appropriate completion stages in the generated JSON structure.
1-73:Details
❓ Verification inconclusive
Verify the fix with test data.
The changes look good for addressing the specific issue with Profit Taker rewards. To ensure the fix works correctly and doesn't affect other data, it would be beneficial to test it with both the Profit Taker data and other bounty tables.
🏁 Script executed:
#!/bin/bash # Verify behavior with the Profit Taker data # Look for the relevant JSON output to confirm proper structure # Find files potentially importing bountyRewards.js fd -t f -e js | xargs grep -l "bountyRewards" | grep -v "lib/bountyRewards.js" # Check for any test files that might verify this functionality fd -t f -e test.js -e spec.jsLength of output: 124
Profit Taker Rewards Fix – Verify with Broader Test Cases
The fix addresses the Profit Taker rewards issue as expected. The integration in files like
generateData.jsindicates that the changes are being used correctly. However, to ensure that other bounty table data isn’t inadvertently impacted, please run a full suite of tests (or add explicit test cases) covering both Profit Taker and non–Profit Taker data.
- Confirm that the output in
generateData.js(and any existing test files) reflects the correct structure for all bounty levels.- Consider augmenting the test suite to include varied bounty table scenarios for additional reassurance.
|
what would that look like? could you drop in an example? |
It's the same bounty as the one shown but with changes (some rewards were removed to shorten the example). The only difference is that I believe that by "completion" DE means one full run, i.e. a new player running profit taker phase 3 for the first time will only get a Gravimag (most likely to introduce said new player to heavy guns) {
"id": "d30d044dd734153462ada8512ccf9cbb",
"level": "Level 40 - 60 PROFIT-TAKER - PHASE 3",
"rewards": {
"a": [],
"b": [],
"c": [
{
"id": "6a43383b6a6f13033b6a8ef6bb821d55",
"name": "Gravimag",
"rarity": "Common",
"chance": 100.0,
"stage": "First Completion"
},
{
"id": "1becbf9844d55739ab3861f4682352d7",
"name": "5X Gyromag Systems",
"rarity": "Uncommon",
"chance": 25.0,
"stage": "Subsequent Completions"
},
...
]
}
} |
2 participants
Warframe Drop Data Site Pull Request
What I did:
I used completion runs as stage text since profit taker has different rewards for first runs and subsequent runs
Why I did it:
The tables for profit taker have table headers for "First Completion" and "Second completion" so because these are headers they don't contain more then one element causing the parser to create this json structure
[ { "id": "d30d044dd734153462ada8512ccf9cbb", "level": "Level 40 - 60 PROFIT-TAKER - PHASE 3", "rewards": { "a": [], "b": [], "c": [] } }, { "id": "e5460fddd3491fc6098d28b5b502418a", "level": "First Completion", "rewards": { "a": [], "b": [], "c": [... ] } }, { "id": "370eee33ad52f69bcf9c47021dfeb548", "level": "Subsequent Completions", "rewards": { "a": [], "b": [], "c": [...] } } ]Fixes issue (include link):
Describe the issue, if there was one
Mockups, screenshots, evidence:
Was this an issue fix or a suggestion fulfillment?
Summary by CodeRabbit