Use script.remove instead of removeChild for unity loader hook #576
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sorry I didn’t open a discussion beforehand. This fix was pretty urgent for our team so I went ahead and (hopefully) patched it. There might be a better approach, but I’m hoping this at least addresses the immediate issue. Would love your thoughts. |
alexqhj
force-pushed
the
use-reference-counting
branch
from
3938add to
ea30505
Compare
alexqhj
changed the title
|
yeah sorry for the mess. I completely overengineered the issue at first, after spending time debugging. Realized script.remove() would gracefully attempt to remove the node without having to keep references to how many instances of the loader script was initialized. |
jeffreylanters
approved these changes
Feb 26, 2025
|
No problem! Thank you very much for your valuable contributions! I'll releases these improvements soon! |
jeffreylanters
approved these changes
May 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Key changes:
undefinedinstead of an implicit return.removemethod instead ofwindow.document.body.removeChild. Previous version would produce error if the node was not present, i.e. when unloading multiple instances.