feat: rerolling rivens #806

Sainan · 2025-01-18T05:08:15-08:00

Sainan commented

2025-01-18 05:08:15 -08:00

No description provided.

coderabbitai[bot] commented

2025-01-18 05:08:28 -08:00

(Migrated from github.com)

Walkthrough

This pull request introduces changes across multiple files to enhance the functionality of a mod reroll system in an application. The modifications involve updating the warframe-public-export-plus dependency to version 0.5.24 in the package.json file.

The primary focus is on the rerollRandomModController in the API controllers, which now implements an asynchronous approach to handling mod rerolls. The changes include the introduction of several new types and interfaces to support the reroll process, the addition of a randomiseStats function, and the implementation of detailed logic for managing inventory upgrades.

Complementary updates were made to the inventory model, specifically changing the upgradesSchema type and adding a PendingRerollFingerprint field. Additionally, modifications to the IInventoryDatabase interface were made to update property types and enhance type safety, aligning with MongoDB object representations. The getInventory function in the inventory service was also updated to allow for an optional projection parameter, enhancing data retrieval flexibility. These modifications reflect a comprehensive refinement of the mod reroll mechanism and its underlying data structures.

Finishing Touches

📝 Generate Docstrings (Beta)

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?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

## Walkthrough This pull request introduces changes across multiple files to enhance the functionality of a mod reroll system in an application. The modifications involve updating the `warframe-public-export-plus` dependency to version `0.5.24` in the `package.json` file. The primary focus is on the `rerollRandomModController` in the API controllers, which now implements an asynchronous approach to handling mod rerolls. The changes include the introduction of several new types and interfaces to support the reroll process, the addition of a `randomiseStats` function, and the implementation of detailed logic for managing inventory upgrades. Complementary updates were made to the inventory model, specifically changing the `upgradesSchema` type and adding a `PendingRerollFingerprint` field. Additionally, modifications to the `IInventoryDatabase` interface were made to update property types and enhance type safety, aligning with MongoDB object representations. The `getInventory` function in the inventory service was also updated to allow for an optional projection parameter, enhancing data retrieval flexibility. These modifications reflect a comprehensive refinement of the mod reroll mechanism and its underlying data structures.      ## Finishing Touches - [ ]  📝 Generate Docstrings (Beta)   --- 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? <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> <details> <summary>🪧 Tips</summary> ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit <commit_id>, please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` 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) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai generate docstrings` to [generate docstrings](https://docs.coderabbit.ai/finishing-touches/docstrings) for this PR. ([Beta](https://docs.coderabbit.ai/early-access#beta)) - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements. </details>

coderabbitai[bot] (Migrated from github.com) reviewed 2025-01-18 05:11:52 -08:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 5

🧹 Nitpick comments (2)

src/controllers/api/rerollRandomModController.ts (1)
92-92: Correct the type definition of lvlReq in IUnveiledRivenFingerprint.

The property lvlReq is currently defined as lvlReq: 0;, which sets its type to the literal 0. This should be typed as number to allow for flexibility in assigning different numeric values.

Apply this diff to correct the type:
- lvlReq: 0;
+ lvlReq: number;
src/models/inventoryModels/inventoryModel.ts (1)
289-289: Consider omitting the redundant required: false in schema definition.

In Mongoose schema definitions, the default value for required is false. Therefore, explicitly setting required: false is redundant and can be omitted for cleaner code.

Apply this diff to remove the redundancy:
PendingRerollFingerprint: { type: String, required: false },
PendingRerollFingerprint: { type: String },

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1c3dcbefc and 29a9fe5716.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (4)

package.json (1 hunks)
src/controllers/api/rerollRandomModController.ts (1 hunks)
src/models/inventoryModels/inventoryModel.ts (1 hunks)
src/types/inventoryTypes/inventoryTypes.ts (1 hunks)

✅ Files skipped from review due to trivial changes (1)

package.json

🔇 Additional comments (2)

src/types/inventoryTypes/inventoryTypes.ts (1)

422-422: Addition of PendingRerollFingerprint property to ICrewShipSalvagedWeaponSkin.

The addition of the PendingRerollFingerprint?: string property to the ICrewShipSalvagedWeaponSkin interface is appropriate and aligns with the changes in the controller logic. Ensure that all implementations and usages of this interface are updated accordingly.

src/models/inventoryModels/inventoryModel.ts (1)

286-289: Ensure consistency after updating upgradesSchema type and adding new property.

The upgradesSchema has been updated to use the ICrewShipSalvagedWeaponSkin type, and the PendingRerollFingerprint property has been added. This change enhances type safety and aligns the schema with the interface.

**Actionable comments posted: 5** <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/controllers/api/rerollRandomModController.ts (1)</summary><blockquote> `92-92`: **Correct the type definition of `lvlReq` in `IUnveiledRivenFingerprint`.** The property `lvlReq` is currently defined as `lvlReq: 0;`, which sets its type to the literal `0`. This should be typed as `number` to allow for flexibility in assigning different numeric values. Apply this diff to correct the type: ```diff - lvlReq: 0; + lvlReq: number; ``` </blockquote></details> <details> <summary>src/models/inventoryModels/inventoryModel.ts (1)</summary><blockquote> `289-289`: **Consider omitting the redundant `required: false` in schema definition.** In Mongoose schema definitions, the default value for `required` is `false`. Therefore, explicitly setting `required: false` is redundant and can be omitted for cleaner code. Apply this diff to remove the redundancy: ```diff PendingRerollFingerprint: { type: String, required: false }, PendingRerollFingerprint: { type: String }, ``` </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between f1c3dcbefc1dcc536c77c1accdd0c2fc3b13e04b and 29a9fe571636008a43461a299a08eb5818d78776. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (4)</summary> * `package.json` (1 hunks) * `src/controllers/api/rerollRandomModController.ts` (1 hunks) * `src/models/inventoryModels/inventoryModel.ts` (1 hunks) * `src/types/inventoryTypes/inventoryTypes.ts` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * package.json </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>src/types/inventoryTypes/inventoryTypes.ts (1)</summary> `422-422`: **Addition of `PendingRerollFingerprint` property to `ICrewShipSalvagedWeaponSkin`.** The addition of the `PendingRerollFingerprint?: string` property to the `ICrewShipSalvagedWeaponSkin` interface is appropriate and aligns with the changes in the controller logic. Ensure that all implementations and usages of this interface are updated accordingly. </details> <details> <summary>src/models/inventoryModels/inventoryModel.ts (1)</summary> `286-289`: **Ensure consistency after updating `upgradesSchema` type and adding new property.** The `upgradesSchema` has been updated to use the `ICrewShipSalvagedWeaponSkin` type, and the `PendingRerollFingerprint` property has been added. This change enhances type safety and aligns the schema with the interface. </details> </blockquote></details> </details>

src/controllers/api/rerollRandomModController.ts

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 05:11:51 -08:00

⚠️ Potential issue

Handle potential null references when accessing inventory upgrades.

Similar to the previous case, the code uses the null assertion operator ! without checking if the upgrade exists. This may lead to runtime errors if the upgrade is not found.

Apply this diff to add null checks:

- const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId)!;
+ const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId);
+ if (!upgrade) {
+     return res.status(404).json({ error: "Upgrade not found in inventory." });
+ }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId);
        if (!upgrade) {
            return res.status(404).json({ error: "Upgrade not found in inventory." });
        }
        if (request.CommitReroll) {

_:warning: Potential issue_ **Handle potential null references when accessing inventory upgrades.** Similar to the previous case, the code uses the null assertion operator `!` without checking if the upgrade exists. This may lead to runtime errors if the upgrade is not found. Apply this diff to add null checks: ```diff - const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId)!; + const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId); + if (!upgrade) { + return res.status(404).json({ error: "Upgrade not found in inventory." }); + } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId); if (!upgrade) { return res.status(404).json({ error: "Upgrade not found in inventory." }); } if (request.CommitReroll) { ````` </details>

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 05:11:51 -08:00

⚠️ Potential issue

Prevent potential infinite loops when selecting curses.

Similarly, if none of the meta.upgradeEntries have canBeCurse set to true, the while loop will result in an infinite loop. Ensure that there is at least one entry that can be a curse before entering the loop.

Apply this diff to check for curseable entries:

+ const curseableEntries = meta.upgradeEntries!.filter(entry => entry.canBeCurse);
+ if (curseableEntries.length === 0) {
+     // It's acceptable for an upgrade to have no curses
+     break;
+ }
- let entry = getRandomElement(meta.upgradeEntries!);
+ let entry = getRandomElement(curseableEntries);
- while (!entry.canBeCurse) {
-     entry = getRandomElement(meta.upgradeEntries!);
- }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        const curseableEntries = meta.upgradeEntries!.filter(entry => entry.canBeCurse);
        if (curseableEntries.length === 0) {
            // It's acceptable for an upgrade to have no curses
            break;
        }
        let entry = getRandomElement(curseableEntries);
        fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });

_:warning: Potential issue_ **Prevent potential infinite loops when selecting curses.** Similarly, if none of the `meta.upgradeEntries` have `canBeCurse` set to `true`, the `while` loop will result in an infinite loop. Ensure that there is at least one entry that can be a curse before entering the loop. Apply this diff to check for curseable entries: ```diff + const curseableEntries = meta.upgradeEntries!.filter(entry => entry.canBeCurse); + if (curseableEntries.length === 0) { + // It's acceptable for an upgrade to have no curses + break; + } - let entry = getRandomElement(meta.upgradeEntries!); + let entry = getRandomElement(curseableEntries); - while (!entry.canBeCurse) { - entry = getRandomElement(meta.upgradeEntries!); - } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion const curseableEntries = meta.upgradeEntries!.filter(entry => entry.canBeCurse); if (curseableEntries.length === 0) { // It's acceptable for an upgrade to have no curses break; } let entry = getRandomElement(curseableEntries); fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); ````` </details>

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 05:11:51 -08:00

⚠️ Potential issue

Prevent potential infinite loops when selecting buffs.

If none of the meta.upgradeEntries have canBeBuff set to true, the while loop will result in an infinite loop. To prevent this, ensure that there is at least one entry that can be a buff before entering the loop.

Apply this diff to check for buffable entries:

+ const buffableEntries = meta.upgradeEntries!.filter(entry => entry.canBeBuff);
+ if (buffableEntries.length === 0) {
+     throw new Error("No available buffs for this upgrade type.");
+ }
- let entry = getRandomElement(meta.upgradeEntries!);
+ let entry = getRandomElement(buffableEntries);
- while (!entry.canBeBuff) {
-     entry = getRandomElement(meta.upgradeEntries!);
- }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        const buffableEntries = meta.upgradeEntries!.filter(entry => entry.canBeBuff);
        if (buffableEntries.length === 0) {
            throw new Error("No available buffs for this upgrade type.");
        }
        let entry = getRandomElement(buffableEntries);
        fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });
    }

_:warning: Potential issue_ **Prevent potential infinite loops when selecting buffs.** If none of the `meta.upgradeEntries` have `canBeBuff` set to `true`, the `while` loop will result in an infinite loop. To prevent this, ensure that there is at least one entry that can be a buff before entering the loop. Apply this diff to check for buffable entries: ```diff + const buffableEntries = meta.upgradeEntries!.filter(entry => entry.canBeBuff); + if (buffableEntries.length === 0) { + throw new Error("No available buffs for this upgrade type."); + } - let entry = getRandomElement(meta.upgradeEntries!); + let entry = getRandomElement(buffableEntries); - while (!entry.canBeBuff) { - entry = getRandomElement(meta.upgradeEntries!); - } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion const buffableEntries = meta.upgradeEntries!.filter(entry => entry.canBeBuff); if (buffableEntries.length === 0) { throw new Error("No available buffs for this upgrade type."); } let entry = getRandomElement(buffableEntries); fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); } ````` </details>

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 05:11:51 -08:00

⚠️ Potential issue

Handle potential null references when accessing inventory upgrades.

Currently, the code uses the null assertion operator ! when accessing the upgrade in the inventory without checking if it exists. If the upgrade is not found, this will cause a runtime error. It's important to add a null check to handle cases where the upgrade may not exist.

Apply this diff to add null checks:

- const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0])!;
+ const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0]);
+ if (!upgrade) {
+     return res.status(404).json({ error: "Upgrade not found in inventory." });
+ }

Committable suggestion skipped: line range outside the PR's diff.

_:warning: Potential issue_ **Handle potential null references when accessing inventory upgrades.** Currently, the code uses the null assertion operator `!` when accessing the upgrade in the inventory without checking if it exists. If the upgrade is not found, this will cause a runtime error. It's important to add a null check to handle cases where the upgrade may not exist. Apply this diff to add null checks: ```diff - const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0])!; + const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0]); + if (!upgrade) { + return res.status(404).json({ error: "Upgrade not found in inventory." }); + } ``` > Committable suggestion skipped: line range outside the PR's diff.

src/controllers/api/rerollRandomModController.ts

						
				@ -7,0 +49,4 @@

				        upgrade.PendingRerollFingerprint = undefined;

				        await inventory.save();

				        res.send(upgrade.UpgradeFingerprint);

				    }

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 05:11:51 -08:00

⚠️ Potential issue

Check for undefined PendingRerollFingerprint before committing reroll.

When CommitReroll is true, the code assigns upgrade.PendingRerollFingerprint to upgrade.UpgradeFingerprint without verifying if PendingRerollFingerprint exists. This could result in upgrade.UpgradeFingerprint being set to undefined.

Apply this diff to ensure PendingRerollFingerprint is defined:

if (request.CommitReroll) {
+   if (upgrade.PendingRerollFingerprint) {
      upgrade.UpgradeFingerprint = upgrade.PendingRerollFingerprint;
+   } else {
+     return res.status(400).json({ error: "No pending reroll to commit." });
+   }
}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            if (upgrade.PendingRerollFingerprint) {
                upgrade.UpgradeFingerprint = upgrade.PendingRerollFingerprint;
            } else {
                return res.status(400).json({ error: "No pending reroll to commit." });
            }

_:warning: Potential issue_ **Check for undefined `PendingRerollFingerprint` before committing reroll.** When `CommitReroll` is `true`, the code assigns `upgrade.PendingRerollFingerprint` to `upgrade.UpgradeFingerprint` without verifying if `PendingRerollFingerprint` exists. This could result in `upgrade.UpgradeFingerprint` being set to `undefined`. Apply this diff to ensure `PendingRerollFingerprint` is defined: ```diff if (request.CommitReroll) { + if (upgrade.PendingRerollFingerprint) { upgrade.UpgradeFingerprint = upgrade.PendingRerollFingerprint; + } else { + return res.status(400).json({ error: "No pending reroll to commit." }); + } } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion if (upgrade.PendingRerollFingerprint) { upgrade.UpgradeFingerprint = upgrade.PendingRerollFingerprint; } else { return res.status(400).json({ error: "No pending reroll to commit." }); } ````` </details>

coderabbitai[bot] (Migrated from github.com) reviewed 2025-01-18 05:18:45 -08:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 1

♻️ Duplicate comments (4)

src/controllers/api/rerollRandomModController.ts (4)
13-14: ⚠️ Potential issue

Handle potential null references when accessing inventory upgrades.

Currently, the code uses the null assertion operator ! when accessing the upgrade in the inventory without checking if it exists. If the upgrade is not found, this will cause a runtime error. It's important to add a null check to handle cases where the upgrade may not exist.

Apply this diff to add null checks:
- const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0])!;
+ const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0]);
+ if (!upgrade) {
+     return res.status(404).json({ error: "Upgrade not found in inventory." });
+ }
43-44: ⚠️ Potential issue

Handle potential null references when accessing inventory upgrades.

Currently, the code uses the null assertion operator ! when accessing the upgrade in the inventory without checking if it exists. If the upgrade is not found, this will cause a runtime error. It's important to add a null check to handle cases where the upgrade may not exist.

Apply this diff to add null checks:
- const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId)!;
+ const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId);
+ if (!upgrade) {
+     return res.status(404).json({ error: "Upgrade not found in inventory." });
+ }
58-62: ⚠️ Potential issue

Handle cases where no buff entries are available.

If buffEntries is empty, calling getRandomElement(buffEntries) will result in an error or undefined behavior. Ensure that there are available buffs before attempting to select one.

Apply this diff to add a check:
 const buffEntries = meta.upgradeEntries!.filter(x => x.canBeBuff);
+ if (buffEntries.length === 0) {
+     // Handle case where no buff entries are available
+     return;
+ }
 for (let i = 0; i != numBuffs; ++i) {
     const entry = getRandomElement(buffEntries);
     fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });
 }
66-67: ⚠️ Potential issue

Handle cases where no curse entries are available.

If there are no entries that can be curses, getRandomElement may cause an error. It's essential to check if there are curseable entries before attempting to select one.

Apply this diff to add a check:
 if (Math.random() < 0.5) {
+   const curseEntries = meta.upgradeEntries!.filter(x => x.canBeCurse);
+   if (curseEntries.length > 0) {
       const entry = getRandomElement(curseEntries);
       fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });
+   }
 }

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29a9fe5716 and b6cb000078.

📒 Files selected for processing (1)

src/controllers/api/rerollRandomModController.ts (1 hunks)

🔇 Additional comments (1)

src/controllers/api/rerollRandomModController.ts (1)

44-46: Good job fixing the undefined PendingRerollFingerprint issue.

The code now correctly checks if upgrade.PendingRerollFingerprint exists before assigning it to UpgradeFingerprint, preventing potential undefined assignments.

**Actionable comments posted: 1** <details> <summary>♻️ Duplicate comments (4)</summary><blockquote> <details> <summary>src/controllers/api/rerollRandomModController.ts (4)</summary><blockquote> `13-14`: _:warning: Potential issue_ **Handle potential null references when accessing inventory upgrades.** Currently, the code uses the null assertion operator `!` when accessing the upgrade in the inventory without checking if it exists. If the upgrade is not found, this will cause a runtime error. It's important to add a null check to handle cases where the upgrade may not exist. Apply this diff to add null checks: ```diff - const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0])!; + const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemIds[0]); + if (!upgrade) { + return res.status(404).json({ error: "Upgrade not found in inventory." }); + } ``` --- `43-44`: _:warning: Potential issue_ **Handle potential null references when accessing inventory upgrades.** Currently, the code uses the null assertion operator `!` when accessing the upgrade in the inventory without checking if it exists. If the upgrade is not found, this will cause a runtime error. It's important to add a null check to handle cases where the upgrade may not exist. Apply this diff to add null checks: ```diff - const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId)!; + const upgrade = inventory.Upgrades.find(x => x._id?.toString() == request.ItemId); + if (!upgrade) { + return res.status(404).json({ error: "Upgrade not found in inventory." }); + } ``` --- `58-62`: _:warning: Potential issue_ **Handle cases where no buff entries are available.** If `buffEntries` is empty, calling `getRandomElement(buffEntries)` will result in an error or undefined behavior. Ensure that there are available buffs before attempting to select one. Apply this diff to add a check: ```diff const buffEntries = meta.upgradeEntries!.filter(x => x.canBeBuff); + if (buffEntries.length === 0) { + // Handle case where no buff entries are available + return; + } for (let i = 0; i != numBuffs; ++i) { const entry = getRandomElement(buffEntries); fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); } ``` --- `66-67`: _:warning: Potential issue_ **Handle cases where no curse entries are available.** If there are no entries that can be curses, `getRandomElement` may cause an error. It's essential to check if there are curseable entries before attempting to select one. Apply this diff to add a check: ```diff if (Math.random() < 0.5) { + const curseEntries = meta.upgradeEntries!.filter(x => x.canBeCurse); + if (curseEntries.length > 0) { const entry = getRandomElement(curseEntries); fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); + } } ``` </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 29a9fe571636008a43461a299a08eb5818d78776 and b6cb000078ef034d11f6e69fb4fdfe540091345f. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/controllers/api/rerollRandomModController.ts` (1 hunks) </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>src/controllers/api/rerollRandomModController.ts (1)</summary> `44-46`: **Good job fixing the undefined `PendingRerollFingerprint` issue.** The code now correctly checks if `upgrade.PendingRerollFingerprint` exists before assigning it to `UpgradeFingerprint`, preventing potential undefined assignments. </details> </blockquote></details> </details>

src/controllers/api/rerollRandomModController.ts

						
				@ -10,0 +58,4 @@

				    fingerprint.buffs = [];

				    const numBuffs = 2 + Math.trunc(Math.random() * 2); // 2 or 3

				    const buffEntries = meta.upgradeEntries!.filter(x => x.canBeBuff);

				    for (let i = 0; i != numBuffs; ++i) {

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 05:18:44 -08:00

⚠️ Potential issue

Check for undefined meta when accessing ExportUpgrades[randomModType].

It's possible that randomModType is not a key in ExportUpgrades, resulting in meta being undefined. Accessing meta.upgradeEntries would then cause a runtime error. It's important to check if meta exists before proceeding.

Apply this diff to add a null check:

 const meta = ExportUpgrades[randomModType];
+ if (!meta) {
+     throw new Error(`No upgrade data found for mod type: ${randomModType}`);
+ }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    const meta = ExportUpgrades[randomModType];
    if (!meta) {
        throw new Error(`No upgrade data found for mod type: ${randomModType}`);
    }

_:warning: Potential issue_ **Check for undefined `meta` when accessing `ExportUpgrades[randomModType]`.** It's possible that `randomModType` is not a key in `ExportUpgrades`, resulting in `meta` being `undefined`. Accessing `meta.upgradeEntries` would then cause a runtime error. It's important to check if `meta` exists before proceeding. Apply this diff to add a null check: ```diff const meta = ExportUpgrades[randomModType]; + if (!meta) { + throw new Error(`No upgrade data found for mod type: ${randomModType}`); + } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion const meta = ExportUpgrades[randomModType]; if (!meta) { throw new Error(`No upgrade data found for mod type: ${randomModType}`); } ````` </details>

nrbdev commented

2025-01-18 06:00:58 -08:00

(Migrated from github.com)

at \src\services\inventoryService.ts:660:43
    at Array.forEach (<anonymous>)
    at addMiscItems (\src\services\inventoryService.ts:656:17)
    at rerollRandomModController (\src\controllers\api\rerollRandomModController.ts:17:21)```

Error that occurs when rerolling back to back

```ValidationError: Inventory validation failed: MiscItems.122.ItemCount: Cast to Number failed for value "NaN" (type number) at path "ItemCount" while processing /api/rerollRandomMod.php request at \src\services\inventoryService.ts:660:43 at Array.forEach (<anonymous>) at addMiscItems (\src\services\inventoryService.ts:656:17) at rerollRandomModController (\src\controllers\api\rerollRandomModController.ts:17:21)``` Error that occurs when rerolling back to back

Sainan commented

2025-01-18 06:02:26 -08:00

What's the error message?

coderabbitai[bot] (Migrated from github.com) reviewed 2025-01-18 06:11:01 -08:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 5

🧹 Nitpick comments (1)

src/controllers/api/rerollRandomModController.ts (1)
101-101: Consider moving rerollCosts to a configuration file.

The reroll costs array would be better maintained in a separate configuration file, making it easier to adjust costs without modifying controller logic.

Consider creating a new file config/gameConstants.ts:
export const RIVEN_CONSTANTS = {
    REROLL_COSTS: [900, 1000, 1200, 1400, 1700, 2000, 2350, 2750, 3150],
    DEFAULT_COST: 3500,
    // Add other riven-related constants here
} as const;

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b6cb000078 and 65f1fe28c1.

📒 Files selected for processing (1)

src/controllers/api/rerollRandomModController.ts (1 hunks)

**Actionable comments posted: 5** <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>src/controllers/api/rerollRandomModController.ts (1)</summary><blockquote> `101-101`: **Consider moving rerollCosts to a configuration file.** The reroll costs array would be better maintained in a separate configuration file, making it easier to adjust costs without modifying controller logic. Consider creating a new file `config/gameConstants.ts`: ```typescript export const RIVEN_CONSTANTS = { REROLL_COSTS: [900, 1000, 1200, 1400, 1700, 2000, 2350, 2750, 3150], DEFAULT_COST: 3500, // Add other riven-related constants here } as const; ``` </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between b6cb000078ef034d11f6e69fb4fdfe540091345f and 65f1fe28c149e548ac674e30ab7560518799abb4. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/controllers/api/rerollRandomModController.ts` (1 hunks) </details> </details>

src/controllers/api/rerollRandomModController.ts

						
				@ -7,0 +18,4 @@

				        addMiscItems(inventory, [

				            {

				                ItemType: "/Lotus/Types/Items/MiscItems/Kuva",

				                ItemCount: kuvaCost * -1

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 06:11:01 -08:00

⚠️ Potential issue

Add validation for rerolls count and kuva cost calculation.

The current implementation could lead to NaN values if fingerprint.rerolls is undefined, causing inventory validation errors. This matches the reported error in the PR comments.

Apply this diff to add proper validation:

-        fingerprint.rerolls ??= 0;
-        const kuvaCost = fingerprint.rerolls < rerollCosts.length ? rerollCosts[fingerprint.rerolls] : 3500;
+        const currentRerolls = fingerprint.rerolls ?? 0;
+        if (typeof currentRerolls !== 'number' || isNaN(currentRerolls)) {
+            return res.status(400).json({ error: "Invalid reroll count" });
+        }
+        const kuvaCost = currentRerolls < rerollCosts.length ? rerollCosts[currentRerolls] : 3500;
+        fingerprint.rerolls = currentRerolls + 1;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        const currentRerolls = fingerprint.rerolls ?? 0;
        if (typeof currentRerolls !== 'number' || isNaN(currentRerolls)) {
            return res.status(400).json({ error: "Invalid reroll count" });
        }
        const kuvaCost = currentRerolls < rerollCosts.length ? rerollCosts[currentRerolls] : 3500;
        fingerprint.rerolls = currentRerolls + 1;

_:warning: Potential issue_ **Add validation for rerolls count and kuva cost calculation.** The current implementation could lead to NaN values if `fingerprint.rerolls` is undefined, causing inventory validation errors. This matches the reported error in the PR comments. Apply this diff to add proper validation: ```diff - fingerprint.rerolls ??= 0; - const kuvaCost = fingerprint.rerolls < rerollCosts.length ? rerollCosts[fingerprint.rerolls] : 3500; + const currentRerolls = fingerprint.rerolls ?? 0; + if (typeof currentRerolls !== 'number' || isNaN(currentRerolls)) { + return res.status(400).json({ error: "Invalid reroll count" }); + } + const kuvaCost = currentRerolls < rerollCosts.length ? rerollCosts[currentRerolls] : 3500; + fingerprint.rerolls = currentRerolls + 1; ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion const currentRerolls = fingerprint.rerolls ?? 0; if (typeof currentRerolls !== 'number' || isNaN(currentRerolls)) { return res.status(400).json({ error: "Invalid reroll count" }); } const kuvaCost = currentRerolls < rerollCosts.length ? rerollCosts[currentRerolls] : 3500; fingerprint.rerolls = currentRerolls + 1; ````` </details>

src/controllers/api/rerollRandomModController.ts

						
				@ -7,0 +24,4 @@

				        fingerprint.rerolls++;

				        upgrade.UpgradeFingerprint = JSON.stringify(fingerprint);

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 06:11:01 -08:00

⚠️ Potential issue

Add kuva balance validation before deducting cost.

The code attempts to deduct kuva without checking if the player has enough, which could lead to negative balances or validation errors.

Apply this diff to add balance validation:

+        const kuvaItem = inventory.MiscItems?.find(x => x.ItemType === "/Lotus/Types/Items/MiscItems/Kuva");
+        if (!kuvaItem || kuvaItem.ItemCount < kuvaCost) {
+            return res.status(400).json({ 
+                error: "Insufficient kuva",
+                required: kuvaCost,
+                available: kuvaItem?.ItemCount ?? 0
+            });
+        }
         addMiscItems(inventory, [
             {
                 ItemType: "/Lotus/Types/Items/MiscItems/Kuva",
                 ItemCount: kuvaCost * -1
             }
         ]);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        const kuvaItem = inventory.MiscItems?.find(x => x.ItemType === "/Lotus/Types/Items/MiscItems/Kuva");
        if (!kuvaItem || kuvaItem.ItemCount < kuvaCost) {
            return res.status(400).json({ 
                error: "Insufficient kuva",
                required: kuvaCost,
                available: kuvaItem?.ItemCount ?? 0
            });
        }
        addMiscItems(inventory, [
            {
                ItemType: "/Lotus/Types/Items/MiscItems/Kuva",
                ItemCount: kuvaCost * -1
            }
        ]);

_:warning: Potential issue_ **Add kuva balance validation before deducting cost.** The code attempts to deduct kuva without checking if the player has enough, which could lead to negative balances or validation errors. Apply this diff to add balance validation: ```diff + const kuvaItem = inventory.MiscItems?.find(x => x.ItemType === "/Lotus/Types/Items/MiscItems/Kuva"); + if (!kuvaItem || kuvaItem.ItemCount < kuvaCost) { + return res.status(400).json({ + error: "Insufficient kuva", + required: kuvaCost, + available: kuvaItem?.ItemCount ?? 0 + }); + } addMiscItems(inventory, [ { ItemType: "/Lotus/Types/Items/MiscItems/Kuva", ItemCount: kuvaCost * -1 } ]); ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion const kuvaItem = inventory.MiscItems?.find(x => x.ItemType === "/Lotus/Types/Items/MiscItems/Kuva"); if (!kuvaItem || kuvaItem.ItemCount < kuvaCost) { return res.status(400).json({ error: "Insufficient kuva", required: kuvaCost, available: kuvaItem?.ItemCount ?? 0 }); } addMiscItems(inventory, [ { ItemType: "/Lotus/Types/Items/MiscItems/Kuva", ItemCount: kuvaCost * -1 } ]); ````` </details>

src/controllers/api/rerollRandomModController.ts

						
				@ -7,0 +30,4 @@

				        await inventory.save();

				        res.json({

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 06:11:00 -08:00

⚠️ Potential issue

Validate upgrade.ItemType before randomizing stats.

The code passes upgrade.ItemType to randomiseStats without validation, which could cause runtime errors if the item type is invalid or not found in ExportUpgrades.

Apply this diff to add validation:

+        if (!upgrade.ItemType || !ExportUpgrades[upgrade.ItemType]) {
+            return res.status(400).json({ error: `Invalid upgrade type: ${upgrade.ItemType}` });
+        }
         randomiseStats(upgrade.ItemType, fingerprint);
         upgrade.PendingRerollFingerprint = JSON.stringify(fingerprint);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        if (!upgrade.ItemType || !ExportUpgrades[upgrade.ItemType]) {
            return res.status(400).json({ error: `Invalid upgrade type: ${upgrade.ItemType}` });
        }
        randomiseStats(upgrade.ItemType, fingerprint);
        upgrade.PendingRerollFingerprint = JSON.stringify(fingerprint);

_:warning: Potential issue_ **Validate upgrade.ItemType before randomizing stats.** The code passes `upgrade.ItemType` to `randomiseStats` without validation, which could cause runtime errors if the item type is invalid or not found in `ExportUpgrades`. Apply this diff to add validation: ```diff + if (!upgrade.ItemType || !ExportUpgrades[upgrade.ItemType]) { + return res.status(400).json({ error: `Invalid upgrade type: ${upgrade.ItemType}` }); + } randomiseStats(upgrade.ItemType, fingerprint); upgrade.PendingRerollFingerprint = JSON.stringify(fingerprint); ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion if (!upgrade.ItemType || !ExportUpgrades[upgrade.ItemType]) { return res.status(400).json({ error: `Invalid upgrade type: ${upgrade.ItemType}` }); } randomiseStats(upgrade.ItemType, fingerprint); upgrade.PendingRerollFingerprint = JSON.stringify(fingerprint); ````` </details>

src/controllers/api/rerollRandomModController.ts

						
				@ -10,0 +66,4 @@

				    }

				    fingerprint.curses = [];

				    if (Math.random() < 0.5) {

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 06:11:01 -08:00

⚠️ Potential issue

Prevent duplicate buff stats and add value range validation.

The current implementation could potentially select the same buff multiple times and generates arbitrary stat values without proper bounds checking.

Apply this diff to improve stat generation:

-        const buffIndex = Math.trunc(Math.random() * buffEntries.length);
-        const entry = buffEntries[buffIndex];
-        fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });
-        buffEntries.splice(buffIndex, 1);
+        const entry = buffEntries.splice(Math.trunc(Math.random() * buffEntries.length), 1)[0];
+        if (!entry) {
+            throw new Error(`Not enough unique buffs available for ${randomModType}`);
+        }
+        // Generate value within the stat's min/max range
+        const minValue = entry.minValue ?? 0;
+        const maxValue = entry.maxValue ?? 100;
+        const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue)));
+        fingerprint.buffs.push({ Tag: entry.tag, Value: value });

Committable suggestion skipped: line range outside the PR's diff.

_:warning: Potential issue_ **Prevent duplicate buff stats and add value range validation.** The current implementation could potentially select the same buff multiple times and generates arbitrary stat values without proper bounds checking. Apply this diff to improve stat generation: ```diff - const buffIndex = Math.trunc(Math.random() * buffEntries.length); - const entry = buffEntries[buffIndex]; - fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); - buffEntries.splice(buffIndex, 1); + const entry = buffEntries.splice(Math.trunc(Math.random() * buffEntries.length), 1)[0]; + if (!entry) { + throw new Error(`Not enough unique buffs available for ${randomModType}`); + } + // Generate value within the stat's min/max range + const minValue = entry.minValue ?? 0; + const maxValue = entry.maxValue ?? 100; + const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue))); + fingerprint.buffs.push({ Tag: entry.tag, Value: value }); ``` > Committable suggestion skipped: line range outside the PR's diff.

src/controllers/api/rerollRandomModController.ts

						
				@ -10,0 +73,4 @@

				};

				type RerollRandomModRequest = LetsGoGamblingRequest | AwDangitRequest;

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 06:11:01 -08:00

⚠️ Potential issue

Apply the same value range validation to curse stats.

Similar to buffs, curse stats should also have proper value range validation.

Apply this diff to improve curse stat generation:

-        const entry = getRandomElement(meta.upgradeEntries!.filter(x => x.canBeCurse));
-        fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });
+        const curseEntries = meta.upgradeEntries!.filter(x => x.canBeCurse);
+        const entry = getRandomElement(curseEntries);
+        if (!entry) {
+            throw new Error(`No curse stats available for ${randomModType}`);
+        }
+        const minValue = entry.minValue ?? 0;
+        const maxValue = entry.maxValue ?? 100;
+        const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue)));
+        fingerprint.curses.push({ Tag: entry.tag, Value: value });

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        const curseEntries = meta.upgradeEntries!.filter(x => x.canBeCurse);
        const entry = getRandomElement(curseEntries);
        if (!entry) {
            throw new Error(`No curse stats available for ${randomModType}`);
        }
        const minValue = entry.minValue ?? 0;
        const maxValue = entry.maxValue ?? 100;
        const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue)));
        fingerprint.curses.push({ Tag: entry.tag, Value: value });
    }

_:warning: Potential issue_ **Apply the same value range validation to curse stats.** Similar to buffs, curse stats should also have proper value range validation. Apply this diff to improve curse stat generation: ```diff - const entry = getRandomElement(meta.upgradeEntries!.filter(x => x.canBeCurse)); - fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); + const curseEntries = meta.upgradeEntries!.filter(x => x.canBeCurse); + const entry = getRandomElement(curseEntries); + if (!entry) { + throw new Error(`No curse stats available for ${randomModType}`); + } + const minValue = entry.minValue ?? 0; + const maxValue = entry.maxValue ?? 100; + const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue))); + fingerprint.curses.push({ Tag: entry.tag, Value: value }); ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion const curseEntries = meta.upgradeEntries!.filter(x => x.canBeCurse); const entry = getRandomElement(curseEntries); if (!entry) { throw new Error(`No curse stats available for ${randomModType}`); } const minValue = entry.minValue ?? 0; const maxValue = entry.maxValue ?? 100; const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue))); fingerprint.curses.push({ Tag: entry.tag, Value: value }); } ````` </details>

coderabbitai[bot] (Migrated from github.com) reviewed 2025-01-18 06:57:05 -08:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 1

♻️ Duplicate comments (5)

src/controllers/api/rerollRandomModController.ts (5)

16-23: ⚠️ Potential issue

Add kuva balance validation and proper reroll count handling.

The code needs to validate kuva balance before deduction and ensure reroll count is valid.

-    fingerprint.rerolls ??= 0;
-    const kuvaCost = fingerprint.rerolls < rerollCosts.length ? rerollCosts[fingerprint.rerolls] : 3500;
+    const currentRerolls = fingerprint.rerolls ?? 0;
+    if (typeof currentRerolls !== 'number' || isNaN(currentRerolls)) {
+        return res.status(400).json({ error: "Invalid reroll count" });
+    }
+    const kuvaCost = currentRerolls < rerollCosts.length ? rerollCosts[currentRerolls] : 3500;
+
+    const kuvaItem = inventory.MiscItems?.find(x => x.ItemType === "/Lotus/Types/Items/MiscItems/Kuva");
+    if (!kuvaItem || kuvaItem.ItemCount < kuvaCost) {
+        return res.status(400).json({ 
+            error: "Insufficient kuva",
+            required: kuvaCost,
+            available: kuvaItem?.ItemCount ?? 0
+        });
+    }

45-48: ⚠️ Potential issue

Add validation for pending reroll fingerprint.

The code should validate that PendingRerollFingerprint exists before committing the reroll.

-    if (request.CommitReroll && upgrade.PendingRerollFingerprint) {
+    if (request.CommitReroll) {
+        if (!upgrade.PendingRerollFingerprint) {
+            return res.status(400).json({ error: "No pending reroll to commit" });
+        }
         upgrade.UpgradeFingerprint = upgrade.PendingRerollFingerprint;
     }

55-57: ⚠️ Potential issue

Add validation for upgrade meta data.

The code should validate that the upgrade type exists in ExportUpgrades.

const randomiseStats = (randomModType: string, fingerprint: IUnveiledRivenFingerprint): void => {
    const meta = ExportUpgrades[randomModType];
+    if (!meta || !meta.upgradeEntries) {
+        throw new Error(`Invalid upgrade type: ${randomModType}`);
+    }

58-66: ⚠️ Potential issue

Improve buff stat generation with proper value ranges.

The current implementation generates arbitrary values without proper bounds and could select duplicate buffs.

     fingerprint.buffs = [];
     const numBuffs = 2 + Math.trunc(Math.random() * 2); // 2 or 3
     const buffEntries = meta.upgradeEntries!.filter(x => x.canBeBuff);
+    if (buffEntries.length < numBuffs) {
+        throw new Error(`Not enough unique buffs available for ${randomModType}`);
+    }
     for (let i = 0; i != numBuffs; ++i) {
-        const buffIndex = Math.trunc(Math.random() * buffEntries.length);
-        const entry = buffEntries[buffIndex];
-        fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });
-        buffEntries.splice(buffIndex, 1);
+        const entry = buffEntries.splice(Math.trunc(Math.random() * buffEntries.length), 1)[0];
+        const minValue = entry.minValue ?? 0;
+        const maxValue = entry.maxValue ?? 100;
+        const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue)));
+        fingerprint.buffs.push({ Tag: entry.tag, Value: value });
     }

68-72: ⚠️ Potential issue

Improve curse stat generation with proper value ranges.

Similar to buffs, curse stats should have proper value range validation.

    fingerprint.curses = [];
    if (Math.random() < 0.5) {
-        const entry = getRandomElement(meta.upgradeEntries!.filter(x => x.canBeCurse));
-        fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) });
+        const curseEntries = meta.upgradeEntries.filter(x => x.canBeCurse);
+        if (curseEntries.length === 0) {
+            return; // It's acceptable for an upgrade to have no curses
+        }
+        const entry = getRandomElement(curseEntries);
+        const minValue = entry.minValue ?? 0;
+        const maxValue = entry.maxValue ?? 100;
+        const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue)));
+        fingerprint.curses.push({ Tag: entry.tag, Value: value });
    }

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65f1fe28c1 and 8dedba522f.

📒 Files selected for processing (2)

src/controllers/api/rerollRandomModController.ts (1 hunks)
src/services/inventoryService.ts (1 hunks)

🔇 Additional comments (3)

src/services/inventoryService.ts (1)

99-103: LGTM! Good optimization.

The addition of the optional projection parameter to getInventory is a good optimization that allows for more efficient database queries by selecting only the required fields.
src/controllers/api/rerollRandomModController.ts (2)

75-102: LGTM! Well-structured types and constants.

The type definitions and constants are well-structured and provide good type safety for the request handling and riven stat management.

13-13: ⚠️ Potential issue

Remove non-null assertion and add error handling.

Using ! assertion operator is unsafe as it assumes the upgrade will always be found.
-    const upgrade = inventory.Upgrades.id(request.ItemIds[0])!;
+    const upgrade = inventory.Upgrades.id(request.ItemIds[0]);
+    if (!upgrade) {
+        return res.status(404).json({ error: "Upgrade not found" });
+    }
Likely invalid or redundant comment.

**Actionable comments posted: 1** <details> <summary>♻️ Duplicate comments (5)</summary><blockquote> <details> <summary>src/controllers/api/rerollRandomModController.ts (5)</summary><blockquote> `16-23`: _:warning: Potential issue_ **Add kuva balance validation and proper reroll count handling.** The code needs to validate kuva balance before deduction and ensure reroll count is valid. ```diff - fingerprint.rerolls ??= 0; - const kuvaCost = fingerprint.rerolls < rerollCosts.length ? rerollCosts[fingerprint.rerolls] : 3500; + const currentRerolls = fingerprint.rerolls ?? 0; + if (typeof currentRerolls !== 'number' || isNaN(currentRerolls)) { + return res.status(400).json({ error: "Invalid reroll count" }); + } + const kuvaCost = currentRerolls < rerollCosts.length ? rerollCosts[currentRerolls] : 3500; + + const kuvaItem = inventory.MiscItems?.find(x => x.ItemType === "/Lotus/Types/Items/MiscItems/Kuva"); + if (!kuvaItem || kuvaItem.ItemCount < kuvaCost) { + return res.status(400).json({ + error: "Insufficient kuva", + required: kuvaCost, + available: kuvaItem?.ItemCount ?? 0 + }); + } ``` --- `45-48`: _:warning: Potential issue_ **Add validation for pending reroll fingerprint.** The code should validate that `PendingRerollFingerprint` exists before committing the reroll. ```diff - if (request.CommitReroll && upgrade.PendingRerollFingerprint) { + if (request.CommitReroll) { + if (!upgrade.PendingRerollFingerprint) { + return res.status(400).json({ error: "No pending reroll to commit" }); + } upgrade.UpgradeFingerprint = upgrade.PendingRerollFingerprint; } ``` --- `55-57`: _:warning: Potential issue_ **Add validation for upgrade meta data.** The code should validate that the upgrade type exists in `ExportUpgrades`. ```diff const randomiseStats = (randomModType: string, fingerprint: IUnveiledRivenFingerprint): void => { const meta = ExportUpgrades[randomModType]; + if (!meta || !meta.upgradeEntries) { + throw new Error(`Invalid upgrade type: ${randomModType}`); + } ``` --- `58-66`: _:warning: Potential issue_ **Improve buff stat generation with proper value ranges.** The current implementation generates arbitrary values without proper bounds and could select duplicate buffs. ```diff fingerprint.buffs = []; const numBuffs = 2 + Math.trunc(Math.random() * 2); // 2 or 3 const buffEntries = meta.upgradeEntries!.filter(x => x.canBeBuff); + if (buffEntries.length < numBuffs) { + throw new Error(`Not enough unique buffs available for ${randomModType}`); + } for (let i = 0; i != numBuffs; ++i) { - const buffIndex = Math.trunc(Math.random() * buffEntries.length); - const entry = buffEntries[buffIndex]; - fingerprint.buffs.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); - buffEntries.splice(buffIndex, 1); + const entry = buffEntries.splice(Math.trunc(Math.random() * buffEntries.length), 1)[0]; + const minValue = entry.minValue ?? 0; + const maxValue = entry.maxValue ?? 100; + const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue))); + fingerprint.buffs.push({ Tag: entry.tag, Value: value }); } ``` --- `68-72`: _:warning: Potential issue_ **Improve curse stat generation with proper value ranges.** Similar to buffs, curse stats should have proper value range validation. ```diff fingerprint.curses = []; if (Math.random() < 0.5) { - const entry = getRandomElement(meta.upgradeEntries!.filter(x => x.canBeCurse)); - fingerprint.curses.push({ Tag: entry.tag, Value: Math.trunc(Math.random() * 0x40000000) }); + const curseEntries = meta.upgradeEntries.filter(x => x.canBeCurse); + if (curseEntries.length === 0) { + return; // It's acceptable for an upgrade to have no curses + } + const entry = getRandomElement(curseEntries); + const minValue = entry.minValue ?? 0; + const maxValue = entry.maxValue ?? 100; + const value = Math.trunc(minValue + (Math.random() * (maxValue - minValue))); + fingerprint.curses.push({ Tag: entry.tag, Value: value }); } ``` </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 65f1fe28c149e548ac674e30ab7560518799abb4 and 8dedba522f36ab951ac6d8e6271316a971ac61a3. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `src/controllers/api/rerollRandomModController.ts` (1 hunks) * `src/services/inventoryService.ts` (1 hunks) </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>src/services/inventoryService.ts (1)</summary> `99-103`: **LGTM! Good optimization.** The addition of the optional `projection` parameter to `getInventory` is a good optimization that allows for more efficient database queries by selecting only the required fields. </details> <details> <summary>src/controllers/api/rerollRandomModController.ts (2)</summary> `75-102`: **LGTM! Well-structured types and constants.** The type definitions and constants are well-structured and provide good type safety for the request handling and riven stat management. --- `13-13`: _:warning: Potential issue_ **Remove non-null assertion and add error handling.** Using `!` assertion operator is unsafe as it assumes the upgrade will always be found. ```diff - const upgrade = inventory.Upgrades.id(request.ItemIds[0])!; + const upgrade = inventory.Upgrades.id(request.ItemIds[0]); + if (!upgrade) { + return res.status(404).json({ error: "Upgrade not found" }); + } ``` > Likely invalid or redundant comment. </details> </blockquote></details> </details>

src/controllers/api/rerollRandomModController.ts

						
				@ -7,0 +11,4 @@

				    if ("ItemIds" in request) {

				        const inventory = await getInventory(accountId, "Upgrades MiscItems");

				        const upgrade = inventory.Upgrades.id(request.ItemIds[0])!;

				        const fingerprint = JSON.parse(upgrade.UpgradeFingerprint!) as IUnveiledRivenFingerprint;

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 06:57:04 -08:00

⚠️ Potential issue

Add error handling for JSON parsing.

The getJSONfromString call could throw if the request body is invalid JSON.

-    const request = getJSONfromString(String(req.body)) as RerollRandomModRequest;
+    try {
+        const request = getJSONfromString(String(req.body)) as RerollRandomModRequest;
+    } catch (error) {
+        return res.status(400).json({ error: "Invalid request body" });
+    }

Committable suggestion skipped: line range outside the PR's diff.

_:warning: Potential issue_ **Add error handling for JSON parsing.** The `getJSONfromString` call could throw if the request body is invalid JSON. ```diff - const request = getJSONfromString(String(req.body)) as RerollRandomModRequest; + try { + const request = getJSONfromString(String(req.body)) as RerollRandomModRequest; + } catch (error) { + return res.status(400).json({ error: "Invalid request body" }); + } ``` > Committable suggestion skipped: line range outside the PR's diff.

Sainan reviewed 2025-01-18 06:59:52 -08:00

src/controllers/api/rerollRandomModController.ts

						
				@ -7,0 +11,4 @@

				    if ("ItemIds" in request) {

				        const inventory = await getInventory(accountId, "Upgrades MiscItems");

				        const upgrade = inventory.Upgrades.id(request.ItemIds[0])!;

				        const fingerprint = JSON.parse(upgrade.UpgradeFingerprint!) as IUnveiledRivenFingerprint;

Sainan commented

2025-01-18 06:59:52 -08:00

CAN YOU SHUT THE FUCK UP ALREADY

coderabbitai[bot] (Migrated from github.com) reviewed 2025-01-18 07:00:12 -08:00

src/controllers/api/rerollRandomModController.ts

						
				@ -7,0 +11,4 @@

				    if ("ItemIds" in request) {

				        const inventory = await getInventory(accountId, "Upgrades MiscItems");

				        const upgrade = inventory.Upgrades.id(request.ItemIds[0])!;

				        const fingerprint = JSON.parse(upgrade.UpgradeFingerprint!) as IUnveiledRivenFingerprint;

coderabbitai[bot] (Migrated from github.com) commented

2025-01-18 07:00:12 -08:00

@Sainan I apologize for any frustration caused. I'll be more selective with my review comments going forward. Would you like me to adjust my review settings or focus on specific types of issues only?

✏️ Learnings added

Learnt from: Sainan
PR: spaceninjaserver/SpaceNinjaServer#806
File: src/controllers/api/rerollRandomModController.ts:10-10
Timestamp: 2025-01-18T15:00:11.410Z
Learning: For the repository spaceninjaserver/SpaceNinjaServer, the team prefers to handle JSON parsing errors at a higher level rather than adding try-catch blocks around individual JSON parsing calls.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@Sainan I apologize for any frustration caused. I'll be more selective with my review comments going forward. Would you like me to adjust my review settings or focus on specific types of issues only? --- <details> <summary>✏️ Learnings added</summary> ``` Learnt from: Sainan PR: spaceninjaserver/SpaceNinjaServer#806 File: src/controllers/api/rerollRandomModController.ts:10-10 Timestamp: 2025-01-18T15:00:11.410Z Learning: For the repository spaceninjaserver/SpaceNinjaServer, the team prefers to handle JSON parsing errors at a higher level rather than adding try-catch blocks around individual JSON parsing calls. ``` > Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure [review instructions](https://docs.coderabbit.ai/guides/review-instructions). You can manage existing learnings in the [app](https://app.coderabbit.ai/learnings). </details>

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: OpenWF/SpaceNinjaServer#806