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 caching to get #2183

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

added caching to get #2183

wants to merge 1 commit into from

Conversation

ArnoldSmith86
Copy link
Owner

@ArnoldSmith86 ArnoldSmith86 commented Jul 21, 2024

Every time get is called, it evaluates the current value of the given property. For cards or when inheritFrom is involved, this can take a bit.

This PR makes get remember the last value it determined and hopefully every possibility of changing that value, removes the cached value.

Now I am very certain that this does not handle all the cases yet. But at least now my test case does not go in an endless loop anymore. The test case was switching the game to Color Whist in the Card Games game. Before this change, it took 2.77s on my laptop. Now it takes 1.19s.

Please help to find stuff that breaks these changes.

  • Check if this breaks properties with ${PROPERTY x}.

@ArnoldSmith86
Copy link
Owner Author

PR-SERVER-BOT: You can play around with it here: https://test.virtualtabletop.io/PR-2183/pr-test (or any other room on that server)

@bjalder26
Copy link
Collaborator

Here is one thing (I think).

Faces doesn't appear to work the same as on the production server. It doesn't seem to actually update anything as you change the faces.

https://test.virtualtabletop.io/PR-2183/pr-test
https://virtualtabletop.io/PR-2183

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.

2 participants