-
Notifications
You must be signed in to change notification settings - Fork 60
Embezzler lucky #2367
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
base: main
Are you sure you want to change the base?
Embezzler lucky #2367
Conversation
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.
Great first step. I think this will be a good value-add
packages/garbo/src/outfit/target.ts
Outdated
@@ -44,6 +44,9 @@ export function meatTargetOutfit( | |||
if (target === $location`Crab Island`) { | |||
const meat = meatDrop($monster`giant giant crab`) + songboomMeat(); | |||
outfit.modifier.push(`${meat / 100} Meat Drop`, "-tie"); | |||
} else if (target === $location`Cobb's Knob Treasury`) { |
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.
unfortunately I think we need a more precise check for this given that wanderers can be placed in the treasury (they can't be placed in crab island)
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.
Added a check for Lucky!, hopefully that should work
packages/garbo/src/tasks/barfTurn.ts
Outdated
do: () => getBestLuckyAdventure().location, | ||
prepare: () => { | ||
if (!have($effect`Lucky!`)) { | ||
retrieveItem($item`Eight Days a Week Pill Keeper`); | ||
cliExecute("pillkeeper semirare"); | ||
if (!have($effect`Lucky!`)) { | ||
set("_freePillKeeperUsed", true); | ||
return; | ||
} | ||
} | ||
}, |
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 we should use a system similar to the one you've set up in embezzler.ts and for similar reasons (that is, separate lucky source from lucky execution).
BUT, the turns
field should remain on the sources rather than on the lucky adventure task.
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.
Does that cause any problems with the task itself having spendsTurn: false,
but actually having a turns
field?
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.
Did this and also added counting the embezzler sources in copyTargetCount
packages/garbo/src/target/fights.ts
Outdated
@@ -1098,8 +1015,9 @@ export const copyTargetSources = [ | |||
]; | |||
|
|||
export function copyTargetCount(): number { |
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.
Is this worth having two separate functions (or a boolean arg) for? is everywhere we call copyTargetCount
a place where we want to include embezzlers? even if you're fighting, say, roaches?
b07c3c2
to
40b2500
Compare
@@ -60,18 +59,15 @@ function drivebyValue(): number { | |||
} | |||
|
|||
function entendreValue(): number { | |||
const targets = copyTargetCount(); | |||
const targets = highMeatMonsterCount("Scepter"); // Scepter can cause circular logic |
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.
could you do a quick refactor to both have highmeatMonsterCount
passed in rather than calculated both times, and to deduplicate some of the logic here?
Problem I'm unsure how to fix: We need to count how many embezzlers we're fighting here to account for them when buffing with potions (I don't think we should actually account for them cost wise, but we are spending turns fighting them, so that might cause us to run out of turns of buffs before we finish all our copiers, so the copytargetcount might need updating to account for that?