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
Fixed Tiberian Sun building repair and power down indicator palette #7957
Fixed Tiberian Sun building repair and power down indicator palette #7957
Conversation
@@ -19,6 +19,11 @@ public class CanPowerDownInfo : UpgradableTraitInfo, ITraitInfo, Requires<PowerI | |||
[Desc("Restore power when this trait is disabled.")] | |||
public readonly bool CancelWhenDisabled = false; | |||
|
|||
public readonly string PoweroffImage = "poweroff"; |
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.
You use a different naming style for each option. Please be consistent! My preference is IndicatorImage
, IndicatorSequence
, and IndicatorPalette
.
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.
Renamed.
The repair indicator has the same palette problem, so it would make sense to fix that at the same time. |
5ddd20f
to
18fda23
Compare
Fixed the repair palette as well. |
[Desc("Overrides the IndicatorPalettePrefix.")] | ||
public readonly string IndicatorPalette = ""; | ||
|
||
[Desc("Suffixed by the interal repairing player name.")] | ||
public readonly string IndicatorPalettePrefix = "player"; |
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.
Sorry to keep feature-creeping, but could we change PalettePrefix
→ PlayerPalette
to match what we do in the render traits?
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.
Can I get away without an update rule?
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.
I didn't think about that... lets leave this for a future pr which can include an upgrade rule.
49fbdce
to
ca30ee4
Compare
Was forced to rebase this. |
Works ingame. 👍 / ✅ |
{ | ||
this.building = building; | ||
this.palettePrefix = palettePrefix; | ||
|
||
rb = building.TraitOrDefault<RepairableBuilding>(); |
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.
This should be .Trait so that it throws a useful exception instead of NREing below.
For some additional polish it would be nice if you could fix the indicator offsets, but that's not a blocking issue here. |
ca30ee4
to
2abe0c2
Compare
Updated. |
int shownPlayer = 0; | ||
PaletteReference palette; |
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.
This is only ever used as a local variable, so it shouldn't be stored as a field.
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.
Moved.
2abe0c2
to
a5f60de
Compare
👍 |
Fixed Tiberian Sun building repair and power down indicator palette
Also exposes the required sequences to https://github.com/OpenRA/OpenRA/wiki/Traits documentation.