fix: correct weapon type classifications in itemTypes.json #805
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdded many new weapon category entries to Changes
Sequence Diagram(s)(omitted — changes are data/type additions and a small classification change; no new multi-component control flow to visualize) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-12-22T12:16:24.709ZApplied to files:
📚 Learning: 2024-12-03T23:47:24.677ZApplied to files:
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reuzehagel
force-pushed
the
fix/weapon-type-classifications
branch
from
155b4dd to
0cb49cb
Compare
|
@Reuzehagel please rebase. (DO NOT MERGE UPSTREAM INTO YOUR BRANCH) |
TobiTenno
force-pushed
the
fix/weapon-type-classifications
branch
from
0cb49cb to
19fc989
Compare
|
@Reuzehagel i just rebased it, you'll want to pull |
![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 (2)
index.d.ts (1)
940-956: WeaponType expansion aligns with data; note TS surface-area changeAdding
Bow,Crossbow,Launcher,Sniper Rifle, andSpeargun(while retainingKitgunandCompanion Weapon) looks correct and matches the new item type mappings, soRivenMod['type']can now express the intended categories via${WeaponType} Riven Mod. This is a widening of the exported union, so TS consumers doing exhaustiveswitch/neverchecks may see new compile-time cases; worth calling out in release notes or a minor/major bump depending on your versioning policy.config/itemTypes.json (1)
966-1177: Primary weapon subclass mappings match design; verify ordering and coverageThis block cleanly encodes the intended primary subclasses:
- Shotguns: Kuva/Vaykor Hek, Kuva Sobek/Drakgoon/Kohm, Drakgoon/Kohm bases, Sancti/Tigris Prime, Astilla/Corinth/Cedo primes, Tenet Arca Plasmor, Mk1-Strun, Bubonico (
InfArmCannon), Convectrix (CrpSplitLaser).- Bows: Mk1-Paris, all
/Bows/and/Bow/paths.- Crossbows: Zhuge/Zhuge Prime, Attica, Nagantaka/Nagantaka Prime.
- Launchers: Ogris/Kuva Ogris, Tonkor/Kuva Tonkor, Penta/Secura/Carmine Penta, Zarr/Kuva Zarr, Torid, Tenet Envoy.
- Spearguns: Afentis, Scourge/Scourge Prime, Javlok (incl. Grineer flame spear variants), Tenet Ferrox.
- Sniper rifles: Rubico/Rubico Prime, Vectis/Vectis Prime, Lanka, Vulkar/Vulkar Wraith, Snipetron/Vandal, Komorex, Sporothrix/Coda Sporothrix, Perigale, plus the generic sniper paths.
The placements before
/LongGuns→RifleandClanTech/.*Weapon→Riflelook correct to ensure these don’t get typed as generic Rifles instead.Two follow-ups to consider:
- Confirm the matcher truly stops on the first hit for non-
appendentries; if it keeps scanning and overwriting, later generic patterns could still re-type these asRifle.- Double-check whether any base variants (e.g., non-Tenet Ferrox, any newer launcher/speargun/sniper with slightly different paths) are missing and should also get explicit entries alongside these.
If both checks pass, this block should fix the reported mis-typings without collateral reclassification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
config/itemTypes.json(2 hunks)index.d.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
config/itemTypes.json
🔇 Additional comments (1)
config/itemTypes.json (1)
906-931: Secondary classifications look right; confirm Grimoire/Tome intentThe new mappings here look consistent with in-game behavior:
/KuvaLich/Secondaries/,VTDetron, andMK1Furis→PistolMK1Kunai→Throwing- Entrati wrist gun and Twin Grakatas →
Pistol/Dual PistolsrespectivelyThe only subtle point is
/Tenno/Grimoire/→PistolwhileTenno/Grimoirelater in the file still maps toTome. Depending on how the matcher walks this array (first-match vs last-match, substring vs regex), one of these will dominate. Please double-check which type you actually want exposed for Grimoire indata/json/Secondary.jsonand that the matching logic won’t silently pick the unintended one.
|
Thanks for the PR always good to fix some mistakes in the data, it seems the type checks were not happy though, gonna check it tomorrow |
|
well, just on the last error in the list, it's a Bow Mod and it's getting |
There was a problem hiding this comment.
First good job i know it takes some time to add these things (if you didn't use ai), most of the additions are okay and could be merged fast but the ones that add a new data type we're gonna to have to discuss them first.
The short and long format for id are not really a thing in the itemTypes.json but using the long format helps in knowing that entry refers to a single item, so long id format should be used here in most entries.
Let's just agree on the already defined types:
Shotgun is basically reserved for weapons that shoots a lot of pellets, that's a shotgun in warframe.
The rifle is a weapon that (in general) shoots one pellet per round and for pratical purposes is basically any weapon that doesn't have a more defined type.
We should discuss the following:
Sniper Rifle should we use just Sniper or Sniper Rifle? Would a sniper be a rifle that has a scope (right click to zoom)?
Pistol would be basically a secondary rifle
Launcher a rifle that shoots a projectile that explodes?
What would the need be in adding speargun and crossbow?
Dont merge the suggestions until @TobiTenno (need your help on this one) puts a thumbs up emoji on them
In retrospect i should have done fewer suggestions with more code in it, it seems github doesnt handle very well this amount of suggestions.
|
i agree with the overall suggestions for change, i'm worried that these are all going to perpetuate a pattern of adding individual items into this itemTypes file, which is not the intention. Commonalities in unique names should be found and summarized, and every item should not need to be overridden to have the right type |
|
I agree with you and i think the same but it's pretty messy sometimes that make any pattern out of it basically impossible, as long as we can figure out a pattern it should take precedence otherwise it's gonna to be messy and there's basically nothing we can do about it (other than not adding these because we dont like it) |
|
@Reuzehagel please refactor the ids that are meant to a single weapon and ill delete the suggestions, its killing browser performance. |
|
For weapon type definitions:
I'll also refactor the IDs to use the full format as suggested when I get around to it tomorrow. |
|
Lowkey no idea what happened... let me try again |
Reuzehagel
force-pushed
the
fix/weapon-type-classifications
branch
from
6231275 to
7d3d59a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/itemTypes.json (1)
967-1043: Consider consolidating individual weapon IDs with regex patterns for performance and maintainability.This section introduces many entries with individual, fully-qualified weapon paths (e.g.,
/Lotus/Weapons/Grineer/KuvaLich/LongGuns/Hek/KuvaHekWeapon). While this approach ensures precise matching, it creates maintenance overhead and was flagged by reviewers as potentially harmful to browser performance when representing single weapons.The code already uses broader pattern-based entries elsewhere in the file (e.g., line 1031–1036 with trailing slashes like
/Lotus/Weapons/Tenno/Bows/). Consolidating similar weapon families with regex patterns would improve performance and reduce duplication.Examples of potential consolidation opportunities:
- Lines 971–985 (Kuva variants: Hek, Sobek, Drakgoon, Kohm) could use a regex pattern like
/(Tenno|Grineer)/KuvaLich/LongGuns/.*withregex: true- Lines 1067–1085 (Launcher entries) include several individual paths that might benefit from broader patterns
Consider reviewing the regex suggestions from prior review comments (e.g., AyAyEm's Bow pattern suggestion) and applying similar consolidation here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
config/itemTypes.json(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
config/itemTypes.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (1)
config/itemTypes.json (1)
1087-1137: Verify individual Sniper rifle IDs match game data and cannot be consolidated further.The Sniper entries mix specific full-path IDs (e.g., lines 1103–1104, 1131–1132) with regex patterns (e.g., lines 1107–1108). Confirm that each individual entry is necessary and that there isn't a shared naming pattern that would allow consolidation (e.g., paths containing
SniperorSniperRifle).This aligns with the earlier feedback about avoiding per-item overrides where patterns are available.
|
Sorry for making some mistakes btw. Ive never really used Git, besides having a single branch for some personal things. |
There was a problem hiding this comment.
Pull request overview
This PR fixes weapon type classifications in the itemTypes.json configuration file to correctly categorize primary weapons by their specific weapon classes (Shotgun, Bow, Sniper, Launcher) instead of defaulting all of them to "Rifle". This is important for mod compatibility in Warframe, as different weapon classes accept different sets of mods. The changes include updates to TypeScript type definitions to add new weapon types ('Bow', 'Launcher', 'Sniper') to the WeaponType union, rename 'Sniper Rifle' to 'Sniper', and remove unused types ('Crossbow', 'Speargun').
Key changes:
- Added ~230 explicit weapon path mappings for Shotguns, Bows, Launchers, and Snipers that were previously falling through to the generic "Rifle" classification
- Added 'Bow', 'Launcher', 'Sniper', 'Companion Weapon', and 'Kitgun' to the WeaponType union
- Renamed 'Sniper Rifle' to 'Sniper' and removed 'Crossbow' and 'Speargun' from ItemType
- Corrected PrimeStrun path from partial to full weapon path
- Added explicit mappings for secondary weapons including Kuva Lich variants, MK1 series, Grimoires, and Twin Grakatas
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| index.d.ts | Updates TypeScript type definitions: adds weapon types to WeaponType union, renames 'Sniper Rifle' to 'Sniper', removes 'Crossbow' and 'Speargun' types |
| config/itemTypes.json | Adds ~240 explicit weapon path mappings to correctly classify Shotguns, Bows, Launchers, Snipers, and various secondary weapons instead of defaulting to "Rifle" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Reuzehagel when you have time, take a look at the requested changes. |
Reuzehagel
force-pushed
the
fix/weapon-type-classifications
branch
from
a93d671 to
3e4b006
Compare
|
Added to the description, the first three are in the unresolved comments above the last one is something i told Reuzehagel in the discord channel |
Head branch was pushed to by a user without write access
Reuzehagel
force-pushed
the
fix/weapon-type-classifications
branch
from
15d020a to
073493e
Compare
|
I keep pressing wrong things lmao. Ill try to update these today |
|
I added citations w/ links to the description checklist |
Yep gonna try to do it rn |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/itemTypes.json (1)
1112-1114: Data generation needed: Primary.json must be rebuilt with the new config to apply correct weapon classifications.The config/itemTypes.json ordering is correct—specific weapon type entries (Shotgun, Sniper, Launcher, Bow) properly precede the
/LongGunscatch-all (line 1112). The matching logic uses first-match-wins via abreakstatement in the parser, so the current ordering will correctly classify weapons. However, Primary.json still reflects old data and needs to be regenerated by running the build script for the new classifications to take effect.
🧹 Nitpick comments (1)
config/itemTypes.json (1)
967-1047: Consider consolidating explicit shotgun entries into regex patterns.While the explicit entries ensure correct classification, many follow predictable patterns that could be consolidated:
- Kuva Lich shotguns (lines 971-985): All follow
/Lotus/Weapons/Grineer/KuvaLich/LongGuns/*/- Prime shotguns (lines 1003-1017): Multiple follow
/Lotus/Weapons/Tenno/LongGuns/Prime*/Consolidating into patterns would reduce the entry count while maintaining correct classification, aligning with the maintainer feedback to "find common naming patterns to summarize groups instead of individual entries."
Example consolidation:
- { - "id": "/Lotus/Weapons/Grineer/KuvaLich/LongGuns/Hek/KuvaHekWeapon", - "name": "Shotgun" - }, - { - "id": "/Lotus/Weapons/Grineer/KuvaLich/LongGuns/Sobek/KuvaSobek", - "name": "Shotgun" - }, - { - "id": "/Lotus/Weapons/Grineer/KuvaLich/LongGuns/Drakgoon/KuvaDrakgoon", - "name": "Shotgun" - }, - { - "id": "/Lotus/Weapons/Grineer/KuvaLich/LongGuns/Kohm/KuvaKohm", - "name": "Shotgun" - }, + { + "id": "/Lotus/Weapons/Grineer/KuvaLich/LongGuns/(Hek|Sobek|Drakgoon|Kohm)/", + "regex": true, + "name": "Shotgun" + },Based on learnings, ensure patterns only include weapons present in the current context to avoid false matches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
config/itemTypes.json(2 hunks)index.d.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
config/itemTypes.json
📚 Learning: 2025-11-28T01:41:19.914Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 778
File: build/parser.mjs:568-574
Timestamp: 2025-11-28T01:41:19.914Z
Learning: In the WFCD/warframe-items repository, for Nightwave Act processing, the regex difference between `build/parser.mjs` (using `/[0-9]{1,3}$/` with anchor) and `build/build.mjs` (using `/[0-9]{1,3}/` without anchor) is intentional and serves different purposes in each file.
Applied to files:
config/itemTypes.json
🔇 Additional comments (5)
index.d.ts (1)
321-323: LGTM: Bow arcane type addition is appropriate.The addition of 'Bow' to the ArcaneType union is consistent with the existing weapon-specific arcane types (Shotgun, Sniper) and aligns with the PR's goal of properly classifying bow weapons.
config/itemTypes.json (4)
906-937: LGTM: Pistol classifications use appropriate patterns and full paths.The pistol entries properly use:
- Pattern-based matching for Kuva Lich secondaries (line 906-909)
- Full path format for performance (addressing earlier maintainer feedback)
- Correct typing for edge cases (e.g., MK1Kunai as "Throwing")
1027-1039: LGTM: Bow classifications properly use regex patterns.The bow entries demonstrate the preferred approach:
- Regex patterns for broad matching (Tenno/Grineer/Corpus/Syndicate variants)
- Minimal explicit entries for edge cases (MK1Paris)
This aligns well with the maintainer guidance to prefer patterns over per-item entries.
1048-1080: LGTM: Launcher classifications balance patterns and explicit entries.The launcher entries use:
- Regex pattern for common grenade/cannon launchers (line 1049-1052)
- Explicit entries for special cases and Kuva variants
The Launcher type is justified based on exclusive mod compatibility (e.g., Adhesive Blast) as discussed in past review comments.
1081-1110: LGTM: Sniper classifications appropriately combine patterns and explicit entries.The sniper entries use:
- Regex patterns to match standard sniper paths (lines 1082-1090)
- Explicit entries for Prime/special variants that don't match patterns
The Sniper type is justified based on scoped functionality and exclusive mod compatibility as verified in past review comments.
Reuzehagel
force-pushed
the
fix/weapon-type-classifications
branch
from
073493e to
235c44d
Compare
|
Wait that undid the regex changes... |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
build/tradable.mjs(1 hunks)config/itemTypes.json(2 hunks)index.d.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- index.d.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
config/itemTypes.json
📚 Learning: 2025-11-28T01:41:19.914Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 778
File: build/parser.mjs:568-574
Timestamp: 2025-11-28T01:41:19.914Z
Learning: In the WFCD/warframe-items repository, for Nightwave Act processing, the regex difference between `build/parser.mjs` (using `/[0-9]{1,3}$/` with anchor) and `build/build.mjs` (using `/[0-9]{1,3}/` without anchor) is intentional and serves different purposes in each file.
Applied to files:
config/itemTypes.json
🔇 Additional comments (4)
build/tradable.mjs (1)
1-1: LGTM!Adding Bow, Launcher, and Sniper to the untradable types list is consistent with the new weapon classifications. The tradableConditions function on line 2 will correctly allow Prime variants of these types to be tradable while keeping base variants untradable.
config/itemTypes.json (3)
906-941: Secondary weapon classifications look reasonable.The combination of the
/KuvaLich/Secondaries/pattern (lines 911-912) with specific overrides for dual pistols and unique weapon paths appears appropriate. Individual entries for MK1-series and special weapons are justified given they lack common path patterns.
1143-1145: Verify classification of Ambassador/Komorex as Rifle vs. Sniper.Line 1143 explicitly classifies
CrpArSniperas Rifle rather than Sniper. Based on past review comments, there was discussion about whether Ambassador should be Rifle. Please confirm this is intentional and aligns with in-game mod compatibility.
1031-1040: Bow pattern entries are sound.The bow patterns (
/Lotus/Weapons/Tenno/Bows/and/Lotus/Weapons/Tenno/Bow/) are correctly placed since bow weapon paths do not contain/LongGuns/in their structure. The entries will not be caught by any/LongGunscatch-all patterns at line 1147.
|
I am so confused again ngl, was my branch out of date and did I recommit those things or something? |
|
yeah, that's what it seems like |
|
ack, sorry i haven't rebased yet. |
Reuzehagel
force-pushed
the
fix/weapon-type-classifications
branch
from
235c44d to
073493e
Compare
TobiTenno
force-pushed
the
fix/weapon-type-classifications
branch
from
073493e to
1848643
Compare
- Add KuvaStubba as Dual Pistols - Classify CrpArSniperRifle as Rifle - Simplify KuvaLich Secondaries path - Add Bow, Launcher, and Sniper to untradable built weapons
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
config/itemTypes.json (1)
971-1118: CRITICAL: Ordering issue causes all specific weapon classifications to be overridden by the /LongGuns catch-all.All weapon entries added here (lines 971-1118) contain
/LongGuns/in their paths and will therefore match the generic/LongGuns→ Rifle rule at line 1120. Due to last-match-wins behavior (confirmed in past review discussions), every weapon in this section will be classified as "Rifle" regardless of their explicit Shotgun/Bow/Launcher/Sniper assignments, completely defeating the purpose of this PR.Past review comments confirmed this issue by examining generated artifacts, and reviewers have requested pattern-based consolidation instead of individual entries.
Solution (aligning with reviewer feedback):
- Move the
/LongGunscatch-all to line 970 (before all specific entries) to establish the Rifle default first- Replace individual weapon entries with consolidated regex patterns placed after the catch-all so they override it:
{ "id": "/LongGuns", "name": "Rifle" }, { "id": "/LongGuns/.*(Hek|Sobek|Drakgoon|Kohm|Tigris|Strun|Astilla|Corinth|Cedo|Boar|ArcaPlasmor|InfArmCannon|CrpSplitLaser)", "regex": true, "name": "Shotgun" }, { "id": "/Bows?/", "regex": true, "name": "Bow" }, { "id": "/LongGuns/.*(Ogris|Tonkor|Zarr|Penta|Launcher|Grenade|Cannon|RocketLauncher|BioWeapon)", "regex": true, "name": "Launcher" }, { "id": "/LongGuns/.*(Sniper|Rubico|Vectis|Lanka|Vulkar|Sporothrix|Perigale|Vadarya|Komorex|Railgun)", "regex": true, "name": "Sniper" }This approach:
- Establishes Rifle as the default via the catch-all
- Uses pattern-based rules that win via last-match-wins
- Drastically reduces per-item entries as requested by reviewers
- Ensures correct weapon classifications
Based on learnings: In the 'warframe-items' repository, when suggesting changes to regex patterns, ensure that added patterns only include items relevant to the current PR context.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
build/tradable.mjs(1 hunks)config/itemTypes.json(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
config/itemTypes.json
📚 Learning: 2025-11-28T01:41:19.914Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 778
File: build/parser.mjs:568-574
Timestamp: 2025-11-28T01:41:19.914Z
Learning: In the WFCD/warframe-items repository, for Nightwave Act processing, the regex difference between `build/parser.mjs` (using `/[0-9]{1,3}$/` with anchor) and `build/build.mjs` (using `/[0-9]{1,3}/` without anchor) is intentional and serves different purposes in each file.
Applied to files:
config/itemTypes.json
🔇 Additional comments (1)
config/itemTypes.json (1)
906-941: LGTM - Secondary weapon mappings use appropriate specificity.The secondary weapon entries correctly use explicit paths where needed and include a pattern-based rule for KuvaLich Secondaries (lines 906-909), which aligns well with the reviewers' preference for patterns over per-item entries.
Co-authored-by: André <[email protected]>
Co-authored-by: André <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
config/itemTypes.json (2)
967-1047: Consider consolidating shotgun families with regex patterns.While the ordering is correct, the shotgun entries could be condensed using weapon-family patterns to reduce maintenance burden. For example, consolidate Hek/Tigris/Drakgoon/Kohm families:
{ "id": "/(Hek|Tigris|Drakgoon|Kohm|Strun|Astilla|Corinth|Cedo|ArcaPlasmor|FlakCannon|SparkRifle|InfArmCannon|SplitLaser)(/|Weapon|Rifle|$)", "regex": true, "name": "Shotgun" }This single pattern could replace many individual entries. However, if DE's data paths are too inconsistent (as noted in PR discussion), explicit entries may be necessary.
Balancing pattern vs explicit entries
Per maintainer feedback, prefer patterns over per-item overrides where possible. However, acknowledge that DE data inconsistencies may require manual entries when no reliable pattern exists.
1049-1080: Good regex pattern; consider consolidating KuvaLich launcher variants.The regex at lines 1049-1052 for
Grenade|Cannonis effective. The KuvaLich entries (lines 1070-1080) could be consolidated:{ "id": "/KuvaLich/LongGuns/(Tonkor|Ogris|Zarr)/", "regex": true, "name": "Launcher" }This would reduce three explicit entries to one pattern, improving maintainability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
build/tradable.mjsconfig/itemTypes.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Learnt from: TobiTenno
Repo: WFCD/warframe-items PR: 805
File: build/tradable.mjs:1-1
Timestamp: 2025-12-22T12:16:24.709Z
Learning: In Warframe, Prime variants of Bow, Launcher, and Sniper weapon types are not tradable, just like Prime variants of Rifle, Shotgun, Pistol, and other built weapon types. These types should be included in the builtUntradable array in build/tradable.mjs.
📚 Learning: 2025-12-22T12:16:24.709Z
Learnt from: TobiTenno
Repo: WFCD/warframe-items PR: 805
File: build/tradable.mjs:1-1
Timestamp: 2025-12-22T12:16:24.709Z
Learning: In Warframe, Prime variants of Bow, Launcher, and Sniper weapon types are not tradable, just like Prime variants of Rifle, Shotgun, Pistol, and other built weapon types. These types should be included in the builtUntradable array in build/tradable.mjs.
Applied to files:
build/tradable.mjsconfig/itemTypes.json
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
build/tradable.mjsconfig/itemTypes.json
📚 Learning: 2025-11-28T01:41:19.914Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 778
File: build/parser.mjs:568-574
Timestamp: 2025-11-28T01:41:19.914Z
Learning: In the WFCD/warframe-items repository, for Nightwave Act processing, the regex difference between `build/parser.mjs` (using `/[0-9]{1,3}$/` with anchor) and `build/build.mjs` (using `/[0-9]{1,3}/` without anchor) is intentional and serves different purposes in each file.
Applied to files:
build/tradable.mjsconfig/itemTypes.json
🔇 Additional comments (4)
build/tradable.mjs (1)
1-12: LGTM! Correct addition of weapon types to untradable list.Adding
Bow,Launcher, andSnipertobuiltUntradablecorrectly ensures their Prime variants remain untradable, consistent with existing built weapon types like Rifle, Shotgun, and Pistol.Based on learnings, Prime variants of Bow, Launcher, and Sniper weapon types are not tradable in Warframe.
config/itemTypes.json (3)
906-937: Correct ordering for first-match-wins logic.The
KuvaLich/Secondaries/default (lines 907-909) is correctly placed before the specificKuvaStubbaentry (lines 935-937). Per the type-matching logic shared by maintainers, the first matching pattern is applied and processing stops, so this ordering ensures KuvaStubba matches the Dual Pistols rule at line 935 rather than falling through to the Pistol default at line 908.
967-1118: Ordering is correct for first-match-wins logic.The specific weapon entries (lines 967-1114) are correctly placed BEFORE the generic
/LongGuns→ Rifle catch-all (line 1116). Per maintainer clarification, the type-matching logic finds the first match and breaks, so this ordering ensures specific classifications take precedence over the generic default.While the ordering is sound, several consolidation opportunities exist (flagged in separate comments) that could reduce the number of per-item entries, as preferred by maintainers.
1082-1114: The explicit sniper entries (lines 1091-1110) are necessary and cannot be consolidated. Verification shows that none of these weapon paths match the existing regex patterns:
- Railgun lacks "Sniper" in its path structure
- RubicoPrime, PrimeLightningGun, and PrimeVectis use alternate naming conventions (ending with "Weapon" or "Rifle" rather than containing "Sniper")
- CodaSporothrix has no sniper indicator in its path
These explicit entries are the only mechanism to classify these weapons as snipers, placed correctly before the
/LongGunscatch-all. The current implementation is sound.
…id the Type change
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
index.d.ts (1)
813-900: Critical: Regenerate data files to complete 'Sniper Rifle' → 'Sniper' migration.Per previous review findings, the Type union literal was changed from 'Sniper Rifle' to 'Sniper', but 63 references remain in generated data files (Primary.json, Mods.json, Arcanes.json, All.json, etc.). Run
npm run buildto regenerate all data files and ensure runtime values match these updated TypeScript definitions.Additionally, reviewers noted that some additions here may be redundant with the
WeaponTypeunion. Confirm whether all additions at lines 813-900 are necessary or if some should rely solely on theWeaponTypeunion reference at line 899.config/itemTypes.json (1)
906-937: Refactor: Consolidate duplicate Grimoire entries.The Pistol section correctly orders specific overrides (KuvaStubba, line 907-909) before the generic KuvaLich/Secondaries pattern (line 911-913). However, per previous reviewer feedback, the Grimoire entry at lines 927-929 can be simplified to a single pattern.
🔎 Suggested consolidation
Replace lines 927-929 with:
{ - "id": "/Weapons/Tenno/Grimoire/", + "id": "/Weapons/Tenno/Grimoire/", "name": "Pistol" },(Removing any duplicate Grimoire entries if they exist elsewhere)
Based on past review comments.
🧹 Nitpick comments (3)
config/itemTypes.json (3)
1049-1080: Consider additional pattern consolidation for Launcher entries.While the Launcher section includes a regex pattern at lines 1049-1052, it still contains many per-item entries (lines 1054-1080). Per reviewer guidance to minimize individual overrides, these could be consolidated into a more comprehensive pattern.
🔎 Suggested pattern consolidation
Consider replacing lines 1054-1080 with an expanded regex that matches launcher weapon families:
{ "id": "/(Ogris|Tonkor|Zarr|Penta|Launcher|Grenade|Cannon|RocketLauncher|BioWeapon|Briefcase)", "regex": true, "name": "Launcher" }This would capture most launchers and reduce maintenance burden. Keep per-item entries only for edge cases that don't match common patterns.
Based on reviewer feedback preferring pattern-based rules.
1082-1114: Sniper section: Ambassador exception is correct, but consider further consolidation.The explicit classification of Ambassador (CrpArSniper) as Rifle (lines 1082-1084) correctly implements previous reviewer discussion. The regex patterns at lines 1086-1094 are good, but the remaining per-item entries (lines 1096-1114) could be consolidated.
🔎 Suggested pattern consolidation
Consider adding a single regex after the existing patterns to capture remaining sniper families:
{ "id": "/(Sniper|Rubico|Vectis|Lanka|Vulkar|Sporothrix|Perigale|Vadarya|Komorex|Railgun|LightningGun)", "regex": true, "name": "Sniper" }This would reduce the need for explicit entries for each Prime/Kuva/Tenet variant.
Based on reviewer feedback preferring pattern-based rules.
967-1047: Consider consolidating repeated Shotgun family entries into pattern-based rules for maintainability.The individual Shotgun entries (lines 967-1025, 1041-1047) are correctly positioned before the
/LongGunscatch-all (line 1116) and the paths match the actual weaponuniqueNamevalues, so they will classify correctly once the build is run. However, consolidating similar weapons into regex patterns would reduce maintenance burden and improve readability.For example, weapons like Hek, Sobek, Drakgoon, Kohm, Tigris, Strun, Astilla, Corinth, Cedo, ArcaPlasmor, InfArmCannon, and CrpSplitLaser could be matched by their distinctive names using a single pattern rather than individual entries.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
config/itemTypes.jsonindex.d.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Learnt from: TobiTenno
Repo: WFCD/warframe-items PR: 805
File: build/tradable.mjs:1-1
Timestamp: 2025-12-22T12:16:24.709Z
Learning: In Warframe, Prime variants of Bow, Launcher, and Sniper weapon types are not tradable, just like Prime variants of Rifle, Shotgun, Pistol, and other built weapon types. These types should be included in the builtUntradable array in build/tradable.mjs.
📚 Learning: 2025-12-22T12:16:24.709Z
Learnt from: TobiTenno
Repo: WFCD/warframe-items PR: 805
File: build/tradable.mjs:1-1
Timestamp: 2025-12-22T12:16:24.709Z
Learning: In Warframe, Prime variants of Bow, Launcher, and Sniper weapon types are not tradable, just like Prime variants of Rifle, Shotgun, Pistol, and other built weapon types. These types should be included in the builtUntradable array in build/tradable.mjs.
Applied to files:
index.d.tsconfig/itemTypes.json
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
index.d.tsconfig/itemTypes.json
📚 Learning: 2025-11-28T01:41:19.914Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 778
File: build/parser.mjs:568-574
Timestamp: 2025-11-28T01:41:19.914Z
Learning: In the WFCD/warframe-items repository, for Nightwave Act processing, the regex difference between `build/parser.mjs` (using `/[0-9]{1,3}$/` with anchor) and `build/build.mjs` (using `/[0-9]{1,3}/` without anchor) is intentional and serves different purposes in each file.
Applied to files:
config/itemTypes.json
🔇 Additional comments (2)
index.d.ts (1)
939-953: LGTM! WeaponType union properly updated for new classifications.The additions of 'Bow', 'Launcher', 'Sniper', 'Kitgun', and 'Companion Weapon' align with the PR's goal to correct weapon type classifications. The replacement of 'Sniper Rifle' with 'Sniper' is consistent with the broader changes.
Per retrieved learnings, Prime variants of Bow, Launcher, and Sniper should be marked untradable in
build/tradable.mjs, which appears to have been addressed.config/itemTypes.json (1)
1027-1039: LGTM! Bow classification uses pattern-based approach.The Bow entries properly leverage regex patterns (lines 1031-1039) to match weapon families rather than listing individual weapons. The specific MK1Paris entry (line 1027-1029) is appropriately placed before the broader patterns.
This approach aligns with reviewer guidance to prefer patterns over per-item entries.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
index.d.ts (1)
813-813: Remove redundant Type union entries already covered by WeaponType.Lines 813 ('Bow'), 817 ('Companion Weapon'), and 848 ('Launcher') add literals to the
Typeunion that are also being added toWeaponType(lines 942-945). SinceTypealready includesWeaponTypeat line 898, these explicit additions are redundant and create unnecessary maintenance overhead.Based on past review comments, these should be removed.
🔎 Proposed fix
Remove the three redundant lines from the Type union:
| 'Boosters' - | 'Bow' | 'Captura'| 'Color Palette' - | 'Companion Weapon' | 'Conservation Tag'| 'Kubrow' - | 'Launcher' | 'Medallion'These types will still be included in
Typethrough theWeaponTypeunion at line 898.Also applies to: 817-817, 848-848
♻️ Duplicate comments (1)
index.d.ts (1)
951-951: Duplicate: 'Sniper Rifle' → 'Sniper' migration requires data file regeneration.This change from 'Sniper Rifle' to 'Sniper' in the TypeScript definitions has already been flagged in a previous review comment. The generated data files (Primary.json, Mods.json, Arcanes.json, All.json, etc.) still contain 63 references to 'Sniper Rifle', creating a type-safety violation.
Run
npm run buildto regenerate all data files so runtime values match the TypeScript definitions.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
index.d.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Learnt from: TobiTenno
Repo: WFCD/warframe-items PR: 805
File: build/tradable.mjs:1-1
Timestamp: 2025-12-22T12:16:24.709Z
Learning: In Warframe, Prime variants of Bow, Launcher, and Sniper weapon types are not tradable, just like Prime variants of Rifle, Shotgun, Pistol, and other built weapon types. These types should be included in the builtUntradable array in build/tradable.mjs.
📚 Learning: 2025-12-22T12:16:24.709Z
Learnt from: TobiTenno
Repo: WFCD/warframe-items PR: 805
File: build/tradable.mjs:1-1
Timestamp: 2025-12-22T12:16:24.709Z
Learning: In Warframe, Prime variants of Bow, Launcher, and Sniper weapon types are not tradable, just like Prime variants of Rifle, Shotgun, Pistol, and other built weapon types. These types should be included in the builtUntradable array in build/tradable.mjs.
Applied to files:
index.d.ts
📚 Learning: 2024-12-03T23:47:24.677Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 663
File: config/masterableCategories.json:1-2
Timestamp: 2024-12-03T23:47:24.677Z
Learning: In the 'warframe-items' repository, when suggesting changes to regex patterns in 'config/masterableCategories.json', ensure that added patterns only include items relevant to the current PR context, and avoid adding items not present in the context.
Applied to files:
index.d.ts
🔇 Additional comments (1)
index.d.ts (1)
942-945: LGTM! New weapon type literals support classification goals.The additions of 'Bow', 'Kitgun', 'Launcher', and 'Companion Weapon' to the
WeaponTypeunion are appropriate for this PR's objective of correcting weapon type classifications for mod compatibility. These align with the changes being made inconfig/itemTypes.jsonand the existingArcaneTypeentries (lines 321-323).
|
🎉 This PR is included in version 1.1272.87 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
What did you fix?
Primary weapons were incorrectly typed as "Rifle" when they should have specific weapon class types for mod compatibility:
These weapons were falling through to the itemType patterns which assigned them "Rifle" type.
Reproduction steps
npm run builddata/json/Primary.jsonfor weapons like "Kuva Hek", "Paris", "Ogris""type": "Rifle"instead of their correct weapon classEvidence/screenshot/link to line
Reference: https://wiki.warframe.com/w/Template:WeaponNav#Primary
And Discord messages sent by Reuzehagel (Nov 27th 20:48)
The Wiki categorizes these weapons into classes (Shotguns, Bows, Sniper Rifles, etc.) which affects mod compatibility.
Considerations
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Changes needed