This repository was archived by the owner on Dec 29, 2024. It is now read-only.
Fix some regression issues#130
Merged
Merged
Conversation
Member
Author
|
LuaJIT module tests failed, but I can not replicate on my system. I mustve ran tests 20 or 30 times in a row without any crash like what happened on the runner. Ive also run about 10 iteration of tests using the same bin from the github actions build. And since it passed on a rerun I am going to be merging this. Leaving this comment as note though in case something pops up. |
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The previous set of changes caused some regression due to the differences of how GDExtension and Modules work. memove is needed for GDExtension because all Variants for it are pointers to Variants living in the engine, assigning to a null Variant allocated by lua will cause a null pointer deref. Therefore we skip the assignment operator by using memmove. We need to do some manually ref counting to make this work.
For modules memmove caused a lot more issues so this PR has the Module version use the assignment operator.
The other major change was we now test if a Callable is custom and send it as a blank LuaCallableExtra instead. We do this to create a wrapper to the callable custom which will always have a RefCounted. Without this we have no way to manually increment the ref counter so it gets cleaned up.
There was also some steps missing to fully expose the GC options which have been fixed.