Unified FlashPaletteEffect and exposed it to Lua #7872
Conversation
Mailaender
changed the title
Mailaender
force-pushed
the
storm-effects
branch
from
74e11dd
to
0fe2de2
Compare
|
Shouldn't this subclass the global modifier? |
|
Why should it? |
| { | ||
| public readonly string[] ExcludePalettes = { "cursor", "chrome", "colorpicker", "fog", "shroud" }; | ||
|
|
||
| public readonly float LightningChance = 0.99f; |
There was a problem hiding this comment.
I think this should be 0.01f (or 1, as a percentage -- better if this needs to sync up with gameplay code later) and then change Tick below
There was a problem hiding this comment.
I agree with this.
A lower chance value naturally implies the even will (probably) occur less often.
|
Because you are just duplicating some logic. |
| public object Create(ActorInitializer init) { return new Thunder(init, this); } | ||
| } | ||
|
|
||
| class Thunder : ITick |
There was a problem hiding this comment.
Thunder is pretty specific when in all actuality you are just playing ambient audio.
There was a problem hiding this comment.
This plays an audio whenever the flash palette flashes. See #7871 for regular ambient audio.
There was a problem hiding this comment.
True, ambient was a bad word on my part, but my point still stands.
Being that this isn't necessarily a thunder sound, it is just a sound played on a condition (which is determined by another trait).
There was a problem hiding this comment.
Suggest an alternative then.
Okay, will try that. Then this depends on #7863 now. |
|
I don't like being the party pooper all the time, but I think that this use case (thunder/lighting) would be much better served by exposing a Lua API to #7863. That would then allow mappers to also do other things such as day/night cycles, turning the weather on/off, or 70's disco mode. |
|
When I saw how day/night-cycling is done in Tiberian Sun with triggers that also came to my mind. Traits are a bit simpler to use. We could have both ways to set them up. |
|
Traits are simple to use, but limited for this purpose. This trait is only really useable for the one or two maps that might want to feature a thunderstorm as a gimmick. The TS ion storm works rather differently. |
Mailaender
force-pushed
the
storm-effects
branch
3 times, most recently
from
ae3be8e
to
1898c82
Compare
|
I merged #7871 and #7863 in here as this seems to be the last one of them getting merged though not for technical reasons (GlobalPaletteModifier does not share enough logic to justify sub-classing), but to add a test case (Fort Lonestar) here. All those traits work together to create the atmosphere. |
|
Also tried to please the reviewers a bit. |
|
I think the lightnings need to be nerfed. Complete white is too much. |
|
Also needs a rebase. |
|
Can you review and merge #7871 first? |
|
Rebased and applied the requested changes. |
|
Needs a rebase again. |
|
Rebased and updated. |
|
|
||
| namespace OpenRA.Mods.Common.Scripting | ||
| { | ||
| [ScriptGlobal("Palette")] |
There was a problem hiding this comment.
I'd prefer if this reflected the function, not the underlying mechanism. How about "Environment"? We could put all the other (global) ambient sound, weather etc. stuff into that global, too, then.
There was a problem hiding this comment.
Maybe Effect? Nuke flashes aren't really environmental.
There was a problem hiding this comment.
I want to place other palette altering things here for day/night cycles etc.
There was a problem hiding this comment.
Day/night cycles were included in my "etc." :)
I'm just seeing this from a scripter's point of view. Many of those don't know and don't care that you're achieving the effect with modifying a palette, so referring to that in the API makes it less accessible IMO. The API should instead reflect what the scripter actually wants to do, not how it's done behind the scenes.
|
Looks good overall. 👍 once we've decided on a name for the API. |
|
Renamed things. |
Unified FlashPaletteEffect and exposed it to Lua
Split from #7863.