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

Update variables for Flash lua function. #19943

Merged
merged 1 commit into from
Mar 5, 2022

Conversation

MustaphaTR
Copy link
Member

Another issue that was reported on #19920. The variables doesn't match the effect and Duration was being passed as alpha. I couldn't find any use of this in base mods, i assume that's why the issue wasn't found out so far.

Also replaced asPlayer with just Color, you can just use Player.Color as color variable.

I asked on Discord about float variables, it seems to be currently not possible in Lua so i left the Alpha value as the default 0.5f instead of making it settable.

@RoosterDragon
Copy link
Member

Are there any usages of the function that need updating?

@MustaphaTR
Copy link
Member Author

Are there any usages of the function that need updating?

I couldn't find any use of this in base mods, i assume that's why the issue wasn't found out so far.

@abcdefg30
Copy link
Member

It would still be better to have a method with the old signature and a deprecation warning.

@MustaphaTR
Copy link
Member Author

Old method was broken a good while ago, there is no longer a duration variable to give to FlashTarget effect to make it actually work and it was being passed as Alpha value. I don't see any point on keeping it as broken and not sure how fixed one would work there.

@abcdefg30
Copy link
Member

Yes, it wouldn't work properly. But since we can't lint/update Lua scripts automatically changing the signature will crash scripts during execution with a more or less helpful message. Therefore we usually used deprecation. I'm also not convinced this worth the effort here, but it'd still be "nicer"/better to have it.

@MustaphaTR
Copy link
Member Author

Updated.

@abcdefg30
Copy link
Member

Ah crap, now we get an error on the shellmap of RA:
Actor 'rock6' defines the command 'Flash' on multiple traits
I guess the Lua bridge can't handle multiple signatures, so we probably need to remove the deprecated method after all. 😞
Sorry for the back and forth.

@MustaphaTR
Copy link
Member Author

Updated.

@abcdefg30 abcdefg30 merged commit 0203476 into OpenRA:bleed Mar 5, 2022
@abcdefg30
Copy link
Member

Changelog

@MustaphaTR MustaphaTR deleted the update-flash-variables branch March 5, 2022 15:25
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