-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add spirit flakes option to fish command #5948
Changes from 2 commits
cacb62d
affcaef
683601e
2a4bf97
5f311ab
a9ad785
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ import TzTokJad from 'oldschooljs/dist/simulation/monsters/special/TzTokJad'; | |
import Fishing from '../../lib/skilling/skills/fishing'; | ||
import { SkillsEnum } from '../../lib/skilling/types'; | ||
import type { FishingActivityTaskOptions } from '../../lib/types/minions'; | ||
import { formatDuration, itemID, itemNameFromID } from '../../lib/util'; | ||
import { formatDuration, itemID, itemNameFromID, pluraliseItemNameWithQuantity } from '../../lib/util'; | ||
import addSubTaskToActivityTask from '../../lib/util/addSubTaskToActivityTask'; | ||
import { calcMaxTripLength } from '../../lib/util/calcMaxTripLength'; | ||
import type { OSBMahojiCommand } from '../lib/util'; | ||
|
@@ -42,9 +42,19 @@ export const fishCommand: OSBMahojiCommand = { | |
description: 'The quantity you want to fish (optional).', | ||
required: false, | ||
min_value: 1 | ||
}, | ||
{ | ||
type: ApplicationCommandOptionType.Boolean, | ||
name: 'flakes', | ||
description: 'Use spirit flakes?', | ||
required: false | ||
} | ||
], | ||
run: async ({ options, userID, channelID }: CommandRunOptions<{ name: string; quantity?: number }>) => { | ||
run: async ({ | ||
options, | ||
userID, | ||
channelID | ||
}: CommandRunOptions<{ name: string; quantity?: number; flakes?: boolean }>) => { | ||
const user = await mUserFetch(userID); | ||
const fish = Fishing.Fishes.find( | ||
fish => | ||
|
@@ -132,8 +142,24 @@ export const fishCommand: OSBMahojiCommand = { | |
|
||
const maxTripLength = calcMaxTripLength(user, 'Fishing'); | ||
|
||
let { quantity } = options; | ||
if (!quantity) quantity = Math.floor(maxTripLength / scaledTimePerFish); | ||
let { quantity, flakes } = options; | ||
if (!quantity) { | ||
quantity = Math.floor(maxTripLength / scaledTimePerFish); | ||
} | ||
let shouldRemoveFromBank = false; | ||
let flakesQuantity: number | undefined; | ||
const cost = new Bank(); | ||
|
||
if (flakes) { | ||
if (!user.bank.has('Spirit flakes')) { | ||
return 'You need to have at least one spirit flake!'; | ||
} | ||
|
||
flakesQuantity = Math.min(user.bank.amount('Spirit flakes'), quantity); | ||
boosts.push(`More fish from using ${pluraliseItemNameWithQuantity('spirit flake', flakesQuantity)}`); | ||
shouldRemoveFromBank = true; | ||
cost.add('Spirit flakes', flakesQuantity); | ||
} | ||
|
||
if (fish.bait) { | ||
const baseCost = new Bank().add(fish.bait); | ||
|
@@ -146,11 +172,13 @@ export const fishCommand: OSBMahojiCommand = { | |
quantity = maxCanDo; | ||
} | ||
|
||
const cost = new Bank(); | ||
cost.add(baseCost.multiply(quantity)); | ||
shouldRemoveFromBank = true; | ||
cost.add(fish.bait, quantity); | ||
} | ||
|
||
// Remove the bait from their bank. | ||
await user.removeItemsFromBank(new Bank().add(fish.bait, quantity)); | ||
if (shouldRemoveFromBank) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless im misunderstanding, this variable isnt needed, you can just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, I fixed that. |
||
// Remove the bait and/or spirit flakes from their bank. | ||
await user.removeItemsFromBank(cost); | ||
} | ||
|
||
let duration = quantity * scaledTimePerFish; | ||
|
@@ -173,7 +201,8 @@ export const fishCommand: OSBMahojiCommand = { | |
quantity, | ||
iQty: options.quantity ? options.quantity : undefined, | ||
duration, | ||
type: 'Fishing' | ||
type: 'Fishing', | ||
flakesQuantity | ||
}); | ||
|
||
let response = `${user.minionName} is now fishing ${quantity}x ${fish.name}, it'll take around ${formatDuration( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function seems odd to me, if its only 1, it will include the quantity, but if its anything more than 1, it wont show the quantity? I generally prefer now to just write:
1x Coal, 1x Egg, 5x Trout
, this way theres no pluralization needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping that the unit tests written for this method would make its functionality clear, but I can try to elaborate: Even if
quantity != 1
, it will still be shown. Considername
is"pike"
andquantity
is5
.result
==="5 pike"
pluraliseItemName("5 pike")
is called"5 pikes"
I added this method because I noticed the
1x item
format wasn't consistently used. Few examples I have on hand are the output ofk
(Your minion finished killing 553 Man. Your Man KC is now 553.
),mine
(Your minion finished mining 327 Pure essence. You received 1,635 :mining: XP. (3k/Hr)
) andfish
(Your minion finished fishing 24 Sardine. You received 480 :fishing: XP. (18k/Hr)
)It would facilitate correct English if someone wants to run those outputs through
pluraliseItemNameWithQuanity
, as a future refactoring.I can change the output of
fish
to not use this function. Though, after reading this, do you think this function is still useful? Or should I remove it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using the 1x format is better and we should stick with that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the function itself? Delete?