OpenWF/SpaceNinjaServer
12
6
Fork 15
Code Issues 46 Pull Requests 6 Actions Packages Projects Releases Activity

fix: removing an archon shard doesn't refund it #729

Merged
Sainan merged 1 commits from shard-refund into main 2025-01-05 16:21:02 -08:00
Conversation 2 Commits 1 Files Changed 1 +19
1 changed files with 19 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/controllers/api/infestedFoundryController.ts
View File

@ -52,14 +52,33 @@ export const infestedFoundryController: RequestHandler = async (req, res) => {
const request = getJSONfromString(String(req.body)) as IShardUninstallRequest; const request = getJSONfromString(String(req.body)) as IShardUninstallRequest;
const inventory = await getInventory(accountId); const inventory = await getInventory(accountId);
const suit = inventory.Suits.find(suit => suit._id.toString() == request.SuitId.$oid)!; const suit = inventory.Suits.find(suit => suit._id.toString() == request.SuitId.$oid)!;
// refund shard
const shard = Object.entries(colorToShard).find(
([color]) => color == suit.ArchonCrystalUpgrades![request.Slot].Color
)![1];
coderabbitai[bot] commented 2025-01-05 14:38:39 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Validate that a matching shard color is always found
If find() returns undefined (due to an unexpected color string), the ! non-null assertion may cause a runtime error. Consider adding a fallback or error handling to avoid potential crashes.

 const shardEntry = Object.entries(colorToShard).find(([color]) => 
   color === suit.ArchonCrystalUpgrades![request.Slot].Color
 );
-if (!shardEntry) throw new Error("Invalid archon crystal color");
-const shard = shardEntry[1];
+if (!shardEntry) {
+  // handle invalid color scenario
+  return res.status(400).json({ error: "Invalid archon crystal color" });
+}
+const shard = shardEntry[1];

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

_:warning: Potential issue_ **Validate that a matching shard color is always found** If `find()` returns `undefined` (due to an unexpected color string), the `!` non-null assertion may cause a runtime error. Consider adding a fallback or error handling to avoid potential crashes. ```diff const shardEntry = Object.entries(colorToShard).find(([color]) => color === suit.ArchonCrystalUpgrades![request.Slot].Color ); -if (!shardEntry) throw new Error("Invalid archon crystal color"); -const shard = shardEntry[1]; +if (!shardEntry) { + // handle invalid color scenario + return res.status(400).json({ error: "Invalid archon crystal color" }); +} +const shard = shardEntry[1]; ``` > Committable suggestion skipped: line range outside the PR's diff. <!-- This is an auto-generated comment by CodeRabbit -->
const miscItemChanges = [
{
ItemType: shard,
ItemCount: 1
}
];
addMiscItems(inventory, miscItemChanges);
// remove from suit
suit.ArchonCrystalUpgrades![request.Slot] = {}; suit.ArchonCrystalUpgrades![request.Slot] = {};
// remove bile
const bile = inventory.InfestedFoundry!.Resources!.find( const bile = inventory.InfestedFoundry!.Resources!.find(
x => x.ItemType == "/Lotus/Types/Items/InfestedFoundry/HelminthBile" x => x.ItemType == "/Lotus/Types/Items/InfestedFoundry/HelminthBile"
)!; )!;
bile.Count -= 300; bile.Count -= 300;
coderabbitai[bot] commented 2025-01-05 14:38:38 -08:00 (Migrated from github.com)
Review
Copy Link

🛠️ Refactor suggestion

Avoid negative resource counts
Subtracting 300 from bile.Count without checking if there's enough HelminthBile could produce negative counts. This might lead to inconsistencies in the user’s resource inventory.

- bile.Count -= 300;
+ if (bile.Count < 300) {
+   bile.Count = 0; 
+   // or handle insufficient HelminthBile scenario
+ } else {
+   bile.Count -= 300;
+ }
📝 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.

            // remove bile
            const bile = inventory.InfestedFoundry!.Resources!.find(
                x => x.ItemType == "/Lotus/Types/Items/InfestedFoundry/HelminthBile"
            )!;
            if (bile.Count < 300) {
                bile.Count = 0;
                // or handle insufficient HelminthBile scenario
            } else {
                bile.Count -= 300;
            }
_:hammer_and_wrench: Refactor suggestion_ **Avoid negative resource counts** Subtracting 300 from `bile.Count` without checking if there's enough HelminthBile could produce negative counts. This might lead to inconsistencies in the user’s resource inventory. ```diff - bile.Count -= 300; + if (bile.Count < 300) { + bile.Count = 0; + // or handle insufficient HelminthBile scenario + } else { + bile.Count -= 300; + } ``` <!-- suggestion_start --> <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 // remove bile const bile = inventory.InfestedFoundry!.Resources!.find( x => x.ItemType == "/Lotus/Types/Items/InfestedFoundry/HelminthBile" )!; if (bile.Count < 300) { bile.Count = 0; // or handle insufficient HelminthBile scenario } else { bile.Count -= 300; } ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
await inventory.save(); await inventory.save();
res.json({ res.json({
InventoryChanges: { InventoryChanges: {
MiscItems: miscItemChanges,
InfestedFoundry: inventory.toJSON().InfestedFoundry InfestedFoundry: inventory.toJSON().InfestedFoundry
} }
}); });