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 Item Provider #1331

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Add Item Provider #1331

wants to merge 32 commits into from

Conversation

Alium58
Copy link
Member

@Alium58 Alium58 commented Jul 13, 2023

Description

We had item buffing all over the place. Consolidated into a provider

Goal is to make it easier to add future +item buffs. Also eventually to handle 8-bit realm smarter and providers allow for speculation - can determine which zone is best.

Currently only includes the +item buffs I could find already in the code. There are many more sources of +item, but plan is to leave that for a future PR. There is a decent chance I've missed some +item stuff we do, hard to find with how dispersed it was.

How Has This Been Tested?

A gew standard HC and normal runs

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have based my pull request against the main branch or have a good reason not to.

@quarklikeadork
Copy link
Contributor

A reason there wasn't a item% provider yet may be that desired item% is handled by quests and pre adv with different priorities so making a provider with same functionality is more complicated than other providers. Pre adv trying to do routine item buffing to location desired value but limited sources would be kept for something essential like filthworms. Or this is retroactive explanation but auto_zone.ash values seem to be designed for this, zone_needItem() likes up to 2000 item% for the small chance of green ops monster because trying it with routine resources is ok but it's not worth using limited resources.
Example Broken Champagne Bottle 9 charges a day is allowed by several quests scripts instead of any time item% requested, but this is easier to deal with than other limited resources, quests allow it for the same maximizer calls the provider will make.

Is the change on purpose from 50item value of pre adv to 500item in provider? In some locations item% is the only goal but in other locations it may be less important than other values it competes with in maximizer, like The Smut Orc Logging Camp.

@Alium58
Copy link
Member Author

Alium58 commented Jul 17, 2023

Its a good point. Perhaps the doEquips flag should be changed to doEverything flag. When false, just do low cost unlimited skill buffs. When true, do unlimited skills, items, then limited skills to try to reach the goal?

Pre-adv already has doEquips as false where other uses (when there is a specific need), it is true. So sort of following that idea already.

Then the question is what is a low cost skill? My draft PR for using maximizer adds efficiency as a way to sort skills. I'll ponder on this more but maybe I'll incorporate that new maximizer function into this change

Yes change to 500 is on purpose. Follows the proposal above where we are only going to maximize gear if and only if we really want +item

Thanks for the feedback

@quarklikeadork
Copy link
Contributor

Its a good point. Perhaps the doEquips flag should be changed to doEverything flag

Could be issues with non-equip providers #1156

I think most item locations are in zone_needItem() and still need equipment from pre adv, while some quest files added consumables to the highest needs?

There may be more maximizer values in autoscend that were tuned for 50item, like 49food and 49booze that you already have in the provider though they are not in doEquips

Copy link
Member

@Malibu-Stacey Malibu-Stacey left a comment

Choose a reason for hiding this comment

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

Great work on this!

RELEASE/scripts/autoscend/auto_providers.ash Outdated Show resolved Hide resolved
RELEASE/scripts/autoscend/auto_providers.ash Outdated Show resolved Hide resolved
@Alium58 Alium58 requested review from a team June 18, 2024 17:03
@dsimich dsimich mentioned this pull request Sep 24, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants