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

Improve comestible menu performance #26272

Conversation

ralreegorganon
Copy link
Contributor

Summary

SUMMARY: Bugfixes "Improve comestible menu performance."

Purpose of change

The lambdas for determining calories and joy for the player in the comestible menu were capturing a copy of the player. With the current map memory bugs, this meant a potentially very large object was being copied several times, every time the comestible menu was displayed, leading to a large delay.

Describe the solution

Changed the lambdas to capture the player object by reference.

Additional context

Fixing the map memory will also help resolve this, but this will mitigate the issue in the meantime.

The lambdas for determining calories and joy for the player in the
comestible menu were capturing a copy of the player. With the
current map memory bugs, this meant a potentially very large object was
being copied several times, every time the comestible menu was
displayed, leading to a large delay.
@kevingranade
Copy link
Member

Just double checked, player::fun_for() and player::nutrition_for() are const.

@kevingranade kevingranade merged commit 320fbb3 into CleverRaven:master Oct 16, 2018
@jbytheway
Copy link
Contributor

Does it ever make sense to copy a player object? Should it be made non-copyable to catch these sorts of issues?

@kevingranade
Copy link
Member

kevingranade commented Oct 17, 2018 via email

@ralreegorganon ralreegorganon deleted the comestible-inventory-lambda-player-capture branch November 14, 2018 18:05
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.

None yet

3 participants