search

CyclopsMC / CommonCapabilities

Forge Capabilities that can be shared by multiple mods.
http://cyclopsmc.github.io/CommonCapabilities/
MIT License
11 stars 11 forks source link

Refactor getRecipes in preparation for Forge's changes to getValues #14

Closed josephcsible closed 6 years ago

josephcsible commented 6 years ago

The change in question is that they're going to start giving you a Collection<IRecipe> instead of a List<IRecipe>. After I made this, I noticed that this now makes an independent list rather than a view that stays up-to-date. If the latter is important, then we'd need to change IRecipeHandler#getRecipes() to return a Collection<RecipeDefinition> instead of a List<RecipeDefinition> (which would be a breaking change), but then we could just use Collections2.transform in place of Lists.transform.

rubensworks commented 6 years ago

AFAIK, stream().map() doesn't create a lazy-transformed list, so this change will introduce a big performance issue when only subsets of the given list are needed.

I haven't looked into Collections2.transform yet, but that sounds good to me at the moment. The recipe capability is not included in public releases anyways, so breaking changes are fine.

josephcsible commented 6 years ago

It looks like Collections2.transform is exactly what we want then, if breaking changes are okay. I'll change this PR to do that tonight.

josephcsible commented 6 years ago

This now depends on CyclopsMC/CommonCapabilitiesAPI#5.

rubensworks commented 6 years ago

@josephcsible https://github.com/CyclopsMC/CommonCapabilitiesAPI/pull/5 has been merged, could you update the submodule in this PR as well?

josephcsible commented 6 years ago

Done.