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 KingGoldemar Sim #6188

Merged
merged 5 commits into from
Nov 21, 2024
Merged

Add KingGoldemar Sim #6188

merged 5 commits into from
Nov 21, 2024

Conversation

nwjgit
Copy link
Contributor

@nwjgit nwjgit commented Nov 14, 2024

Description:

  • Fix King Goldemar sim not showing dwwh in loot image.

Changes:

  • Add custom logic for /kill name: King Goldemar
  • Update calcDwwhChance to be usable inside simulatedKillables so that variables only ever need updated once.
  • Move simulatedKillable before osjsMonster inside kill.worker.ts to always prioritize custom logic first.
  • Update the autofill logic for /kill to prevent monsters like King Goldemar from showing twice.
  • Add a message field to SimulatedKillable interface. *See example image below

Other checks:

  • I didn't notice the simulate() at the bottom of Boss.ts until after making this PR. I'm not really sure where this is used at besides the unit test. Let me know if changing calcDwwhChance() is a no go, or if you want it done another way.
  • I have tested all my changes thoroughly.
    Discord_rAJSXqc6bE

@nwjgit nwjgit added the BSO Bot School Old label Nov 14, 2024
@@ -17,16 +17,16 @@ import type { Gear } from './Gear';
export const gpCostPerKill = (user: MUser) =>
user.gear.melee.hasEquipped(['Ring of charos', 'Ring of charos(a)'], false) ? 5_000_000 : 10_000_000;

export const calcDwwhChance = (users: MUser[]) => {
const size = Math.min(users.length, 10);
export const calcDwwhChance = (amount: number, hasROL: boolean) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The calcDwwhChance function signature has changed, but this call still uses the old signature. Update the call to use the new parameters: calcDwwhChance(bossUsers.length, bossUsers.some(i => i.user.gear.melee.hasEquipped('Ring of luck'))).

if (roll(calcDwwhChance(1, true))) {
loot.add('Broken dwarven warhammer');
}
loot.add(KingGoldemarLootTable.roll());
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be roll(quantity) outside the loop, its a lot more efficient/faster as it doesnt need to create an object for every single iteration. If you must do it in a loop (not in this case here), the other way is: .roll(1, {targetBank: loot})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't necessarily need to be inside the loop, I was just going off how the others are structured. Let me know if this change looks good and or you want me change the others aswell.

@gc gc merged commit 8acbb7e into oldschoolgg:bso Nov 21, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BSO Bot School Old
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants