-
Notifications
You must be signed in to change notification settings - Fork 35
Windows: Fix unbounded growth of stored scoped attributes #259
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
base: master
Are you sure you want to change the base?
Conversation
…efs/registry. Fix unbounded growth of scoped attributes stored in PlayerPrefs/registry on Windows. - Maintain a HashSet of keys and serialize back to a compact List to prevent duplicates - Skip PlayerPrefs writes when the new value matches the stored value - Parse + cleanup in CleanScopedAttributes/GetScopedAttributes
…lient - Verifies duplicate keys are not added repeatedly - Confirms PlayerPrefs writes are skipped when value unchanged - Retains backward compatibility with legacy attribute reads
|
|
||
| try | ||
| { | ||
| var attributes = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its our contract, and it should never break.
| try | ||
| { | ||
| var attributes = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson); | ||
| if (attributes?.Keys != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the if statement is invalid, then the catch should capture this. Since this is our contract and we require the people to follow it, why we should check it?
| { | ||
| try | ||
| { | ||
| var container = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why you cannot deserialize array into hashset?
| { | ||
| var container = new ScopedAttributesContainer | ||
| { | ||
| Keys = keys.OrderBy(k => k, StringComparer.Ordinal).ToList() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need it? also the same order needs to be applied to values.
WHY
What changed
ref: BT-5953