Skip to content
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

Merged
merged 6 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/lib/types/minions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export interface FishingActivityTaskOptions extends ActivityTaskOptions {
type: 'Fishing';
fishID: number;
quantity: number;
flakesQuantity?: number;
iQty?: number;
}

Expand Down
6 changes: 5 additions & 1 deletion src/lib/util/repeatStoredTrip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,11 @@ const tripHandlers = {
},
[activity_type_enum.Fishing]: {
commandName: 'fish',
args: (data: FishingActivityTaskOptions) => ({ name: data.fishID, quantity: data.iQty })
args: (data: FishingActivityTaskOptions) => ({
name: data.fishID,
quantity: data.iQty,
flakes: data.flakesQuantity !== undefined
})
},
[activity_type_enum.FishingTrawler]: {
commandName: 'minigames',
Expand Down
6 changes: 6 additions & 0 deletions src/lib/util/smallUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ export function pluraliseItemName(name: string): string {
return name + (name.endsWith('s') ? '' : 's');
}

export function pluraliseItemNameWithQuantity(name: string, quantity: number): string {
const result = `${quantity} ${name}`;
if (quantity === 1) return result;
return pluraliseItemName(result);
}
Copy link
Collaborator

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.

Copy link
Contributor Author

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. Consider name is "pike" and quantity is 5.

  1. result === "5 pike"
  2. pluraliseItemName("5 pike") is called
  3. return "5 pike" + "s" => "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 of

  • k (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)) and
  • fish (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?

Copy link
Collaborator

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

Copy link
Contributor Author

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?


export function shuffleRandom<T>(input: number, arr: readonly T[]): T[] {
const engine = MersenneTwister19937.seed(input);
return shuffle(engine, [...arr]);
Expand Down
47 changes: 38 additions & 9 deletions src/mahoji/commands/fish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless im misunderstanding, this variable isnt needed, you can just do if (cost.length > 0) to check if there any items in the cost bank.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Expand All @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions src/tasks/minions/fishingActivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const fishingTask: MinionTask = {
}),
async run(data: FishingActivityTaskOptions) {
const { fishID, quantity, userID, channelID, duration } = data;
let { flakesQuantity } = data;
const user = await mUserFetch(userID);
const currentLevel = user.skillLevel(SkillsEnum.Fishing);
const { blessingEquipped, blessingChance } = radasBlessing(user);
Expand Down Expand Up @@ -160,6 +161,11 @@ export const fishingTask: MinionTask = {
} else {
lootQuantity += blessingEquipped && percentChance(blessingChance) ? 2 : 1;
}

if (flakesQuantity && flakesQuantity > 0) {
lootQuantity += percentChance(50) ? 1 : 0;
flakesQuantity--;
}
}

const loot = new Bank({
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/commands/fish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,41 @@ describe('Fish Command', () => {
**Boosts:** +9 trip minutes for having a Fish sack barrel.`
});
});

it('should handle using flakes without flakes in bank', () => {
testRunCmd({
cmd: fishCommand,
opts: { name: 'shrimps', flakes: true },
user: {
skills_fishing: 999_999
},
result: 'You need to have at least one spirit flake!'
});
});

it('should fish with flakes', () => {
testRunCmd({
cmd: fishCommand,
opts: { name: 'shrimps', flakes: true },
user: {
bank: new Bank({ 'Spirit flakes': 10000 })
},
result: `<:minion:778418736180494347> Your minion is now fishing 251x Shrimps, it'll take around 29 minutes, 58 seconds to finish.

**Boosts:** More fish from using 251 spirit flakes.`
});
});

it('should still use flakes if bank contains fewer flakes than fish quantity', () => {
testRunCmd({
cmd: fishCommand,
opts: { name: 'shrimps', flakes: true },
user: {
bank: new Bank({ 'Spirit flakes': 100 })
},
result: `<:minion:778418736180494347> Your minion is now fishing 251x Shrimps, it'll take around 29 minutes, 58 seconds to finish.

**Boosts:** More fish from using 100 spirit flakes.`
});
});
});
16 changes: 15 additions & 1 deletion tests/unit/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import { baseModifyBusyCounter } from '../../src/lib/busyCounterCache';
import { deduplicateClueScrolls } from '../../src/lib/clues/clueUtils';
import getUserFoodFromBank from '../../src/lib/minions/functions/getUserFoodFromBank';
import { SkillsEnum } from '../../src/lib/skilling/types';
import { pluraliseItemName, sanitizeBank, skillingPetDropRate, stripEmojis } from '../../src/lib/util';
import {
pluraliseItemName,
pluraliseItemNameWithQuantity,
sanitizeBank,
skillingPetDropRate,
stripEmojis
} from '../../src/lib/util';
import getOSItem from '../../src/lib/util/getOSItem';
import { sellPriceOfItem, sellStorePriceOfItem } from '../../src/mahoji/commands/sell';
import { mockMUser } from './utils';
Expand Down Expand Up @@ -142,4 +148,12 @@ describe('util', () => {
expect(pluraliseItemName('Steel Arrowtips')).toEqual('Steel Arrowtips');
expect(pluraliseItemName('Adamantite nails')).toEqual('Adamantite nails');
});

test('pluraliseItemWithQuantity correctly pluralises items', async () => {
expect(pluraliseItemNameWithQuantity('Shrimp', 0)).toEqual('0 Shrimps');
expect(pluraliseItemNameWithQuantity('Twisted bow', 999)).toEqual('999 Twisted bows');
expect(pluraliseItemNameWithQuantity('Bronze Arrowtips', 0)).toEqual('0 Bronze Arrowtips');
expect(pluraliseItemNameWithQuantity('Spirit flakes', 10)).toEqual('10 Spirit flakes');
expect(pluraliseItemNameWithQuantity('Yellow square', 1)).toEqual('1 Yellow square');
});
});