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

Enchantment cache #34145

Merged
merged 3 commits into from
Sep 28, 2019
Merged

Enchantment cache #34145

merged 3 commits into from
Sep 28, 2019

Conversation

KorGgenT
Copy link
Member

Summary

SUMMARY: Infrastructure "Create enchantment cache for use with enchantment values"

Purpose of change

As part of the ongoing project of the artifact project. This particular PR puts down the infrastructure needed in order to calculate values for things that do not already have a "bonus" (like the stats and speed) This effectively makes an enchantment object on Character that has a list of bonuses.

Describe the solution

Add an enchantment_cache member to Character and logic for building the cache every turn. This cache will be built for avatar and npcs, so this is likely something that can and should be optimized. However, due to the fact that enchantments in mainline will likely only be on artifacts, and artifacts are few and far between, the cost might be negligible.

Also, to demonstrate the use for the cache and that the cache works, implements MOVE_COST from the list in #33837 and adds a small buff to ice gliders in magiclysm.

Describe alternatives you've considered

getting the values from all the items in your inventory, worn, and wielded, checking if they're active, etc. every single time a variable in the code might be modified by an enchantment value. Obviously this thought didn't go too far, since building the cache would be miles better, as you don't have to iterate through who knows how many items.

However, one thing that can be considered is the location of the recalculate_enchantment_cache function; i put it in update_body, since the cache lives in Character, but it doesn't feel quite right to me, as it doesn't really feel like something that would be part of the "body," but that might just be a matter of nomenclature.

Additional context

It might be possible to use this cache rather than the separate logic I wrote in process_artifacts altogether. I might think of that later on in the project's life, when considering removing the previous artifact infrastructure.

@KorGgenT KorGgenT added [JSON] Changes (can be) made in JSON [C++] Changes (can be) made in C++. Previously named `Code` Artifacts Otherworldly items with special effects labels Sep 21, 2019
@kevingranade
Copy link
Member

I'm getting a crash from the ice gliders in game::process_artifact()

@kevingranade
Copy link
Member

Unchanged I'm afraid.
20190925_002438

@KorGgenT
Copy link
Member Author

i'll do some digging, but i have as of yet not been able to replicate the crash

@kevingranade
Copy link
Member

Welp, went away here too 👍

@kevingranade kevingranade merged commit c8f8da4 into CleverRaven:master Sep 28, 2019
@KorGgenT KorGgenT deleted the enchantment-cache branch September 28, 2019 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Artifacts Otherworldly items with special effects [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants