-
Notifications
You must be signed in to change notification settings - Fork 133
Let LuaComponent respect bImplicitSelf #77
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
Open
micheljung
wants to merge
2
commits into
rdeioris:dev
Choose a base branch
from
micheljung:mj/fix-implicit-self
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
Author
|
How about this as a (transitional?) opt-in solution: /**
* Whether to implicitly pass this component as the first argument (usually <code>self</code> when calling a Lua
* function.
*
* Currently, this is only respected by <code>ULuaState::MetaTableFunction__call</code> and
* <code>ULuaState::MetaTableFunction__rawcall</code>. If you want this to work for <code>LuaCall...</code> functions
* as well, you need to enable <code>bImplicitSelfForLuaCalls</code>, too.
*/
UPROPERTY(EditAnywhere, Category = "Lua")
bool bImplicitSelf;
/**
* When enabled, <code>bImplicitSelf</code> will be respected by all <code>LuaCall...</code> functions.
*
* See <a href="https://github.com/rdeioris/LuaMachine/pull/77">this PR</a>.
*/
UPROPERTY(EditAnywhere, Category = "Lua")
bool bImplicitSelfForLuaCalls; |
micheljung
commented
Jul 20, 2025
micheljung
commented
Jul 20, 2025
62e6869 to
3b16669
Compare
micheljung
commented
Jul 20, 2025
Comment on lines
+114
to
+120
| bool bAddSelfArg = bImplicitSelfForLuaCalls && bImplicitSelf; | ||
| // first argument (self/actor) | ||
| L->PushValue(-(ItemsToPop + 1)); | ||
| int NArgs = 1; | ||
| if (bAddSelfArg) | ||
| { | ||
| L->PushValue(-(ItemsToPop + 1)); | ||
| } | ||
| int NArgs = bAddSelfArg ? 1 : 0; |
Author
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.
Please check that I'm not doing it wrong here; see line 108
micheljung
commented
Jul 20, 2025
Comment on lines
+158
to
+164
| bool bAddSelfArg = bImplicitSelfForLuaCalls && bImplicitSelf; | ||
| // first argument (self/actor) | ||
| L->PushValue(-(ItemsToPop + 1)); | ||
| int NArgs = 1; | ||
| if (bAddSelfArg) | ||
| { | ||
| L->PushValue(-(ItemsToPop + 1)); | ||
| } | ||
| int NArgs = bAddSelfArg ? 1 : 0; |
Author
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.
Same, see line 151
This makes it easier for contributors. I'm not sure what the current max line length is, but I guess it's 140. Also, the current code isn't always properly formatted, this should be fixed in a separate commit.
3b16669 to
24d7b60
Compare
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.
My idea is to connect a Lua table to an Actor, but without "mixing" them.
So
UShipLuaComponent::LuaObjectreferences the Lua table:/** The Lua table that is connected to this actor component. */ FLuaValue LuaObject;When the actor is spawned, a Lua table should be created for it:
The reason I do this is because modders should be able to add functions to
Ship:But they can also delegate to the
LuaComponent:And when the engine calls a function like
Tick,selfshould be the Lua table, not theLuaComponent:To achieve this, I need to be able to call a Lua function without passing the
LuaComponentimplicitly asself. I thought that the settingbImplicitSelf = falsewould achieve this. But in methods likeLuaCallFunction, this flag is not respected.This PR fixes that.
Unfortunately, this is a breaking change. To preserve the old behavior,
bImplicitSelfwould need to default totrue, but that would also be a breaking change.Maybe I'm not getting it right anyway. In that case, maybe a new flag could be introduced to control the behavior? Then it would also not be breaking.