Page MenuHomeWildfire Games

Visual Heal Range
ClosedPublic

Authored by Sandarac on May 6 2017, 8:16 AM.

Details

Summary

Now that the aura range visualization option has been added in rP19519, options for displaying heal range and attack range are planned. For attack range, it is mentioned in the trac ticket that it will be important to show the range in the building placement preview, and this will likely require more changes in CmpSelectable.

There needs to be new texture defaults for both of these new range visualizations; I've modified one of Pureon's textures, but different ones will likely be needed.

Also this diff removes a redundant check for this.enabled in RangeVisualization.js, and an unused var in toggleRangeOverlay() in session.js.

Test Plan

Test that the range of healers can be displayed in the game when these units are selected, and that pressing the hotkey toggles the display of the ranges of selected healers. Verify that the healer range is updated instantly (if a healer is selected and the option is enabled) when the techs that modify their ranges are researched.

This diff that modifies template_unit_support_healer.xml can be used to verify that specifying custom textures/line thickness works as expected (when the attached textures are put under binaries/data/mods/public/art/textures/selection/):

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 1634
Build 2577: Vulcan BuildJenkins

Event Timeline

Sandarac created this revision.May 6 2017, 8:16 AM
Sandarac edited the summary of this revision. (Show Details)May 6 2017, 8:25 AM
Vulcan added a subscriber: Vulcan.May 6 2017, 9:03 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1029/ for more details.

Sandarac added a comment.EditedMay 7 2017, 12:45 AM

Using a heart-shaped texture/mask:

The overlay cannot be rendered as anything other than a line, and in order to have jagged/irregular shapes for SOverlayTexturedLine overlays (or rather just rendered overlays in general) there will need to be changes somewhere in the renderer code (or the issues with quad rendering need to be fixed (#1368)).

Sandarac updated this revision to Diff 1838.May 11 2017, 1:51 AM

Move the range visualization options to "General" in the options menu, and use "display" instead of "rendering" in the comment in the config file.

Sandarac edited the test plan for this revision. (Show Details)May 11 2017, 1:55 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1135/ for more details.

elexis added a subscriber: elexis.May 15 2017, 5:56 PM

There is some odd bug. If we train the mauryan healer hero, the range visualization won't be shown until selecting him the first time. Then it will be shown forever.

binaries/data/mods/public/simulation/components/Heal.js
102

After getting used to female auras, 0.2 seems a bit ugly, 0.15 looks better IMO.
(Apparently it's appealing if the thickness increases proportional with the range)

binaries/data/mods/public/simulation/components/RangeVisualization.js
59

The enabled property has no justification for existence, has it?

95

Why exactly did we add the argument if the information is already contained in the function name?
Shouldn'we have only one function GetVisualRanges(type)?

A getter function should get something, so should be called Refresh, Reload, Update or Regenerate

Sandarac planned changes to this revision.May 16 2017, 8:28 AM

Thanks for the comments.

That definitely shouldn't happen for the Mauryan healer hero (I tested with basic healer units, one would think the behavior would be the same, and this leads me to think there could be something askew in cmpSelectable, as the only difference I can see is that the Mauryan healer has m_AlwaysVisible as it is a hero.)

elexis accepted this revision.May 25 2017, 2:05 PM

The healer bug seems to be independent of this patch. The enabled property too.

Sandarac updated this revision to Diff 2187.May 25 2017, 7:34 PM

Actually fix the hero healer bug.

This revision is now accepted and ready to land.May 25 2017, 7:34 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1354/ for more details.

elexis added inline comments.May 26 2017, 2:19 PM
binaries/data/mods/public/simulation/components/RangeVisualization.js
57

The component should be agnostic of the fact that there is a hotkey.
Why doesn't the range vis renderer ignore the m_AlwaysVisible altogether?

Sandarac updated this revision to Diff 2266.EditedMay 28 2017, 5:11 AM

Don't save a another var to the rangevis comp that doesn't need to be saved.

elexis accepted this revision.May 28 2017, 7:04 AM

Thanks. Good that we can make use of this new system!

binaries/data/mods/public/gui/manual/intro.txt
112
binaries/data/mods/public/gui/options/options.json
115

Didn't we want to add %(hotkey)s?

binaries/data/mods/public/gui/session/hotkeys/misc.xml
101

fuzzing failed. this hotkey should be below the other range vis hotkey

binaries/data/mods/public/gui/session/session.js
1279

By the looks of things we just have to call these statements below when closing the options page to make that one actually work.

binaries/data/mods/public/simulation/components/RangeVisualization.js
71

Better avoid defaults, so that callers are required to think about the value they want to pass / enforce people being conscious of the behavior triggered

72–73

I still have trouble understanding why this line has to be this way and why all of this can't be just more straight forward. But I can confirm that this is the only term of these booleans that works.

87–96

Always pass all arguments seems cleaner

This revision was automatically updated to reflect the committed changes.

Build has FAILED

Link to build: http://jw:8080/job/phabricator/1391/
See console output for more information: http://jw:8080/job/phabricator/1391/console