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

Added FieldDescriptionTooltip #14890

Merged
merged 1 commit into from Mar 10, 2018

Conversation

Projects
None yet
6 participants
@xecollons
Copy link
Contributor

xecollons commented Mar 8, 2018

Added the possibility to expand the tooltip shown on units in the field. Everyone can see these tooltips.

The structure used to add this description inside the YAML is
FieldDescriptionTooltip:
-Description: Description of the unit

This will add the "Description" to the unit's tooltip:
openra_fieldtooltip

Pull request related to issue #14453

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 8, 2018

OpenRA.Mods.Common/Traits/FieldDescriptionTooltip.cs(28,18): error CS0414: Warning as Error: The private field `OpenRA.Mods.Common.Traits.FieldDescriptionTooltip.self' is assigned but its value is never used

@ltem

This comment has been minimized.

Copy link
Contributor

ltem commented Mar 8, 2018

Thank you for your Pull Request and, additional to fixing the Error, would you please squash the commits.

@xecollons

This comment has been minimized.

Copy link
Contributor Author

xecollons commented Mar 8, 2018

Pushed again :)

@reaperrr
Copy link
Contributor

reaperrr left a comment

Looks good to me

public object Create(ActorInitializer init) { return new FieldDescriptionTooltip(this); }
}

public class FieldDescriptionTooltip : IProvideTooltipInfo

This comment has been minimized.

@pchote

pchote Mar 9, 2018

Member

This name doesn't sound right to me. How about just TooltipDescription?

This comment has been minimized.

@xecollons

xecollons Mar 9, 2018

Author Contributor

Don't mind changing it. I just wanted to be apart of just Tooltip, since it can be seen as a genereal tooltip, and this is only shown in field units.

This comment has been minimized.

@reaperrr

reaperrr Mar 9, 2018

Contributor

The term 'Field' itself could be confusing here for some people, as Field is also a term in programming.

@pchote's suggestion sounds better.

@xecollons

This comment has been minimized.

Copy link
Contributor Author

xecollons commented Mar 10, 2018

Renamed class to TooltipDescription.


namespace OpenRA.Mods.Common.Traits
{
[Desc("Shown info on the build palette widget for field units.")]

This comment has been minimized.

@GraionDilach

GraionDilach Mar 10, 2018

Contributor

units on the field

This comment has been minimized.

@GraionDilach

GraionDilach Mar 10, 2018

Contributor

Or maybe Additional info shown in the battlefield tooltip.

namespace OpenRA.Mods.Common.Traits
{
[Desc("Shown info on the build palette widget for field units.")]
public class TooltipDescriptionInfo : ITraitInfo

This comment has been minimized.

@GraionDilach

GraionDilach Mar 10, 2018

Contributor

It would be very beneficial to make this into a ConditionalTrait.

@xecollons

This comment has been minimized.

Copy link
Contributor Author

xecollons commented Mar 10, 2018

Changed base class to ConditionalTrait(if Travis likes it). Now, the way of using this trait in the YAML is
TooltipDescription:
-Description: Description of the unit
-RequiresCondition: condition

public TooltipDescription(TooltipDescriptionInfo info)
: base(info)
{
}

This comment has been minimized.

@reaperrr

reaperrr Mar 10, 2018

Contributor

One last style nit: If brackets are completely empty, move them behind the previous line:
: base(info) { }

This comment has been minimized.

@xecollons

xecollons Mar 10, 2018

Author Contributor

Did that, but without the spaces, and Travis didn't like it.

Pushed again :)

@GraionDilach
Copy link
Contributor

GraionDilach left a comment

👍 asap @reaperrr's nit is addressed.

@GraionDilach
Copy link
Contributor

GraionDilach left a comment

Needs another fixup-rebase now though.

@reaperrr reaperrr merged commit 227cf35 into OpenRA:bleed Mar 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 10, 2018

@xecollons xecollons deleted the xecollons:FieldDescriptionTooltip branch Mar 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.