i233 Deep Migration Warning #237
Conversation
| //The size of the underlying data could have changed such that we re-allocated the memory, so we'll update | ||
| //our shared statics | ||
| UpdateSharedStatics(); | ||
| } | ||
|
|
||
| private static void ScanForCollections(string parentPathString, Type type) |
There was a problem hiding this comment.
Should this be a safety only operation? I don't think this would come up uniquely in release
There was a problem hiding this comment.
Yes for now, but I've noted that it won't be in the full implementation of #233 because we need to store the type offset information.
| { | ||
| continue; | ||
| } | ||
| if (fieldType.IsPointer) |
There was a problem hiding this comment.
I don't know where you want to put this for future reference but we discussed how there may be cases where a pointer is being stored that we do not want to dive into an migrate. Maybe that memory address has already been migrated or something.
We want to figure out how to handle these situations. Maybe require developers to annotate unsafe fields with a [RemapUnsafeEntityDataOnMove] or something. Would need an [ExcludeUnsafeEntityDataOnMove] (names WIP)
# Conflicts: # Scripts/Runtime/Entities/Lifecycle/Migration/MigrationUtil.cs
Scanning types that are being registered for Migration and emitting a warning if there's a chance we could be hiding Entity references inside them.
Precursor to more involved work in #233
What is the current behaviour?
No way to tell other than developer thoroughness
What is the new behaviour?
We now scan the type being registered for Entity remapping and see if anything could be hidden via an Unsafe collection's pointer.
We then emit a warning to the console so developers can ensure it is handled.
What issues does this resolve?
What PRs does this depend on?
Does this introduce a breaking change?