fix: readd deserialization-time validation for iota holders - #1167
fix: readd deserialization-time validation for iota holders#1167IridescentVoid wants to merge 3 commits into
Conversation
|
It looks like |
|
I think the update after every frame might lead to issues in multiplayer, as could any update without actually trying to read the iota. |
|
To be clear, "every frame" was referrering to every ContinuationFrame the CastingVM evaluates. Since it only happens as part of an active cast and only works on the stack, it shouldn't impact any inert storage mediums like iota-holding items. |
| result | ||
| result.copy( | ||
| newData = result.newData?.copy( | ||
| stack = validateIotaList(result.newData.stack, world) |
There was a problem hiding this comment.
We should also validate the ravenmind, since that is user-visible. Data internal to frames (such as the foreach accumulator) should be fine to leave as-is as it'd need to surface on the stack at some point for a hex to take advantage of it.
It may also be worth implementing caching for validation (since iotas are immutable) to reduce the performance impact of the eager checking.
There was a problem hiding this comment.
Can you maybe describe a bit more how you imagine the caching? From my understanding an entity could become unavailable at any time (logout, despawn, unload, death), and I don't really see a good way to invalidate a cache when that happens.
There was a problem hiding this comment.
Oops, 3 am iris wasn't thinking straight. We can't cache things that hold volatile iotas. However, most iota types don't decay and always pass validation, so a list holding a large amount of vectors/numbers could still have validation skipped on subsequent passes which could help with performance still.
If the changes in this PR do not impact performance significantly, I think it would be fine to push caching to a later pull request (especially since this technically adds mutable internal state to Iotas, which I think is safe but would need to verify).
Oh, I see, that makes more sense :D |
Fixes #1160
Alternative implementation of #1161 that performs iota validation in the
readIota()call where the iota component is deserialized instead of when OpRead/OpTheCoolerRead return. This should better ensure that the validation code gets called in the future as well.I also opted to validate the stack to null newly disappeared entity iota after each frame, which should properly prevent anything from consuming invalid iota. This might have performance implications. Interestingly, 0.11.3 on 1.20.1 would have allowed operating on nonexistent/despawned entities as EntityIota used to hold Entity references directly.
Closes #1161