Skip to content
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

Add causes for TargetInvocationException in TriggerGlobal #20868

Merged
merged 1 commit into from
May 23, 2023

Conversation

Mailaender
Copy link
Member

@Mailaender Mailaender commented May 19, 2023

This addresses #18802 (comment) where a simple typo of a player name causes a large and difficult to read stacktrace.

InitObjectives = function(player)
Trigger.OnObjectiveCompleted(player, function(p, id)
Media.DisplayMessage(p.GetObjectiveDescription(id), UserInterface.Translate("objective-completed"))
end)

will crash when the player is null because the function expects a certain type

[Desc("Call a function when this player completes an objective. " +
"The callback function will be called as func(Player player, int objectiveID).")]
public void OnObjectiveCompleted(Player player, LuaFunction func)
{
GetScriptTriggers(player.PlayerActor).RegisterCallback(Trigger.OnObjectiveCompleted, func, Context);
}

and the type conversion fails while the script is being loaded, which is also why we don't get line numbers for some reason. This shortens the stacktrace by throwing earlier.

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks all script functions that want to pass nil. (See allies-01 or any other usage of reinforcements for example.)

@Mailaender
Copy link
Member Author

It indeed breaks in allies-01 with the second helicopter. My next approach throws even more

 ---> System.NullReferenceException: actor
   at OpenRA.Mods.Common.Scripting.TriggerGlobal.OnKilled(Actor actor, LuaFunction func) in OpenRA/OpenRA.Mods.Common/Scripting/Global/TriggerGlobal.cs:line 88

but adds the method signature to the exception.

@Mailaender Mailaender changed the title Fixed a TargetInvocationException in the Lua script member wrapper Add causes for TargetInvocationException in TriggerGlobal May 19, 2023
@Mailaender Mailaender requested a review from abcdefg30 May 19, 2023 19:02
abcdefg30
abcdefg30 previously approved these changes May 20, 2023
Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OnAllKilled is missing a null check. Also this should get rebased for easier testing

@PunkPun PunkPun merged commit 8433bc0 into OpenRA:bleed May 23, 2023
3 checks passed
@PunkPun
Copy link
Member

PunkPun commented May 23, 2023

Changelog

@Mailaender Mailaender deleted the lua-target-invocation branch May 23, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants