-
-
Notifications
You must be signed in to change notification settings - Fork 50
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 a JS function to clean up orphaned entities #664
base: master
Are you sure you want to change the base?
Conversation
Q_ASSERT(sessionID.isNull()); | ||
entity->collectChildrenForDelete(entitiesToDeleteImmediately, sessionID); | ||
_deletedEntityCounter += entitiesToDeleteImmediately.size(); | ||
_entityTree->deleteEntitiesByPointer(entitiesToDeleteImmediately); |
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.
Is it okay to delete entities while iterating over them? Perhaps this needs doing after forEachEntity
is done?
why does this need to be invokeable from JS? shouldn't this be run purely from the c++ side? maybe when domains first load and then again on backups or somewhat regularly? also what if someone deletes a parent entity on purpose and just hasn't done anything to handle the child entity yet? should create automatically try to clear the parentID of the children and maybe correct the transform to stay in the same place? |
I totally agree - probably calling this once entity mixer starts could make most sense.
As far as I know deleting an entity normally deletes all its chldren too - please correct me if I'm wrong. I think it's just that there's some bug that makes it not work on some rare occasions. |
ah yeah you're totally right you could take a look at EntityTree::readFromMap for a good place to do this! |
This is a bit a freaky PR. ;-) If i remember well, one of the cause of getting orphan was the delete and copy/paste I have domain that I know that contain some. If you need a copy to make test... I can check for that. |
This allows removing orphaned entities that cannot be removed by other means and persist even in JSON files in backups. It's not tested yet.