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

Added calculation of crafting reagents needed for uncollected Things for selected item #923

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lOlbas
Copy link
Contributor

@lOlbas lOlbas commented Jan 22, 2022

Features

  • Calculating number of reagent needed to craft uncollected Things, with upper and lower bounds in case of randomness or multiple recipes for same reagent being involved.

How to debug

There is a TypeScript implementation of the calculation logic available to ATTCraftCalcSandbox repo. Follow a README there for details.

Problems and considerations

  • Calculation for deeply nested Things does not account for all items. Currently ATT only displays 4 levels of nesting, but in case of Primal Earth for example, it requires more than 4 to calculate all included items.
  • Alchemy transmutations are recursive which causes problems in calculations. One way would be to have an option to disable transmutations (will have to create a list of these spells though).
  • Recipes are saved account-wide, not realm-/server-/faction-wide
  • The tooltip will not show calculation unless user profession windows were opened before.
  • In certain cases the calculation may not be performed and thus not showing in the tooltip. For example, the resulting item in the TradeSkill UI, or world quest rewards (in this case, item ID (paramB) value is not whole number, but sometimes a seeming float value #.003407).
  • In cases where random number of crafted items generated, the calculation will show range of reagent count needed. However the lower bound will most likely be higher than what the actual lower bound would be. This happens because if we "overcraft" certain reagent, we do not account for it in consequent calculations, which builds up over time.
  • In certain cases where collectable uncollected Thing is used as reagent for another collectable uncollected thing, reagents for the former are calculated multiple times. It is possible to account for that by tracking items to craft, and this could be used as an option, but I believe that it might also bring confusion about how numbers are calculated, and also it gets trickier when item can be used N times, but we calculate for number < N.
  • The calculation does not account for items that the player already has. Read next paragraph for more.
  • Currently the new data should take about 1MB for all professions. This data exists alongside an already existing "Reagents" data.

Future improvements

  • This could be used to create a separate list of items user can track. Will be easier to search them on auction, or even integrate list with TSM. This sounds more like a separate addon though.
  • Account for existing materials across characters. For example if I need 100 Living Steel but I already have 50, hovering over Trillium Bar will show number twice as low as what it would show now. This will make planning way easier.
  • Instead of "Reagents" and "Recipes" use one structure for all use cases.
  • Option view count of material itself used to craft uncollected. For example, hovering over some Cloth will not count materials needed for Bolts for the same collectables (can be already calculated with the help of calculateNested parameter - untested).
  • Option to disable alchemy transmutations.
  • Option to exclude item from craft list, means that even if I can craft this item, it will not be counted as such.

Examples

Shal'dorei Silk

изображение

River's Heart

изображение

TODO

  • Add more spells like the one mentioned here
  • Update option description with some warnings

@lOlbas lOlbas changed the title Merged master branch Added calculation of crafting reagents needed for uncollected Things for selected item Jan 22, 2022
@Molkree Molkree requested a review from ImUnicke January 23, 2022 14:39
@Molkree Molkree added Professions Issue related to Professions and Recipes Suggestion New feature or request labels Jan 23, 2022
@Molkree
Copy link
Collaborator

Molkree commented Feb 6, 2022

Any progress on this?

@ImUnicke
Copy link
Collaborator

ImUnicke commented Feb 6, 2022

One note is (in my opinion) pretty much all of the 'future improvements' are out of scope for ATT and potentially would be better suited as a separate addon which could integrate with/rely on the base ATT addon. Currently, ATT tries to give a consolidated 'what/where/how' for collection of a Thing. Expanding that into actual planning of the means by which to get Things is a little out of scope I would say (as far as checking characters for reagent counts and modifying tooltip values based on existing items, integrated with TSM for shopping lists, etc.)

In certain cases the calculation may not be performed and thus not showing in the tooltip. For example, the resulting item in the TradeSkill UI, or world quest rewards (in this case, item ID (paramB) value is not whole number, but sometimes a seeming float value #.003407).

This is done because sometimes Items have additional values defined (which makes life more terrible) that potentially should cause the Item to be treated differently. This value is called a modItemID internally in ATT, and denotes the ItemID, ModID, BonusID of a given Item. Using the function GetItemIDAndModID() we can split the float value into the proper 3 components. However, sometimes the float value specifically is necessary for determining accurate results, such as when the same Item is used to buy variable sets of Things based on the specific float value (i.e. Covenant Weapon vendors from Castle Nathria. Same Item used for all 4 difficulties of weapons, but differing ModID defines unique purchases).

Alchemy transmutations are recursive which causes problems in calculations. One way would be to have an option to disable transmutations (will have to create a list of these spells though).

This would need to be handled similar to the logic in app.BuildCrafted() where the set of calculated Items is tracked "globally" so as to prevent an infinite recursion loop. This would be preferred over having that possibility in the logic, and manually defining recipes which need special treatment.

Currently the new data should take about 1MB for all professions. This data exists alongside an already existing "Reagents" data.

We can detect and force the data to be re-generated by the user if the structure needs to be changed for the storage of reagent/recipe data, since with this technique you are definitely duplicating a lot of the already-existing information (though it is harder to access in the way it is currently stored). So if another structure is desired that efficiently meets the needs of existing logic as well as this new logic, we can incorporate that 'alert' to the user. For instance the current check of it being out of date expects a version of 2:

	-- Verify that reagent cache is of the correct format by checking a special key
	local reagentCache, reagentCacheVer = app.GetDataMember("Reagents", {}), 2;
	if not reagentCache[-1] or reagentCache[-1] < reagentCacheVer then
		C_Timer.After(30, function() app.print(L["REAGENT_CACHE_OUT_OF_DATE"]); end);
		wipe(reagentCache);
		reagentCache[-1] = reagentCacheVer;
	end

Calculation for deeply nested Things does not account for all items. Currently ATT only displays 4 levels of nesting, but in case of Primal Earth for example, it requires more than 4 to calculate all included items.

This is because you are basing your calculations from the entries variable which includes only potential information to display in the tooltip. If the logic was altered to instead utilize the group variable in the respective scope, you will find there should not be the same limit of depth, though the structure of the data would be a bit different and may require slightly different calculation technique.

I haven't gotten to testing this myself yet, and it looks like there are further adjustments still planned. I will try to keep a better eye on any changes here 👍

@Molkree
Copy link
Collaborator

Molkree commented Feb 22, 2022

@lOlbas @ImUnicke
Any progress on this? Even if not all TODOs are implemented I would like this merged in after 9.2 release, people are asking for this feature!

If there are no addon breaking bugs and this functionality can be disabled it should be fine and we can refine it later.

@lOlbas
Copy link
Contributor Author

lOlbas commented Feb 22, 2022

Having this feature as a separate addon is definitely a great idea given the possible spectrum of features. If it were up to me, I would add this as Experimental given few limitations related to accuracy of calculation and the fact that the calculation is based off of what can be displayed in tooltip. I'd maybe even add a tooltip saying something along the lines of "Looking for developers willing to move this feature in a separate addon". Although players feedback will likely help with the decision

@Molkree
Copy link
Collaborator

Molkree commented Jun 5, 2022

@ImUnicke, any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Professions Issue related to Professions and Recipes Suggestion New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants